Conversation
Garbee
reviewed
Sep 17, 2025
| if (messages.includes('axe-loaded')) { | ||
| resolve(); | ||
| } else { | ||
| window.addEventListener('message', function (msg) { |
Member
There was a problem hiding this comment.
Minor thing and since this in our tests I don't think it'll matter too much. But should we not remove this listener after it has resolved? That way there are no extraneous listeners during test execution. And we can do this through a signal to cancel listening instead of doing a named function then removeEventListener. Bit cleaner these days.
Contributor
Author
There was a problem hiding this comment.
Nah, this page runs in isolation so can't affect other pages and is closed once it's run so nothing will remain in memory or anything
dbjorge
approved these changes
Sep 17, 2025
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
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 test in firefox started to fail all of a sudden, and almost consistently. I couldn't figure out why the test kept failing, so after playing around with the order of operations I figured out that the iframe is loading axe and calling the
axe-loadedevent before mocha runs thebeforewhere we look for the event. So to fix it I changed up the loading order so the script runs first and adds the event tracking, and now the event tracking now happens outside thebeforefunction and keeps track of the logs. Then inside thebeforewe see if the message has already been fired, otherwise we listen to the event as we were doing before.