search

linebender / parley

Rich text layout library
Apache License 2.0
228 stars 28 forks source link

Initial version of snapshot testing #173

Open spirali opened 1 week ago

spirali commented 1 week ago

Hi,

This PR contains an initial version of snapshot testing.

By design it contains only a minimal API and usage inspired by vello snapshot tests. The ambition of this PR is not to do any test coverage, it contains only two tests for demonstration purposes of the testing environment.

All the bells and whistles (html report, image diffs, not loading fonts in every test) are now missing to make the code more straightforward.

Usage

$ cargo test

If a test fails, you can compare images in parley/tests/current (images created by the current test) and parley/tests/snapshots (the accepted versions).

If you think that everything is ok, you can start tests as follows:

$ PARLEY_TEST="accept" cargo test

It will update snapshots of the failed tests.

How it works

It reuses tiny-skia-renderer and renders the testing layout into an image and compares it pixel by pixel.

This PR also adds fonts into the tests, to have tests independent on the system fonts. I have chosen DejaVu fonts, but it was a random pick.

spirali commented 1 week ago

We definitely shouldn't import several megabytes of fonts.

We also need to follow the license for whatever fonts we do import.

The fonts are used only in tests, Parley itself can be build without it.

In the last change I have added license of the fonts.

spirali commented 1 week ago

Thank you - it will be great to get some testing landed.

However, We definitely shouldn't import several megabytes of fonts - it's pretty much a worst-case scenario for git; I'd suggest using only two fonts files; presumably we do want to test multi-font cases. We also need to follow the license for whatever fonts we do import.

Ok, I will try to prune it. However it would not be that bad for git, these files should only rarely changed, so it just a few constant megabytes.

I have also attached more variants so we can test e.g. different weights, because variable weighted font does not for me in Parley (I do not know if it a feature or a bug).

spirali commented 1 week ago

I have removed (and squashed) other variants and left just DejaVu Sans and DejaVu Sans Bold (= 2x 700KiB)

xorgy commented 5 days ago

It might make more sense to use a pinned version of a distro font package in the eventual CI (maybe Noto?), and just have it spit out a useful message when you don't have those fonts installed.

spirali commented 4 days ago

I have completely removed fonts from repository & added a script that downloads Noto Fonts.

spirali commented 4 days ago

Note: CI is now failing, because font download is not part of CI scripts.

DJMcNab commented 1 day ago

We had some discussion in office hours; the suggested path forward was to have one or two small font files in this repository. For example, in Vello, we have Roboto and Inconsolata. Those files in total are almost exactly 500KiB; I'd target a megabyte as the hard limit for total font files in the repository initially. An important tool here is subsetting - see for example https://github.com/linebender/vello/tree/main/examples/assets/noto_color_emoji which describes doing that for Emoji. My preference would be to choose fonts with either the OFL 1.1 or the Apache 2.0 license, because those are well-understood licenses. Using downloaded fonts has reproducibility and stability challenges, as the download service is a very easy point of failure.

This is a collaborative process, so feel free to push back on this - I recognise that we moved on quite quickly from this in the meeting today without getting confirmation from you.

At a first pass, the rest of the code looks really good, I hope to find time for a more thorough review soon. Thanks so much for working on this!

spirali commented 8 hours ago

Hi, I agree with the outcome of the discussion. Last changes: