Add jest lint rules (#29760)

## Overview Updates `eslint-plugin-jest` and enables the recommended rules with some turned off that are unhelpful. The main motivations is: a) we have a few duplicated tests, which this found an I deleted b) making sure we don't accidentally commit skipped tests
facebook · Jun 10, 2024 · d172bda · d172bda
1 parent c50d593
commit d172bda
Show file tree

Hide file tree

Showing 91 changed files with 790 additions and 812 deletions.
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -12,7 +12,7 @@ const WARNING = 1;
 const ERROR = 2;
 
 module.exports = {
-  extends: ['prettier'],
+  extends: ['prettier', 'plugin:jest/recommended'],
 
   // Stop ESLint from looking for a configuration file in parent folders
   root: true,
@@ -376,16 +376,49 @@ module.exports = {
       files: ['**/__tests__/*.js'],
       rules: {
         // https://github.com/jest-community/eslint-plugin-jest
-        'jest/no-focused-tests': ERROR,
-        'jest/valid-expect': ERROR,
-        'jest/valid-expect-in-promise': ERROR,
+        // Meh, who cares.
+        'jest/consistent-test-it': OFF,
+        // Meh, we have a lot of these, who cares.
+        'jest/no-alias-methods': OFF,
+        // We do conditions based on feature flags.
+        'jest/no-conditional-expect': OFF,
+        // We have our own assertion helpers.
+        'jest/expect-expect': OFF,
+        // Lame rule that fires in itRender helpers or in render methods.
+        'jest/no-standalone-expect': OFF,
       },
     },
     {
-      // disable no focused tests for test setup helper files even if they are inside __tests__ directory
-      files: ['**/setupTests.js'],
+      // Rules specific to test setup helper files.
+      files: [
+        '**/setupTests.js',
+        '**/setupEnv.js',
+        '**/jest/TestFlags.js',
+        '**/dom-event-testing-library/testHelpers.js',
+        '**/utils/ReactDOMServerIntegrationTestUtils.js',
+        '**/babel/transform-react-version-pragma.js',
+        '**/babel/transform-test-gate-pragma.js',
+      ],
       rules: {
+        // Some helpers intentionally focus tests.
         'jest/no-focused-tests': OFF,
+        // Test fn helpers don't use static text names.
+        'jest/valid-title': OFF,
+        // We have our own assertion helpers.
+        'jest/expect-expect': OFF,
+        // Some helpers intentionally disable tests.
+        'jest/no-disabled-tests': OFF,
+        // Helpers export text function helpers.
+        'jest/no-export': OFF,
+        // The examples in comments trigger false errors.
+        'jest/no-commented-out-tests': OFF,
+      },
+    },
+    {
+      files: ['**/jest/TestFlags.js'],
+      rules: {
+        // The examples in comments trigger false errors.
+        'jest/no-commented-out-tests': OFF,
       },
     },
     {

diff --git a/package.json b/package.json
@@ -56,7 +56,7 @@
     "eslint-plugin-babel": "^5.3.0",
     "eslint-plugin-eslint-plugin": "^3.5.3",
     "eslint-plugin-ft-flow": "^2.0.3",
-    "eslint-plugin-jest": "^22.15.0",
+    "eslint-plugin-jest": "28.4.0",
     "eslint-plugin-no-for-of-loops": "^1.0.0",
     "eslint-plugin-no-function-declare-after-return": "^1.0.0",
     "eslint-plugin-react": "^6.7.1",

diff --git a/packages/dom-event-testing-library/__tests__/index-test.internal.js b/packages/dom-event-testing-library/__tests__/index-test.internal.js
@@ -45,7 +45,7 @@ describe('createEventTarget', () => {
     resetActivePointers();
   });
 
-  test('returns expected API', () => {
+  it('returns expected API', () => {
     const target = createEventTarget(node);
     expect(target.node).toEqual(node);
     expect(Object.keys(target)).toMatchInlineSnapshot(`
@@ -77,15 +77,15 @@ describe('createEventTarget', () => {
    */
 
   describe('.blur()', () => {
-    test('default', () => {
+    it('default', () => {
       const target = createEventTarget(node);
       node.addEventListener('blur', e => {
         expect(e.relatedTarget).toMatchInlineSnapshot(`null`);
       });
       target.blur();
     });
 
-    test('custom payload', () => {
+    it('custom payload', () => {
       const target = createEventTarget(node);
       node.addEventListener('blur', e => {
         expect(e.relatedTarget).toMatchInlineSnapshot(`null`);
@@ -95,7 +95,7 @@ describe('createEventTarget', () => {
   });
 
   describe('.click()', () => {
-    test('default', () => {
+    it('default', () => {
       const target = createEventTarget(node);
       node.addEventListener('click', e => {
         expect(e.altKey).toEqual(false);
@@ -122,7 +122,7 @@ describe('createEventTarget', () => {
       target.click();
     });
 
-    test('custom payload', () => {
+    it('custom payload', () => {
       const target = createEventTarget(node);
       node.addEventListener('click', e => {
         expect(e.altKey).toEqual(true);
@@ -162,15 +162,15 @@ describe('createEventTarget', () => {
   });
 
   describe('.focus()', () => {
-    test('default', () => {
+    it('default', () => {
       const target = createEventTarget(node);
       node.addEventListener('focus', e => {
         expect(e.relatedTarget).toMatchInlineSnapshot(`null`);
       });
       target.blur();
     });
 
-    test('custom payload', () => {
+    it('custom payload', () => {
       const target = createEventTarget(node);
       node.addEventListener('focus', e => {
         expect(e.relatedTarget).toMatchInlineSnapshot(`null`);
@@ -180,7 +180,7 @@ describe('createEventTarget', () => {
   });
 
   describe('.keydown()', () => {
-    test('default', () => {
+    it('default', () => {
       const target = createEventTarget(node);
       node.addEventListener('keydown', e => {
         expect(e.altKey).toEqual(false);
@@ -195,7 +195,7 @@ describe('createEventTarget', () => {
       target.keydown();
     });
 
-    test('custom payload', () => {
+    it('custom payload', () => {
       const target = createEventTarget(node);
       node.addEventListener('keydown', e => {
         expect(e.altKey).toEqual(true);
@@ -217,7 +217,7 @@ describe('createEventTarget', () => {
   });
 
   describe('.keyup()', () => {
-    test('default', () => {
+    it('default', () => {
       const target = createEventTarget(node);
       node.addEventListener('keyup', e => {
         expect(e.altKey).toEqual(false);
@@ -232,7 +232,7 @@ describe('createEventTarget', () => {
       target.keydown();
     });
 
-    test('custom payload', () => {
+    it('custom payload', () => {
       const target = createEventTarget(node);
       node.addEventListener('keyup', e => {
         expect(e.altKey).toEqual(true);
@@ -254,7 +254,7 @@ describe('createEventTarget', () => {
   });
 
   describe('.scroll()', () => {
-    test('default', () => {
+    it('default', () => {
       const target = createEventTarget(node);
       node.addEventListener('scroll', e => {
         expect(e.type).toEqual('scroll');
@@ -264,7 +264,7 @@ describe('createEventTarget', () => {
   });
 
   describe('.virtualclick()', () => {
-    test('default', () => {
+    it('default', () => {
       const target = createEventTarget(node);
       node.addEventListener('click', e => {
         expect(e.altKey).toEqual(false);
@@ -291,7 +291,7 @@ describe('createEventTarget', () => {
       target.virtualclick();
     });
 
-    test('custom payload', () => {
+    it('custom payload', () => {
       const target = createEventTarget(node);
       node.addEventListener('click', e => {
         // expect most of the custom payload to be ignored
@@ -334,7 +334,7 @@ describe('createEventTarget', () => {
    * Other APIs
    */
 
-  test('.setBoundingClientRect()', () => {
+  it('.setBoundingClientRect()', () => {
     const target = createEventTarget(node);
     expect(node.getBoundingClientRect()).toMatchInlineSnapshot(`
       {

diff --git a/packages/internal-test-utils/__tests__/ReactInternalTestUtils-test.js b/packages/internal-test-utils/__tests__/ReactInternalTestUtils-test.js
@@ -36,7 +36,7 @@ const {
 } = require('../ReactInternalTestUtils');
 
 describe('ReactInternalTestUtils', () => {
-  test('waitFor', async () => {
+  it('waitFor', async () => {
     const Yield = ({id}) => {
       Scheduler.log(id);
       return id;
@@ -61,7 +61,7 @@ describe('ReactInternalTestUtils', () => {
     expect(root).toMatchRenderedOutput(<div>foobarbaz</div>);
   });
 
-  test('waitForAll', async () => {
+  it('waitForAll', async () => {
     const Yield = ({id}) => {
       Scheduler.log(id);
       return id;
@@ -82,7 +82,7 @@ describe('ReactInternalTestUtils', () => {
     expect(root).toMatchRenderedOutput(<div>foobarbaz</div>);
   });
 
-  test('waitForThrow', async () => {
+  it('waitForThrow', async () => {
     const Yield = ({id}) => {
       Scheduler.log(id);
       return id;
@@ -117,7 +117,7 @@ describe('ReactInternalTestUtils', () => {
     ]);
   });
 
-  test('waitForPaint', async () => {
+  it('waitForPaint', async () => {
     function App({prop}) {
       const deferred = useDeferredValue(prop);
       const text = `Urgent: ${prop}, Deferred: ${deferred}`;
@@ -143,7 +143,7 @@ describe('ReactInternalTestUtils', () => {
     expect(root).toMatchRenderedOutput('Urgent: B, Deferred: B');
   });
 
-  test('assertLog', async () => {
+  it('assertLog', async () => {
     const Yield = ({id}) => {
       Scheduler.log(id);
       React.useEffect(() => {
@@ -732,7 +732,7 @@ describe('ReactInternalTestUtils console assertions', () => {
       await waitForAll(['foo', 'bar', 'baz']);
     });
 
-    test('should fail if waitForThrow is called before asserting', async () => {
+    it('should fail if waitForThrow is called before asserting', async () => {
       const Yield = ({id}) => {
         Scheduler.log(id);
         return id;
@@ -774,7 +774,7 @@ describe('ReactInternalTestUtils console assertions', () => {
       await waitForAll(['A', 'B', 'A', 'B']);
     });
 
-    test('should fail if waitForPaint is called before asserting', async () => {
+    it('should fail if waitForPaint is called before asserting', async () => {
       function App({prop}) {
         const deferred = useDeferredValue(prop);
         const text = `Urgent: ${prop}, Deferred: ${deferred}`;
@@ -1664,7 +1664,7 @@ describe('ReactInternalTestUtils console assertions', () => {
       await waitForAll(['foo', 'bar', 'baz']);
     });
 
-    test('should fail if waitForThrow is called before asserting', async () => {
+    it('should fail if waitForThrow is called before asserting', async () => {
       const Yield = ({id}) => {
         Scheduler.log(id);
         return id;
@@ -1706,7 +1706,7 @@ describe('ReactInternalTestUtils console assertions', () => {
       await waitForAll(['A', 'B', 'A', 'B']);
     });
 
-    test('should fail if waitForPaint is called before asserting', async () => {
+    it('should fail if waitForPaint is called before asserting', async () => {
       function App({prop}) {
         const deferred = useDeferredValue(prop);
         const text = `Urgent: ${prop}, Deferred: ${deferred}`;
@@ -2640,7 +2640,7 @@ describe('ReactInternalTestUtils console assertions', () => {
       await waitForAll(['foo', 'bar', 'baz']);
     });
 
-    test('should fail if waitForThrow is called before asserting', async () => {
+    it('should fail if waitForThrow is called before asserting', async () => {
       const Yield = ({id}) => {
         Scheduler.log(id);
         return id;
@@ -2682,7 +2682,7 @@ describe('ReactInternalTestUtils console assertions', () => {
       await waitForAll(['A', 'B', 'A', 'B']);
     });
 
-    test('should fail if waitForPaint is called before asserting', async () => {
+    it('should fail if waitForPaint is called before asserting', async () => {
       function App({prop}) {
         const deferred = useDeferredValue(prop);
         const text = `Urgent: ${prop}, Deferred: ${deferred}`;

diff --git a/packages/react-devtools-inline/__tests__/__e2e__/utils.js b/packages/react-devtools-inline/__tests__/__e2e__/utils.js
@@ -7,6 +7,7 @@ const config = require('../../playwright.config');
 const {test} = require('@playwright/test');
 
 function runOnlyForReactRange(range) {
+  // eslint-disable-next-line jest/no-disabled-tests
   test.skip(
     !semver.satisfies(config.use.react_version, range),
     `This test requires a React version of ${range} to run. ` +

diff --git a/packages/react-devtools-shared/src/__tests__/TimelineProfiler-test.js b/packages/react-devtools-shared/src/__tests__/TimelineProfiler-test.js
@@ -141,7 +141,8 @@ describe('Timeline profiler', () => {
       // TODO(hoxyq): investigate why running this test with React 18 fails
       // @reactVersion <= 18.2
       // @reactVersion >= 18.0
-      xit('should mark sync render with suspense that resolves', async () => {
+      // eslint-disable-next-line jest/no-disabled-tests
+      it.skip('should mark sync render with suspense that resolves', async () => {
         const fakeSuspensePromise = Promise.resolve(true);
         function Example() {
           throw fakeSuspensePromise;
@@ -186,7 +187,8 @@ describe('Timeline profiler', () => {
       // TODO(hoxyq): investigate why running this test with React 18 fails
       // @reactVersion <= 18.2
       // @reactVersion >= 18.0
-      xit('should mark sync render with suspense that rejects', async () => {
+      // eslint-disable-next-line jest/no-disabled-tests
+      it.skip('should mark sync render with suspense that rejects', async () => {
         const fakeSuspensePromise = Promise.reject(new Error('error'));
         function Example() {
           throw fakeSuspensePromise;
@@ -1528,7 +1530,7 @@ describe('Timeline profiler', () => {
                   `);
         });
 
-        it('should mark concurrent render without suspends or state updates', () => {
+        it('should mark concurrent render without suspends with state updates', () => {
           let updaterFn;
 
           function Example() {

diff --git a/packages/react-devtools-shared/src/__tests__/__serializers__/dehydratedValueSerializer.js b/packages/react-devtools-shared/src/__tests__/__serializers__/dehydratedValueSerializer.js
@@ -7,7 +7,7 @@
  * @flow
  */
 
-// test() is part of Jest's serializer API
+// `test` is part of Jest's serializer API
 export function test(maybeDehydratedValue) {
   const {meta} = require('react-devtools-shared/src/hydration');
 

diff --git a/packages/react-devtools-shared/src/__tests__/__serializers__/hookSerializer.js b/packages/react-devtools-shared/src/__tests__/__serializers__/hookSerializer.js
@@ -34,7 +34,7 @@ function serializeHook(hook) {
   };
 }
 
-// test() is part of Jest's serializer API
+// `test` is part of Jest's serializer API
 export function test(maybeHook) {
   if (maybeHook === null || typeof maybeHook !== 'object') {
     return false;

diff --git a/packages/react-devtools-shared/src/__tests__/__serializers__/inspectedElementSerializer.js b/packages/react-devtools-shared/src/__tests__/__serializers__/inspectedElementSerializer.js
@@ -1,4 +1,4 @@
-// test() is part of Jest's serializer API
+// `test` is part of Jest's serializer API
 export function test(maybeInspectedElement) {
   if (
     maybeInspectedElement === null ||

diff --git a/packages/react-devtools-shared/src/__tests__/__serializers__/numberToFixedSerializer.js b/packages/react-devtools-shared/src/__tests__/__serializers__/numberToFixedSerializer.js
@@ -1,6 +1,6 @@
 const MAX_DECIMAL_PLACES = 3;
 
-// test() is part of Jest's serializer API
+// `test` is part of Jest's serializer API
 export function test(maybeNumber) {
   return (
     typeof maybeNumber === 'number' &&

diff --git a/packages/react-devtools-shared/src/__tests__/__serializers__/profilingSerializer.js b/packages/react-devtools-shared/src/__tests__/__serializers__/profilingSerializer.js
@@ -2,7 +2,7 @@ import hasOwnProperty from 'shared/hasOwnProperty';
 
 const FILTERED_VERSION_STRING = '<filtered-version>';
 
-// test() is part of Jest's serializer API
+// `test` is part of Jest's serializer API
 export function test(maybeProfile) {
   if (
     maybeProfile != null &&

diff --git a/packages/react-devtools-shared/src/__tests__/__serializers__/storeSerializer.js b/packages/react-devtools-shared/src/__tests__/__serializers__/storeSerializer.js
@@ -1,6 +1,6 @@
 import {printStore} from 'react-devtools-shared/src/devtools/utils';
 
-// test() is part of Jest's serializer API
+// `test` is part of Jest's serializer API
 export function test(maybeStore) {
   // It's important to lazy-require the Store rather than imported at the head of the module.
   // Because we reset modules between tests, different Store implementations will be used for each test.