Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Start for automated tests using pixelmatch #175

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

frigus02
Copy link

@frigus02 frigus02 commented Apr 9, 2020

I want to say a big thank you for building 渭Plot. It's working really nice for me.

In #72 you mentioned you're looking to add some (non-granular) tests. I wanted to play with pixel based snapshot tests for a while now and took this as an opportunity.

This PR uses jsdom to render all the existing demos and then pixelmatch to compare them to a stored snapshot. Everything is packaged in a GitHub workflow to run on push. If any snapshot doesn't match, the build fails and uploads diffs for all mismatched snapshots as an artifact to the build.

See recent push for an example: https://github.com/frigus02/uPlot/actions/runs/74668141

The whole thing runs in Docker to make font rendering consistent. I fond that otherwise labels are rendered slightly differently based on operating system and installed fonts.

  • npm test runs tests and stores diffs
  • npm run test:update runs tests and updates snapshots

Current status: some demos dont't work because they're generating random data. However this satisfied my desire to play, so I decided to stop here and see what you think 馃檪.

Is this the direction you want tests to go? Did you expect something very different?

If this goes in the right direction, I'm happy to either finish it or for you to take over if you prefer.

The labels uPlot draws on the canvas seem to look different based on
operating system and installed fonts. This change uses Docker to get
reliable results independent on the host the tests are executed on.
@leeoniya
Copy link
Owner

leeoniya commented Apr 9, 2020

hey @frigus02

thanks a lot for starting this! i'll make some general comments for now before i dive into the code.

In #72 you mentioned you're looking to add some (non-granular) tests.

yes, i think this can be expanded further now to basically take a JSON snapshot of the uplot objects (excluding some stuff like Path2D cache objects) and deep-diff the expected and actual state, too.

This PR uses jsdom to render all the existing demos and then pixelmatch to compare them to a stored snapshot.

i'm thinking instead of stringing together a bunch of heavy js libs (jsdom, canvas, canvas-5-polyfill), maybe running this via Puppeteer as one easily-updated dependency could make debugging easier, since we can opt in/out of headless and play with devtools in an actual browser instance.

https://dev.to/benjaminmock/how-to-take-a-screenshot-of-a-page-with-javascript-1e7c

Everything is packaged in a GitHub workflow to run on push. If any snapshot doesn't match, the build fails and uploads diffs for all mismatched snapshots as an artifact to the build.

honestly, this is a pretty crappy feedback loop. i don't want to first make commits, push, and then have to rely on an external service to run and github UI to ensure i haven't made errors. i want to make sure there are no errors before i even make a commit, so the tests must be runnable locally. i would prefer to create some genRefPngs("58ccf32...") function which runs when npm run test is called with an empty cache and it checks out the known-good commit and creates the pngs. i think this would solve the different rendering inconsistencies between platforms as well and allow it to be run locally for much more sane DX. i'd like to avoid committing reference pngs for a specific platform into the repo. if this flow can be incorporated into Github actions as well, then bonus points!

@frigus02
Copy link
Author

i'm thinking instead of stringing together a bunch of heavy js libs (jsdom, canvas, canvas-5-polyfill), maybe running this via Puppeteer as one easily-updated dependency could make debugging easier, since we can opt in/out of headless and play with devtools in an actual browser instance.

I considered Puppeteer as well and thought not starting a full browser would make tests faster. There are however advantages with running in a real browser, so I understand what you're saying.

honestly, this is a pretty crappy feedback loop.

Running this on push is obviously optional. npm test runs the tests against committed snapshots.

[...] checks out the known-good commit and creates the pngs. [...]

That's a nice idea. I does mean that the first test run would be slower because it has to build up the cache. But it doesn't require the PNGs to be committed.

Overall it seems like this is not quite what you're looking for, which is completely understandable. I'll see if I can fine the muse to implement your suggested changes. If not, your ideas and this PR can serve as a starting point for the next person 馃檪.

@leeoniya
Copy link
Owner

another wild idea is to ditch the dom and canvas entirely and replace them with thin mocks which act as simple command buffers. then compare whats in those buffers to known references. if a mismatch happens, then the good and bad buffers can be re-played in a real browser for comparison. this would take some effort to set up but would be platform agnostic, zero dependency and run very fast, plus can be committed into the repo.

i have never worked with Proxies, but mocking/command buffers feels like a great use case to make a general testing approch like this work. i haven't seen a testing framework use this strategy. could be a good tangent project!

Overall it seems like this is not quite what you're looking for, which is completely understandable.

i never fully understood why contributers pick up and work on open issues without attempting to ping me beforehand to discuss the impl specifics before doing all the work, hehe :). but as long as you got value out of it, that's great.

I'll see if I can fine the muse to implement your suggested changes. If not, your ideas and this PR can serve as a starting point for the next person slightly :)

yeah, for sure. as you can tell i'm still tossing around different ideas, as well. i'll leace this pr open for now.

@frigus02
Copy link
Author

Using proxies (or some other way of mocking DOM and canvas) sounds like an interesting idea.

Yeah, I did this mainly for myself to be honest. If this would have fit your needs, it would have been a nice opportunity to contribute as well.

@leeoniya
Copy link
Owner

leeoniya commented Apr 10, 2020

i currently use https://github.com/developit/undom to test https://github.com/domvm/domvm with good success, and domvm needs a lot more dom than uPlot. so this is another option, but uPlot does not need to read back or traverse the dom, so i think something like a command buffer would work, too, for tests.

i very well might cherry pick some of this PR, lots of good stuff in here :) i'll likely use this as a springboard to get into basic CI.

@milahu
Copy link

milahu commented Dec 20, 2020

another wild idea is to ditch the dom and canvas entirely and replace them with thin mocks which act as simple command buffers. then compare whats in those buffers to known references. if a mismatch happens, then the good and bad buffers can be re-played in a real browser for comparison.

thats exactly what jest-canvas-mock does : )

i started adding this to uPlot, but i'm stuck at ....

git clone --depth 1 https://github.com/milahu/uPlot.git --branch add-test-snapshot-replay
cd uPlot/test/replay-tests
npm install
npm run start # will start app in browser

obvious problem: only a small part is rendered to canvas
i only see some grid lines now and then
canvas-clearing silently fails

possible reasons:

  • some problem between uPlot and jsdom (used internally by jest, maybe an old version)
  • not all draw calls are recorded by jest-canvas-mock (async? timeout?)
  • draw calls are recorded with wrong parameters
    for example ctx.fillText("context test", 40, 40, null) has no effect
    (with maxWidth = null, as recorded by jest)
    but ctx.fillText("context test", 40, 40) works fine
    jest-canvas-mock was only made to compare snapsnots, not to replay the API calls
    (note to self: in uPlot, hide canvas behind a catch-all-proxy-object and record outgoing API calls)
  • whats also weird: data from Path2D objects contain the beginPath method
    which only exists as ctx.beginPath but not as path2d.beginPath
    this is a bug in jest-canvas-mock

@leeoniya you're the canvas expert here, do you have an idea .. ?

@leeoniya
Copy link
Owner

thanks for trying this out, @milahu -- only the brave venture into this issue!

here's another similar idea that might work better (canvas -> svg pathstring api facade): grafana/grafana#29552. in the end, though, i suspect we'll end up with using reference images, testing with a real browser (playwright or puppeteer) + pixelmatch due to incomplete or broken mocks :(. i think the tests should still run quickly as long as we don't have to spawn a browser instance for each test. we just need to make sure we can cleanup up after each run and re-use the existing process for the next test.

i don't have too much time to look into this now & debug, unfortunately, but if you get something working i'll certainly take a look.

@milahu
Copy link

milahu commented Dec 21, 2020

canvas -> svg pathstring

naah .. my canvas solution is almost ready : )

i had an error in my "glue code" to import the canvas events from jest-canvas-mock

jest-canvas-mock uses different property names (for example width instead of w)
than the typescript interface in typescript/lib/lib.dom.d.ts
which i use to auto-generate my glue code

my code was too simple/naive, so i had these mysterious silent fails,
cos canvas silently accepted these undefined parameters

in the end, though, i suspect we'll end up with using reference images, testing with a real browser (playwright or puppeteer) + pixelmatch due to incomplete or broken mocks :(

we can still have that, but puppeteer / playwright is huuge overkill
these install their own full browser (~200MB) which is just unnecessary
a more lightweight solution is testcafe

to only generate images from canvas, we have node-canvas
including pngStream() and pdfStream() (yay vector graphs!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants