test: fix flakiness in rerun integration test#4598
Merged
Conversation
straker
reviewed
Oct 4, 2024
| }, | ||
| "is-element-focusable": { | ||
| "pass": "Элемент может быть сфокусирован.", | ||
| "pass": "Элемент может быть сфокусирован.", |
Contributor
There was a problem hiding this comment.
I think you included this file by accident.
Contributor
Author
There was a problem hiding this comment.
The autoformatter pulled it in on its own
straker
previously requested changes
Oct 4, 2024
Contributor
straker
left a comment
There was a problem hiding this comment.
Need to just remove the locale file and should be good to approve
Comment on lines
+717
to
+719
| // testEnvironment is ignored by default because Chrome's UI animations for the | ||
| // "an automated test is controlling this browser" notification can cause | ||
| // inconsistencies in windowHeight for otherwise-identical scans. |
Member
There was a problem hiding this comment.
Thank you for adding this comment. I didn't understand why we were ignoring it before 👍
Merged develop to address extra localization file being pulled in
WilcoFiers
approved these changes
Oct 7, 2024
straker
pushed a commit
that referenced
this pull request
Oct 16, 2024
This addresses some test flakiness in `/test/integration/rerun/rerun.js` which has caused ~4 e2e test failures in CI builds in the last 3 weeks. It manifests as a timeout because the test uses assertions within callbacks to `axe.run` without propogating errors to the `done` callback; this addresses that similarly to other tests using the run callback pattern, which should cause new errors in this test to report the actual assertion failure instead of just timing out. I wasn't able to locally repro the actual failure, but I think a good guess as to why it's failing flakily is the same issue we saw with other tests doing deep comparison of axe result objects (testEnvironment differing between nearby scans due to windowHeight differences). I addressed that by reusing the test util we added for doing result comparison. I didn't want to hardcode knowledge of the testEnvironment thing into yet another individual test case, so I also refactored the test utility to move that knowledge into one location with a comment explaining it. --------- Co-authored-by: dbjorge <dbjorge@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This addresses some test flakiness in
/test/integration/rerun/rerun.jswhich has caused ~4 e2e test failures in CI builds in the last 3 weeks. It manifests as a timeout because the test uses assertions within callbacks toaxe.runwithout propogating errors to thedonecallback; this addresses that similarly to other tests using the run callback pattern, which should cause new errors in this test to report the actual assertion failure instead of just timing out.I wasn't able to locally repro the actual failure, but I think a good guess as to why it's failing flakily is the same issue we saw with other tests doing deep comparison of axe result objects (testEnvironment differing between nearby scans due to windowHeight differences). I addressed that by reusing the test util we added for doing result comparison.
I didn't want to hardcode knowledge of the testEnvironment thing into yet another individual test case, so I also refactored the test utility to move that knowledge into one location with a comment explaining it.