Closed Bug 1499096 Opened 11 months ago Closed 11 months ago

Update tests using ok() with 3 or more arguments to use is() when possible

Categories

(Firefox Build System :: Lint and Formatting, enhancement)

enhancement
Not set

Tracking

(firefox65 fixed)

RESOLVED FIXED
mozilla65
Tracking Status
firefox65 --- fixed

People

(Reporter: jdescottes, Assigned: jdescottes)

References

(Blocks 6 open bugs)

Details

Attachments

(4 files)

Before updating our test helpers to throw when calling ok() with more than 2 arguments, I would like to land a first cleanup that will switch most of the wrong usage of ok() to is() or to other forms of ok().

Normally they should all pass but I want to land this separately and as early as possible in case some of them might be intermittent.
This changeset updates all the test that were wrongly using ok() and wanted to
use is() AND for which the assert is still passing without any modification
required.
Depends on D8739.
This changeset updates calls to ok() that were most likely intended 
for is(), but are not working as is.
Depends on D8740.
This changeset replaces calls to ok with 3 arguments to calls with 2 arguments
in situations where the switch does not have a significant impact on the assert.
Depends on D8741
This changeset updates some calls to ok() that should actually be calls to is()
and that needed tiny fixes to match the expected value.
Summary: Add an eslint rule to check the usage of browser-test ok() → Update tests using ok() with 3 or more arguments to use is() when possible
Blocks: 1500913
Blocks: 1500959
Blocks: 1500960
Blocks: 1500961
Blocks: 1500962
Blocks: 1500964
Blocks: 1500965
Blocks: 1500966
Blocks: 1500967
Blocks: 1500971
Thank you for filing all the bugs.
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/eee0effaa1f0
Update tests using ok() to is();r=Standard8
https://hg.mozilla.org/integration/autoland/rev/a82f6a836cf3
Update wrong usage of ok() with todo_is();r=Standard8
https://hg.mozilla.org/integration/autoland/rev/dfaaf3978e3c
Use ok() with 2 arguments instead of 3 when possible;r=Standard8
https://hg.mozilla.org/integration/autoland/rev/2e4bdafb70ce
Update tests using ok() to is(), with minor fixes;r=Standard8
Hi Julian,

Considering all the implementation bugs you logged as blocking this one: Is the General component the correct one? If not, can you tell me which one would be correct so we can modify it accordingly? This is related to bug triage. Thank you.
Flags: needinfo?(jdescottes)
If we have an general component related to tests we could move them there. Otherwise I can try to triage them individually in the components responsible for the test file impacted by each bug. Let me know what you prefer.
Flags: needinfo?(jdescottes) → needinfo?(daniel.bodea)
It appears that the component specific for automated tests is (Testing) Marionette. I will mark them as such. Thanks.
Flags: needinfo?(daniel.bodea)
(In reply to Bodea Daniel [:danibodea] from comment #10)
> It appears that the component specific for automated tests is (Testing)
> Marionette. I will mark them as such. Thanks.

Testing:Marionette is for items related to the marionette test suite.

They should be located according to `./mach file-info bugzilla-component <file>`
Flags: needinfo?(daniel.bodea)
I am sorry for the misunderstanding. Would it be OK if I left them on the General component, but for the "Testing" product?
Flags: needinfo?(daniel.bodea)
Don't worry, I can put them into the correct places. It is better if they are in the right components as the triage owners can set the priorities correctly as a result.
Yes, that is good. Thank you.
(In reply to Mark Banner (:standard8) from comment #13)
> Don't worry, I can put them into the correct places. It is better if they
> are in the right components as the triage owners can set the priorities
> correctly as a result.

You also want to make sure to file them as normal bugs rather than enhancement requests, as the latter aren't necessarily triaged in the same way.
Depends on: 1501644
No longer depends on: 1501644
Blocks: 1569235
No longer blocks: 1569235
You need to log in before you can comment on or make changes to this bug.