Closed Bug 1276490 Opened 3 years ago Closed 3 years ago

performance-timeline/po-resource.html has a risk of intermittent failures

Categories

(Testing :: web-platform-tests, defect)

defect
Not set

Tracking

(firefox49 fixed)

RESOLVED FIXED
mozilla49
Tracking Status
firefox49 --- fixed

People

(Reporter: hiro, Unassigned)

References

Details

Attachments

(1 file)

po-resource.html has never been run in our automation tests because PerformanceObserver is not yet enabled by default.  It will be enabled by bug 1271487.

A try[1] run with enabling PerformanceObserver made me realize that there is a risk of intermittent failures in po-resource.html

[1] https://treeherder.mozilla.org/logviewer.html#?job_id=21421337&repo=try#L15163

There is an obvious anti-pattern[2] to cause intermittent failures.  We should fix it.

https://dxr.mozilla.org/mozilla-central/rev/4d63dde701b47b8661ab7990f197b6b60e543839/testing/web-platform/tests/performance-timeline/po-resource.html#21
Hi, Joel.  Could you please take a look this?
I am not sure you are a proper person for reviewing this web platform test, but I know you have tremendous amount of knowledge of automation test field.
 
A try run with this patch:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=657984a2903a

Thank you,
Comment on attachment 8757674 [details]
MozReview Request: Bug 1276490 - Do not use Math.random() to create unique values to avoid intermittent failrues. r?jgraham

https://reviewboard.mozilla.org/r/56084/#review52734

I see no problem with this- although img2 was theoretically 10x the value of img1 and now they will roughly be the same magnitude.

I am going to ask jgraham to review this- I do not know if this needs to be done upstream as well.
Attachment #8757674 - Flags: review?(jmaher)
I guess this is OK, but I think it's unnecessary; in practice we run tests in a clean profile, so the only chance of a collision is from the two random numbers being identical in a single run. The probability of that is sufficiently small that it doesn't matter.

But if you want to land this, please fix the typo in the commit message.
Comment on attachment 8757674 [details]
MozReview Request: Bug 1276490 - Do not use Math.random() to create unique values to avoid intermittent failrues. r?jgraham

https://reviewboard.mozilla.org/r/56084/#review52986

Typo in the commit message.
Attachment #8757674 - Flags: review?(james) → review+
Thank you, jgraham.  Right, the collision must be rare, but it actually happened the first try.  I guess it's because I like oranges. ;-)
Comment on attachment 8757674 [details]
MozReview Request: Bug 1276490 - Do not use Math.random() to create unique values to avoid intermittent failrues. r?jgraham

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/56084/diff/1-2/
Attachment #8757674 - Attachment description: MozReview Request: Bug 1276490 - Do not use Math.ranrom() to create unique values to avoid intermittent failrues. r?jmaher → MozReview Request: Bug 1276490 - Do not use Math.random() to create unique values to avoid intermittent failrues. r?jgraham
https://hg.mozilla.org/mozilla-central/rev/da391c890d07
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in before you can comment on or make changes to this bug.