1753696 - Use an eslint rule to transform existing setup tasks to use add_setup

Status: RESOLVED FIXED

(Testing :: Mochitest, task, P3)

Description

[:Gijs (he/him)]

bug 1720335 adds add_setup. Now let's switch existing tests to it, so that .skip and .only will start working correctly in more cases, and we can experiment with having other tools like pref environments work correctly depending on the context in which they're invoked.

Comment 1

[BugBot [:suhaib / :marco]]

The severity field is not set for this bug.
:ahal, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 2

[:Gijs (he/him)]

Attachment: Bug 1753696 - use an eslint rule to switch to add_setup(), r?Standard8

Comment 3

[:Gijs (he/him)]

Attachment: Bug 1753696 - Automatic rewrite: replace add_task(setup with add_setup in browser mochitest, r?Standard8

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a9d957ea887b
use an eslint rule to switch to add_setup(), r=Standard8
https://hg.mozilla.org/integration/autoland/rev/67fd9edfbcea
Automatic rewrite: replace add_task(setup with add_setup in browser mochitest, r=Standard8,webcompat-reviewers,twisniewski

Comment 5

[:Gijs (he/him)]

Attachment: Bug 1753696 - go back to using add_task in files that have no other tasks at all, r?Standard8

Comment 6

[Norisz Fay [:noriszfay]]

Backed out for causing multiple dt and bc failures

Backout link

Push with failures

Failure log 1 // Failure log 2 // Failure log 3 // Failure log 4 // Failure log 5 // Failure log 6

Comment 7

[:Gijs (he/him)]

Attachment: Bug 1753696 - move common setup/init tasks in head.js files to use add_setup, r?Standard8

Without this, changing to add_setup in individual test files causes the tasks to be reordered, which causes tests to fail.

I also had to adjust an enterprise policy test that was expecting setup tests to run inbetween tasks.

Depends on D142441

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/259e01cdb165
use an eslint rule to switch to add_setup(), r=Standard8
https://hg.mozilla.org/integration/autoland/rev/d0f843d504bb
Automatic rewrite: replace add_task(setup with add_setup in browser mochitest, r=Standard8,webcompat-reviewers,twisniewski
https://hg.mozilla.org/integration/autoland/rev/7eb53ef3d0e4
go back to using add_task in files that have no other tasks at all, r=Standard8
https://hg.mozilla.org/integration/autoland/rev/6167d8a8aa15
move common setup/init tasks in head.js files to use add_setup, r=Standard8

Comment 9

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/259e01cdb165
https://hg.mozilla.org/mozilla-central/rev/d0f843d504bb
https://hg.mozilla.org/mozilla-central/rev/7eb53ef3d0e4
https://hg.mozilla.org/mozilla-central/rev/6167d8a8aa15

Comment 10

[Geoff Lankow (:darktrojan)]

The new rule is case-sensitive, so there's a few missed functions named setUp.
https://searchfox.org/mozilla-central/search?q=function+setUp%28%29&path=&case=true&regexp=false

Comment 11

[:Gijs (he/him)]

(In reply to Geoff Lankow (:darktrojan) from comment #10)

The new rule is case-sensitive, so there's a few missed functions named setUp.
https://searchfox.org/mozilla-central/search?q=function+setUp%28%29&path=&case=true&regexp=false

Thanks for pointing this out, I filed bug 1762444 for this. FWIW, I mostly wrote the rule to help socialize the new concept and avoid copy-paste of existing tests meaning that it goes unused. Taken together, I'm hoping this will pave the way to better logic around e.g. making SpecialPowers.pushPrefEnv more ergonomic, and having a per-task vs per-file version. But that's still a way off.