-
Notifications
You must be signed in to change notification settings - Fork 3.2k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Fix flakiness in timeOrigin test #6759
Fix flakiness in timeOrigin test #6759
Conversation
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.
Already reviewed downstream.
@jgraham do you think this is a reasonable way to tackle the flakiness of the tests or should we just remove the potentially flaky asserts? I was told we should assume that tests could be interrupted for an arbitrary amount of time. While this would be nice to have, I don't think it's something we can afford at this point because it limits our testing abilities. In fact, there are already tests that work under the assumption that the test runs uninterrupted, for example https://github.com/w3c/web-platform-tests/blob/master/hr-time/window-worker-time-origin.html |
I mean all I can do is try it and see what happens. Testing that things are close in time is particularly difficult especially given shared VMs in which we can just lose the timeslice for a few hundred ms. |
This CL uses now() calls instead of arbitrary numbers to prevent flakiness in this test when ran on slower machines or when the test is interrupted by debugging. Bug: chromium:752574 Change-Id: I9b3610a44ca5199b8a906dbe9715d280bd3439c5 Reviewed-on: https://chromium-review.googlesource.com/602381 Cr-Commit-Position: refs/heads/master@{#492112} WPT-Export-Revision: 944cf9134ec1b2fa192de14f43ecc4790e90295b
c29ff24
to
e5beec4
Compare
@bobholt, Firefox is failing, but no results shown for any browser. Are we mid-transition to the PR results web app? |
Build PENDINGStarted: None Unstable ResultsBrowser: "Firefox Nightly"
|
Build PENDINGStarted: None Unstable ResultsBrowser: "Firefox Nightly"
|
Firefox (nightly)Testing web-platform-tests at revision d8ec8c5 Unstable results
All results1 test ran/hr-time/timeOrigin.html
|
Sauce (safari)Testing web-platform-tests at revision 931f1f3 All results1 test ran/hr-time/timeOrigin.html
|
Chrome (unstable)Testing web-platform-tests at revision d8ec8c5 All results1 test ran/hr-time/timeOrigin.html
|
Sauce (MicrosoftEdge)Testing web-platform-tests at revision 931f1f3 All results1 test ran/hr-time/timeOrigin.html
|
@bobholt I ran this test locally and with the travis CI without this PR and found it was not flaky. |
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.
Leaving negative review so that this isn't somehow automatically merged before #6822. (The "do not merge yet" label would get automatically removed.)
#6822 is merged, closing this. |
Fix flakiness in timeOrigin test
This CL uses now() calls instead of arbitrary numbers to prevent
flakiness in this test when ran on slower machines or when the test
is interrupted by debugging.
Bug: chromium:752574
Change-Id: I9b3610a44ca5199b8a906dbe9715d280bd3439c5
Reviewed-on: https://chromium-review.googlesource.com/602381
Cr-Commit-Position: refs/heads/master@{#492112}
WPT-Export-Revision: 944cf9134ec1b2fa192de14f43ecc4790e90295b