Add linting to prevent landing .only() debugging helper in tests
Categories
(Developer Infrastructure :: Source Code Analysis, enhancement)
Tracking
(firefox95 fixed)
| Tracking | Status | |
|---|---|---|
| firefox95 | --- | fixed |
People
(Reporter: nhnt11, Assigned: nhnt11)
References
(Depends on 1 open bug)
Details
Attachments
(1 file)
When tinkering with test files that contain many tasks, I often use .only() to speed up my workflow - to run a specific task without waiting for all the other ones. Recently I inadvertently landed such a debugging addition in bug 1714182, and only caught it when I touched the same code later for a follow-up. This means we had about 2 weeks of running only a single task of this test file (which contains many tasks) in automation.
I think this might be another case currently in m-c of the same mistake: https://searchfox.org/mozilla-central/rev/50c3cf7a3c931409b54efa009795b69c19383541/toolkit/components/normandy/test/browser/browser_PreferenceExperiments.js#2196 (maybe it is intentional, I could be wrong).
We could probably prevent this easily with a lint rule. I think it would be fair to require an explicit exception declaration in cases of intentional usage.
|
Assignee | |
Comment 1•3 years ago
|
||
Forgot to say in comment 0 that this might also apply to e.g. .skip(), hence the bug title.
|
|
Assignee | |
Updated•3 years ago
|
|
|
Assignee | |
Comment 2•3 years ago
|
||
|
||
Updated•3 years ago
|
|
|
Assignee | |
Comment 3•3 years ago
|
||
This mistake happened again recently [1][2], so I made a patch.
[1] commit adding mistake: https://hg.mozilla.org/mozilla-central/rev/6ee09543a37fea2d2964dafe8aa45debddc31d29#l1.57
[2] bug and commit removing mistake: bug 1734856
|
|
||
Updated•3 years ago
|
|
|
Assignee | |
Comment 4•3 years ago
|
||
Reducing the scope of this bug to cover only .only() since there seem to be several purposeful uses of skip() in tests to conditionally skip specific tasks. Inadvertently landing .only() seems far more problematic as well since we would suppress many tests.
Pushed by nhnt11@gmail.com: https://hg.mozilla.org/integration/autoland/rev/6eb94ab5d468 Reject .only() chained onto add_task in tests. r=Gijs,mythmon,Standard8
|
||
Comment 6•3 years ago
|
||
| bugherder | ||
|
||
Updated•2 years ago
|
Description
•