Open Bug 1333683 Opened 4 years ago Updated 2 years ago

Add unit test for the ColorWidget

Categories

(DevTools :: Inspector: Rules, defect, P3)

defect

Tracking

(Not tracked)

People

(Reporter: gl, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 3 obsolete files)

In bug 1332049, we added a new working copy (ColorWidget.js) of the existing color picker (Spectrum.js). This bug is to add a new working copy of the existing spectrum unit test for the ColorWidget.js
Attached patch 1333683.patch [1.0] (obsolete) — Splinter Review
Attachment #8830163 - Flags: review?(jdescottes)
Attached patch 1333683.patch [2.0] (obsolete) — Splinter Review
Attachment #8830163 - Attachment is obsolete: true
Attachment #8830163 - Flags: review?(jdescottes)
Attachment #8830164 - Flags: review?(jdescottes)
Attached patch 1333683.patch [3.0] (obsolete) — Splinter Review
Attachment #8830164 - Attachment is obsolete: true
Attachment #8830164 - Flags: review?(jdescottes)
Attachment #8830166 - Flags: review?(jdescottes)
Comment on attachment 8830166 [details] [diff] [review]
1333683.patch [3.0]

Review of attachment 8830166 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, thanks.

Two minor nits on the test name:  
- the -01 suffix may cause issues later (when we have to split tests, delete tests etc...)
  If we don't have any good reason to do so I'd prefer to stick to descriptive names.
  browser_colorWidget.js is ok here as it's our only test for this widget at the moment.
- we rarely use camel case for test names
Attachment #8830166 - Flags: review?(jdescottes) → review+
Attachment #8830166 - Attachment is obsolete: true
Attachment #8830468 - Flags: review+
Pushed by gabriel.luong@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dd1efa762e49
Add unit test for the ColorWidget. r=jdescottes
For info, the failure was on Linux x64 pgo only.
Backout by kwierso@gmail.com:
https://hg.mozilla.org/mozilla-central/rev/52a34f9a6cf1
Backed out changeset dd1efa762e49 for failing the browser_color_widget test a=backout
Flags: needinfo?(gl)
As discussed, the test was copied from browser_spectrum.js which seems to have similar intermittent failures: https://bugzilla.mozilla.org/show_bug.cgi?id=1286162

Did a few retriggers on https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=dd1efa762e490f5d46aecca694150f993ab458b6 to see if the new failure is also an intermittent.
So the new failure looks intermittent as well, but with a 60% failure rate. Which is probably higher than what we have for the copied test.
Product: Firefox → DevTools
Assignee: gl → nobody
Status: ASSIGNED → NEW
You need to log in before you can comment on or make changes to this bug.