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

Status: RESOLVED FIXED

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

Description

[Julian Descottes [:jdescottes]]

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.

Comment 1

[Julian Descottes [:jdescottes]]

Attachment: Bug 1499096 - Update tests using ok() to is();r=Standard8

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.

Comment 2

[Julian Descottes [:jdescottes]]

Attachment: Bug 1499096 - Update wrong usage of ok() with todo_is();r=Standard8

Depends on D8739.
This changeset updates calls to ok() that were most likely intended 
for is(), but are not working as is.

Comment 3

[Julian Descottes [:jdescottes]]

Attachment: Bug 1499096 - Use ok() with 2 arguments instead of 3 when possible;r=Standard8

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.

Comment 4

[Julian Descottes [:jdescottes]]

Attachment: Bug 1499096 - Update tests using ok() to is(), with minor fixes;r=Standard8

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.

Comment 5

[Julian Descottes [:jdescottes]]

thanks for the reviews. Pushed to try at https://treeherder.mozilla.org/#/jobs?repo=try&revision=d7f78428027bb457794b71051bf4cd4ff79ce3b1

Comment 6

[Mark Banner (:standard8)]

Thank you for filing all the bugs.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
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

Comment 8

[Bodea Daniel [:danibodea]]

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.

Comment 9

[Julian Descottes [: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.

Comment 10

[Bodea Daniel [:danibodea]]

It appears that the component specific for automated tests is (Testing) Marionette. I will mark them as such. Thanks.

Comment 11

[Mark Banner (:standard8)]

(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>`

Comment 12

[Bodea Daniel [:danibodea]]

I am sorry for the misunderstanding. Would it be OK if I left them on the General component, but for the "Testing" product?

Comment 13

[Mark Banner (:standard8)]

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.

Comment 14

[Bodea Daniel [:danibodea]]

Yes, that is good. Thank you.

Comment 15

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/eee0effaa1f0
https://hg.mozilla.org/mozilla-central/rev/a82f6a836cf3
https://hg.mozilla.org/mozilla-central/rev/dfaaf3978e3c
https://hg.mozilla.org/mozilla-central/rev/2e4bdafb70ce

Comment 16

[Dão Gottwald [::dao]]

(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.