-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Adding Visual Test Report in Github Actions #7653
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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev-2.0
Are you sure you want to change the base?
Changes from all commits
b10edd3
8679a7a
4a674e3
a229893
26d2676
c584788
d070e0c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -441,9 +441,15 @@ export function visualTest( | |
: []; | ||
|
||
for (let i = 0; i < actual.length; i++) { | ||
const flatName = name.replace(/\//g, '-'); | ||
const actualFilename = `../actual-screenshots/${flatName}-${i.toString().padStart(3, '0')}.png`; | ||
if (expected[i]) { | ||
const result = await checkMatch(actual[i], expected[i], myp5); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just an idea: Could the missing actual screenshots be potentially explained by some surprising behavior here? Ie, if you move There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you mean moving it out of the if condition... am I right? |
||
// Always save the actual image before potentially throwing an error | ||
writeImageFile(actualFilename, toBase64(actual[i])); | ||
if (!result.ok) { | ||
const diffFilename = `../actual-screenshots/${flatName}-${i.toString().padStart(3, '0')}-diff.png`; | ||
writeImageFile(diffFilename, toBase64(result.diff)); | ||
throw new Error( | ||
`Screenshots do not match! Expected:\n${toBase64(expected[i])}\n\nReceived:\n${toBase64(actual[i])}\n\nDiff:\n${toBase64(result.diff)}\n\n` + | ||
'If this is unexpected, paste these URLs into your browser to inspect them.\n\n' + | ||
|
@@ -452,6 +458,7 @@ export function visualTest( | |
} | ||
} else { | ||
writeImageFile(expectedFilenames[i], toBase64(actual[i])); | ||
writeImageFile(actualFilename, toBase64(actual[i])); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also minor: this looks like it might be an accidental duplicate since it's also on line 449 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this was written because earlier the command was written in both if and else conditions. Now, I've moved |
||
} | ||
} | ||
}); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor, didn't notice it before: for consistency with existing
__screenshots__
could you renameactual-screnshots
to something like__screenshots_actual
, please?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, the
__screenshots__
here are not of the visual tests as they are not in the .gitignore file and are present in the repo.Though I will change the name of the folder to screenshots_actual if you wish.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No you're right it's not comparable, no worries, please disregard