search

quil / quil

Main repo. Quil source code.
Eclipse Public License 1.0
2.96k stars 164 forks source link

Add image unit tests #307

Open nbeloglazov opened 5 years ago

nbeloglazov commented 5 years ago

The goal is to add a set of tests that run all snippets, generate image and compare image with a reference image. It should support clj and cljs.

Some discussion started in https://github.com/quil/quil/pull/304

nbeloglazov commented 5 years ago

@anthonygalea I added :skip-image-diff option to snippets instead of hardcoded list in https://github.com/quil/quil/commit/ab0a8ffb1bcf7d2e9721f47aaa8c3c0cfe87ed1f and improved difference image a little bit in https://github.com/quil/quil/commit/924cf93121ba8623abcef6cd58f26bb18df1718c. Also added some info about running image tests in https://github.com/quil/quil/wiki/Dev-notes at the end, please add more things if I missed anything.

But I got into an issue with cljs image tests. Looks like when you generated reference images you used mac with retina display and all images are 1000x1000. When I run it on my machine (linux) I'm getting 500x500 and comparison obviously fails. Any ideas how to address it? One thing we could do is have different sets of images. LIke "firefox-on-macos-retina", "firefox-on-linux", etc. But I'm not sure how to detect whether current screen is retina or not from the clj test code.

We could also have some smart comparison logic that can resize image if it detects they are 2x size. But I worry that this resizing will cause diffs that exceed threshold unless our diff algorithm is very smart.

What do you think?

anthonygalea commented 5 years ago

Great @nbeloglazov. Thanks for updating dev notes, I don't see anything missing.

Regarding detecting retina in clj can't we use q/display-density? See displayDensity. I think I would go with different sets of images as you suggest. It's just that the combinations go up pretty quick:

This would be 3 x 4 x 2 x 2 = 48 combinations. Maybe we just do the following 4 for now?

anthonygalea commented 5 years ago

Ah ok, just tried adding it and I see the issue you have with using q/display-density. What do you think about https://github.com/anthonygalea/quil/commit/e34927e922df4906df1ee214a1e7aede6998496b?