[dev-2.0] visual tests for Type #7429
Conversation
|
🎉 Thanks for opening this pull request! Please check out our contributing guidelines if you haven't already. And be sure to add yourself to the list of contributors on the readme page! |
There was a problem hiding this comment.
Thanks for adding more tests! Before merging:
- The existing screenshots for other visual tests have been removed. Could you restore them by calling
git checkout dev-2.0 -- test/unit/screenshots? - It looks like the new tests don't yet have screenshots. Running
npm run testlocally will generate new screenshots for tests that don't have any yet. Could I get you to do that and then commit the results?
test/unit/visual/cases/type.js
Outdated
| @@ -18,100 +18,10 @@ visualSuite("Typography", function () { | |||
| p5.text("test", 0, 0); | |||
| screenshot(); | |||
| }); | |||
|
|
|||
| visualTest('with a Google Font URL', async function(p5, screenshot) { | |||
There was a problem hiding this comment.
It looks like this deleted some earlier tests. Would you be able to restore these?
test/unit/visual/cases/type.js
Outdated
| @@ -1,6 +1,6 @@ | |||
| import { visualSuite, visualTest } from "../visualTest"; | |||
|
|
|||
| visualSuite("Typography", function () { | |||
There was a problem hiding this comment.
Maybe let's keep the title "Typograhy" for now so we won't have to move all the existing screenshots to a new folder
|
Thank you @davepagurek! Have updated the Typography tests along with all the tests' screenshots |
| ]; | ||
|
|
||
| p5.createCanvas(300, 80); | ||
| p5.textSize(60); | ||
| p5.createCanvas(300, 200); |
There was a problem hiding this comment.
For speed, each screenshot is resized to fit in 50x50 before comparison. Since this image (and the alignment test too) is so large compared to that, differences in antialiasing between devices could cause this test to fail.
You can call screenshot() multiple times per test and it will generate a separate image each time. Maybe we can make the canvas size smaller for these, and instead of placing each case somewhere else in one canvas, we can clear the canvas for each case and screenshot() them before moving on to the next?
There was a problem hiding this comment.
For added context, the test failure on CI right now is saying it expected this:
but got this:
Visual tests for typography module
Changes:
Screenshots of the change:
PR Checklist
npm run lintpasses