Skip to content

Conversation

straker
Copy link
Contributor

@straker straker commented Sep 16, 2025

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-loaded event before mocha runs the before where 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 the before function and keeps track of the logs. Then inside the before we see if the message has already been fired, otherwise we listen to the event as we were doing before.

@straker straker requested a review from a team as a code owner September 16, 2025 21:41
if (messages.includes('axe-loaded')) {
resolve();
} else {
window.addEventListener('message', function (msg) {
Copy link
Member

@Garbee Garbee Sep 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@straker straker merged commit 26c4892 into develop Sep 17, 2025
24 checks passed
@straker straker deleted the fix-isolated-env branch September 17, 2025 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants