-
Notifications
You must be signed in to change notification settings - Fork 10.2k
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
[test/driver.js] Ensure that Image src
is set *after* the callbacks in resolveImages
#13675
[test/driver.js] Ensure that Image src
is set *after* the callbacks in resolveImages
#13675
Conversation
… in `resolveImages` *While I cannot guarantee that this will fix the recent intermittents, this patch really shouldn't hurt.* By setting the Image `src` first, there's a small possibility that the Image is loaded *before* we've had a change to attach the `onload`/`onerror` callbacks which may cause the Promise to remain in a pending state. Note that prior to PR 13641 we didn't correctly await all image resources to actually load, which could explain the very recent intermittent test-failures.
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.
I don't know either if it'll fix the issues but it's definitely an interesting find
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 1 Live output at: http://54.67.70.0:8877/7d3c2ce66de405f/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://3.101.106.178:8877/dd684a5d775e961/output.txt |
From: Bot.io (Windows)FailedFull output at http://3.101.106.178:8877/dd684a5d775e961/output.txt Total script time: 34.84 mins
Image differences available at: http://3.101.106.178:8877/dd684a5d775e961/reftest-analyzer.html#web=eq.log |
From: Bot.io (Linux m4)FailedFull output at http://54.67.70.0:8877/7d3c2ce66de405f/output.txt Total script time: 24.05 mins
Image differences available at: http://54.67.70.0:8877/7d3c2ce66de405f/reftest-analyzer.html#web=eq.log |
Apparently this didn't help :-( |
I'd say landing it doesn't hurt and might be useful, so... let's land it? |
While I cannot guarantee that this will fix the recent intermittents, this patch really shouldn't hurt.
By setting the Image
src
first, there's a small possibility that the Image is loaded before we've had a change to attach theonload
/onerror
callbacks which may cause the Promise to remain in a pending state.Note that prior to PR #13641 we didn't correctly await all image resources to actually load, which could explain the very recent intermittent test-failures.