-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Add mechanism for dashboard snapshots #15463
Add mechanism for dashboard snapshots #15463
Conversation
Failing on the rendering issues. waitForAllSharedItemsToFinishRendering is not working correctly, though it may be because there are currently issues with the render-counter. |
e1dd3ba
to
55cd94a
Compare
I think these comparisons are the closest we'll get with OS and browser differences: This equates to a difference percentage of 0.009916455696202532 so I'm going to use a threshold of .05 for the area test. We will probably want to set each threshold for each test manually. The other option is to explore going back to os/browser difference specific snapshots. |
17997cf
to
a91cc98
Compare
Well, looks like the render counter stuff is probably not working properly as the screenshot came out blank. I'll have to investigate some more before it's ready for review again. :( |
What do you know!! It's not a mistake, it actually caught a legit bug! Woooooooot. |
a91cc98
to
12d916a
Compare
Should be obsolete when the new render stuff is complete.
…screen resolutions
… when comparing across browser/os
Update screenshot for new window dimensions
There is a timezone bug with tsvb: elastic#15501
As it turns out, the visualization was not done re-rendering to maximized mode
12d916a
to
7b13cde
Compare
And back to passing now that the expanded panel bug is fixed. Ready for review! |
src/functional_test_runner/cli.js
Outdated
@@ -28,6 +29,12 @@ if (cmd.verbose) logLevel = 'verbose'; | |||
const log = createToolingLog(logLevel); | |||
log.pipe(process.stdout); | |||
|
|||
if (cmd.updateBaselines) { |
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.
Determining whether this test-run should update the snapshots via an environment variable seems rather hacky, is there a way to pass this to the tests via the config
/configOverrides
in the createFunctionalTestRunner
; or a way to run dedicated tests within the apps perhaps by hi-jacking the grep
extension in mocha?
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.
Ah yes, I meant to revisit that. Found an alternative solution without using environment variables.
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.
LGTM
Fingers crossed this doesn't create flakiness and I tested it enough! 🤞 |
* Add mechanism for dashboard snapshots * Adjust wait for render function since it needs to be 2, not gt 0. Should be obsolete when the new render stuff is complete. * resize images using new library so comparisons work across different screen resolutions * use jimp comparison and see if expanding to expanded panel mode helps when comparing across browser/os * Try to ensure window size * Experiment with a smaller window, see if screenshot dimensions change Update screenshot for new window dimensions * Try cover + quality, see what the diffs look like. * Stop trying to get TSVB to pass, try area charts There is a timezone bug with tsvb: elastic#15501 * gah, cover didn't work, check resize * bump render counter to 6, as it should be. As it turns out, the visualization was not done re-rendering to maximized mode * Bump threshold for comparison * reduce down to a single test run * Don't use an environment variable to detect updateBaselines cmd line flag
* Add mechanism for dashboard snapshots * Adjust wait for render function since it needs to be 2, not gt 0. Should be obsolete when the new render stuff is complete. * resize images using new library so comparisons work across different screen resolutions * use jimp comparison and see if expanding to expanded panel mode helps when comparing across browser/os * Try to ensure window size * Experiment with a smaller window, see if screenshot dimensions change Update screenshot for new window dimensions * Try cover + quality, see what the diffs look like. * Stop trying to get TSVB to pass, try area charts There is a timezone bug with tsvb: #15501 * gah, cover didn't work, check resize * bump render counter to 6, as it should be. As it turns out, the visualization was not done re-rendering to maximized mode * Bump threshold for comparison * reduce down to a single test run * Don't use an environment variable to detect updateBaselines cmd line flag
* Add mechanism for dashboard snapshots * Adjust wait for render function since it needs to be 2, not gt 0. Should be obsolete when the new render stuff is complete. * resize images using new library so comparisons work across different screen resolutions * use jimp comparison and see if expanding to expanded panel mode helps when comparing across browser/os * Try to ensure window size * Experiment with a smaller window, see if screenshot dimensions change Update screenshot for new window dimensions * Try cover + quality, see what the diffs look like. * Stop trying to get TSVB to pass, try area charts There is a timezone bug with tsvb: elastic#15501 * gah, cover didn't work, check resize * bump render counter to 6, as it should be. As it turns out, the visualization was not done re-rendering to maximized mode * Bump threshold for comparison * reduce down to a single test run * Don't use an environment variable to detect updateBaselines cmd line flag
Resolves #15458
If the test fails due to an expected change, you simply have to run the tests locally with the command line flag
--updateBaselines
which will generate a new screenshot to be uploaded with your PR (similar to -u with the jest snapshots).