Bug 1780071 - turn on no-comparison-or-assignment-inside-ok for xpcshell and browser tests, r=mossop,devtools-reviewers,fxview-reviewers,sclements,ochameau

I'll split up the autofix commits separately assuming we're all happy to go ahead with this.

Differential Revision: https://phabricator.services.mozilla.com/D198594

.eslintrc.js

@@ -191,7 +191,7 @@ module.exports = {
       },
     },
     {
-      ...browserTestConfig,
+      ...removeOverrides(browserTestConfig),
       files: testPaths.browser.map(path => `${path}**`),
       excludedFiles: ["**/*.jsm", "**/*.mjs"],
     },
@@ -221,6 +221,33 @@ module.exports = {
         ...testPaths.chrome.map(path => `${path}/**/*.js`),
       ],
     },
+    {
+      // Some directories have multiple kinds of tests, and some rules
+      // don't work well for HTML-based mochitests, so disable those.
+      files: testPaths.xpcshell
+        .concat(testPaths.browser)
+        .map(path => [`${path}/**/*.html`, `${path}/**/*.xhtml`])
+        .flat(),
+      rules: {
+        // plain/chrome mochitests don't automatically include Assert, so
+        // autofixing `ok()` to Assert.something is bad.
+        "mozilla/no-comparison-or-assignment-inside-ok": "off",
+      },
+    },
+    {
+      // Some directories reuse `test_foo.js` files between mochitest-plain and
+      // unit tests, or use custom postMessage-based assertion propagation into
+      // browser tests. Ignore those too:
+      files: [
+        // Reuses xpcshell unit test scripts in mochitest-plain HTML files.
+        "dom/indexedDB/test/**",
+        // Dispatches functions to the webpage in ways that are hard to detect.
+        "toolkit/components/antitracking/test/**",
+      ],
+      rules: {
+        "mozilla/no-comparison-or-assignment-inside-ok": "off",
+      },
+    },
     {
       // Bug 881389 - Complete switching to console.createInstance from custom
       // modules. To support the gradual switch, we log these as warnings until

browser/components/firefoxview/tests/browser/browser_opentabs_changes.js

@@ -505,8 +505,9 @@ add_task(async function test_tabsFromPrivateWindows() {
   });
   const private2TabsChanges = getTabsTargetForWindow(private2Win);
   private2TabsChanges.addEventListener("TabChange", private2Listener);
-  ok(
-    privateTabsChanges !== getTabsTargetForWindow(private2Win),
+  Assert.notStrictEqual(
+    privateTabsChanges,
+    getTabsTargetForWindow(private2Win),
     "getTabsTargetForWindow creates a distinct instance for a different private window"
   );
 

devtools/client/inspector/animation/test/summary-graph_delay-sign_head.js

@@ -75,8 +75,9 @@ async function testSummaryGraphDelaySign() {
       function assertExpected(key) {
         const actual = parseFloat(delaySignEl.style[key]);
         const expected = parseFloat(expectedResult[key]);
-        ok(
-          Math.abs(actual - expected) < 0.01,
+        Assert.less(
+          Math.abs(actual - expected),
+          0.01,
           `${key} should be ${expected} (got ${actual})`
         );
       }

devtools/client/inspector/animation/test/summary-graph_end-delay-sign_head.js

@@ -69,8 +69,9 @@ async function testSummaryGraphEndDelaySign() {
       function assertExpected(key) {
         const actual = parseFloat(endDelaySignEl.style[key]);
         const expected = parseFloat(expectedResult[key]);
-        ok(
-          Math.abs(actual - expected) < 0.01,
+        Assert.less(
+          Math.abs(actual - expected),
+          0.01,
           `${key} should be ${expected} (got ${actual})`
         );
       }

tools/lint/eslint/eslint-plugin-mozilla/lib/configs/browser-test.js

@@ -57,6 +57,19 @@ module.exports = {
     waitForFocus: false,
   },
 
+  overrides: [
+    {
+      // Some directories have multiple kinds of tests, and some rules
+      // don't work well for plain mochitests, so disable those.
+      files: ["*.html", "*.xhtml"],
+      // plain/chrome mochitests don't automatically include Assert, so
+      // autofixing `ok()` to Assert.something is bad.
+      rules: {
+        "mozilla/no-comparison-or-assignment-inside-ok": "off",
+      },
+    },
+  ],
+
   plugins: ["mozilla", "@microsoft/sdl"],
 
   rules: {
@@ -83,6 +96,7 @@ module.exports = {
     "mozilla/mark-test-function-used": "error",
     "mozilla/no-addtask-setup": "error",
     "mozilla/no-arbitrary-setTimeout": "error",
+    "mozilla/no-comparison-or-assignment-inside-ok": "error",
     "mozilla/no-redeclare-with-import-autofix": [
       "error",
       { errorForNonImports: false },

tools/lint/eslint/eslint-plugin-mozilla/lib/configs/xpcshell-test.js

@@ -9,6 +9,16 @@ module.exports = {
   },
 
   overrides: [
+    {
+      // Some directories have multiple kinds of tests, and some rules
+      // don't work well for plain mochitests, so disable those.
+      files: ["*.html", "*.xhtml"],
+      // plain/chrome mochitests don't automatically include Assert, so
+      // autofixing `ok()` to Assert.something is bad.
+      rules: {
+        "mozilla/no-comparison-or-assignment-inside-ok": "off",
+      },
+    },
     {
       // If it is a head file, we turn off global unused variable checks, as it
       // would require searching the other test files to know if they are used or not.
@@ -44,6 +54,7 @@ module.exports = {
     "mozilla/import-headjs-globals": "error",
     "mozilla/mark-test-function-used": "error",
     "mozilla/no-arbitrary-setTimeout": "error",
+    "mozilla/no-comparison-or-assignment-inside-ok": "error",
     "mozilla/no-useless-run-test": "error",
     "no-shadow": "error",
     // Turn off no-unsanitized for tests, as we do want to be able to use