chore(eslint): Add lint rules for disabled or focused tests (#8489)

Lms24 · web-flow · commit ed889bc3188d · 2023-07-19T10:27:31.000+02:00
We sometimes forget to remove focused or disabled annotations after debugging tests. This of course isn't ideal and as brought up in #8485 we should add checks against this. Therefore, this patch: * Adds the [`eslint-plugin-jest` package ](https://github.com/jest-community/eslint-plugin-jest/tree/main)which contains a bunch of jest-syntax-specific rules * Enables the `no-focused-tests` rule to throw a lint error if a test is focused with `it.only`, `fit` or similar functions. * Enables the `no-disabled-tests` rule to throw a lint error if a test (suite) from a suite is disabled/skipped with `it.skip`, `xit` or similar functions. While we sometimes skip tests on purpose, I'd argue that we generally want to avoid this, as it can also happen accidentally. For the few exceptions of this rule, we can always ignore it.
diff --git a/packages/core/test/lib/transports/offline.test.ts b/packages/core/test/lib/transports/offline.test.ts
@@ -326,6 +326,7 @@ describe('makeOfflineTransport', () => {
     expect(getCalls()).toEqual([]);
   });
 
+  // eslint-disable-next-line jest/no-disabled-tests
   it.skip(
     'Follows the Retry-After header',
     async () => {
diff --git a/packages/eslint-config-sdk/package.json b/packages/eslint-config-sdk/package.json
@@ -26,6 +26,7 @@
     "eslint-config-prettier": "^6.11.0",
     "eslint-plugin-deprecation": "^1.1.0",
     "eslint-plugin-import": "^2.22.0",
+    "eslint-plugin-jest": "^27.2.2",
     "eslint-plugin-jsdoc": "^30.0.3",
     "eslint-plugin-simple-import-sort": "^5.0.3"
   },
diff --git a/packages/eslint-config-sdk/src/index.js b/packages/eslint-config-sdk/src/index.js
@@ -167,7 +167,7 @@ module.exports = {
       },
     },
     {
-      // Configuration for test files
+      // Configuration for files in test directories
       env: {
         jest: true,
       },
@@ -186,6 +186,24 @@ module.exports = {
         '@sentry-internal/sdk/no-nullish-coalescing': 'off',
       },
     },
+    {
+      // Configuration only for test files (this won't apply to utils or other files in test directories)
+      plugins: ['jest'],
+      env: {
+        jest: true,
+      },
+      files: ['test.ts', '*.test.ts', '*.test.tsx', '*.test.js', '*.test.jsx'],
+      rules: {
+        // Prevent permanent usage of `it.only`, `fit`, `test.only` etc
+        // We want to avoid debugging leftovers making their way into the codebase
+        'jest/no-focused-tests': 'error',
+
+        // Prevent permanent usage of `it.skip`, `xit`, `test.skip` etc
+        // We want to avoid debugging leftovers making their way into the codebase
+        // If there's a good reason to skip a test (e.g. bad flakiness), just add an ignore comment
+        'jest/no-disabled-tests': 'error',
+      },
+    },
     {
       // Configuration for config files like webpack/rollup
       files: ['*.config.js'],
diff --git a/packages/node/test/integrations/undici.test.ts b/packages/node/test/integrations/undici.test.ts
@@ -194,6 +194,7 @@ conditionalTest({ min: 16 })('Undici integration', () => {
   });
 
   // This flakes on CI for some reason: https://github.com/getsentry/sentry-javascript/pull/8449
+  // eslint-disable-next-line jest/no-disabled-tests
   it.skip('attaches the sentry trace and baggage headers if there is an active span', async () => {
     expect.assertions(3);
 
@@ -214,6 +215,7 @@ conditionalTest({ min: 16 })('Undici integration', () => {
   });
 
   // This flakes on CI for some reason: https://github.com/getsentry/sentry-javascript/pull/8449
+  // eslint-disable-next-line jest/no-disabled-tests
   it.skip('attaches the sentry trace and baggage headers if there is no active span', async () => {
     const scope = hub.getScope();
 
@@ -228,6 +230,7 @@ conditionalTest({ min: 16 })('Undici integration', () => {
   });
 
   // This flakes on CI for some reason: https://github.com/getsentry/sentry-javascript/pull/8449
+  // eslint-disable-next-line jest/no-disabled-tests
   it.skip('attaches headers if `shouldCreateSpanForRequest` does not create a span using propagation context', async () => {
     const transaction = hub.startTransaction({ name: 'test-transaction' }) as Transaction;
     const scope = hub.getScope();
@@ -259,6 +262,7 @@ conditionalTest({ min: 16 })('Undici integration', () => {
   });
 
   // This flakes on CI for some reason: https://github.com/getsentry/sentry-javascript/pull/8449
+  // eslint-disable-next-line jest/no-disabled-tests
   it.skip('uses tracePropagationTargets', async () => {
     const transaction = hub.startTransaction({ name: 'test-transaction' }) as Transaction;
     hub.getScope().setSpan(transaction);
diff --git a/yarn.lock b/yarn.lock
@@ -2483,6 +2483,13 @@
   resolved "https://registry.yarnpkg.com/@esbuild/win32-x64/-/win32-x64-0.16.17.tgz#c5a1a4bfe1b57f0c3e61b29883525c6da3e5c091"
   integrity sha512-y+EHuSchhL7FjHgvQL/0fnnFmO4T1bhvWANX6gcnqTjtnKWbTvUMCpGnv2+t+31d7RzyEAYAd4u2fnIhHL6N/Q==
 
+"@eslint-community/eslint-utils@^4.2.0":
+  version "4.4.0"
+  resolved "https://registry.yarnpkg.com/@eslint-community/eslint-utils/-/eslint-utils-4.4.0.tgz#a23514e8fb9af1269d5f7788aa556798d61c6b59"
+  integrity sha512-1/sA4dwrzBAyeUoQ6oxahHKmrZvsnLCg4RfxW3ZFGGmQkSNQPFNLV9CUEFQP1x9EYXHTo5p6xdhZM1Ne9p/AfA==
+  dependencies:
+    eslint-visitor-keys "^3.3.0"
+
 "@eslint/eslintrc@^0.4.3":
   version "0.4.3"
   resolved "https://registry.yarnpkg.com/@eslint/eslintrc/-/eslintrc-0.4.3.tgz#9e42981ef035beb3dd49add17acb96e8ff6f394c"
@@ -5304,6 +5311,14 @@
     "@typescript-eslint/types" "5.48.0"
     "@typescript-eslint/visitor-keys" "5.48.0"
 
+"@typescript-eslint/scope-manager@5.62.0":
+  version "5.62.0"
+  resolved "https://registry.yarnpkg.com/@typescript-eslint/scope-manager/-/scope-manager-5.62.0.tgz#d9457ccc6a0b8d6b37d0eb252a23022478c5460c"
+  integrity sha512-VXuvVvZeQCQb5Zgf4HAxc04q5j+WrNAtNh9OwCsCgpKqESMTu3tF/jhZ3xG6T4NZwWl65Bg8KuS2uEvhSfLl0w==
+  dependencies:
+    "@typescript-eslint/types" "5.62.0"
+    "@typescript-eslint/visitor-keys" "5.62.0"
+
 "@typescript-eslint/type-utils@5.48.0":
   version "5.48.0"
   resolved "https://registry.yarnpkg.com/@typescript-eslint/type-utils/-/type-utils-5.48.0.tgz#40496dccfdc2daa14a565f8be80ad1ae3882d6d6"
@@ -5329,6 +5344,11 @@
   resolved "https://registry.yarnpkg.com/@typescript-eslint/types/-/types-5.48.0.tgz#d725da8dfcff320aab2ac6f65c97b0df30058449"
   integrity sha512-UTe67B0Ypius0fnEE518NB2N8gGutIlTojeTg4nt0GQvikReVkurqxd2LvYa9q9M5MQ6rtpNyWTBxdscw40Xhw==
 
+"@typescript-eslint/types@5.62.0":
+  version "5.62.0"
+  resolved "https://registry.yarnpkg.com/@typescript-eslint/types/-/types-5.62.0.tgz#258607e60effa309f067608931c3df6fed41fd2f"
+  integrity sha512-87NVngcbVXUahrRTqIK27gD2t5Cu1yuCXxbLcFtCzZGlfyVWWh8mLHkoxzjsB6DDNnvdL+fW8MiwPEJyGJQDgQ==
+
 "@typescript-eslint/typescript-estree@3.10.1":
   version "3.10.1"
   resolved "https://registry.yarnpkg.com/@typescript-eslint/typescript-estree/-/typescript-estree-3.10.1.tgz#fd0061cc38add4fad45136d654408569f365b853"
@@ -5356,6 +5376,19 @@
     semver "^7.3.7"
     tsutils "^3.21.0"
 
+"@typescript-eslint/typescript-estree@5.62.0":
+  version "5.62.0"
+  resolved "https://registry.yarnpkg.com/@typescript-eslint/typescript-estree/-/typescript-estree-5.62.0.tgz#7d17794b77fabcac615d6a48fb143330d962eb9b"
+  integrity sha512-CmcQ6uY7b9y694lKdRB8FEel7JbU/40iSAPomu++SjLMntB+2Leay2LO6i8VnJk58MtE9/nQSFIH6jpyRWyYzA==
+  dependencies:
+    "@typescript-eslint/types" "5.62.0"
+    "@typescript-eslint/visitor-keys" "5.62.0"
+    debug "^4.3.4"
+    globby "^11.1.0"
+    is-glob "^4.0.3"
+    semver "^7.3.7"
+    tsutils "^3.21.0"
+
 "@typescript-eslint/typescript-estree@^4.8.2":
   version "4.23.0"
   resolved "https://registry.yarnpkg.com/@typescript-eslint/typescript-estree/-/typescript-estree-4.23.0.tgz#0753b292097523852428a6f5a1aa8ccc1aae6cd9"
@@ -5383,6 +5416,20 @@
     eslint-utils "^3.0.0"
     semver "^7.3.7"
 
+"@typescript-eslint/utils@^5.10.0":
+  version "5.62.0"
+  resolved "https://registry.yarnpkg.com/@typescript-eslint/utils/-/utils-5.62.0.tgz#141e809c71636e4a75daa39faed2fb5f4b10df86"
+  integrity sha512-n8oxjeb5aIbPFEtmQxQYOLI0i9n5ySBEY/ZEHHZqKQSFnxio1rv6dthascc9dLuwrL0RC5mPCxB7vnAVGAYWAQ==
+  dependencies:
+    "@eslint-community/eslint-utils" "^4.2.0"
+    "@types/json-schema" "^7.0.9"
+    "@types/semver" "^7.3.12"
+    "@typescript-eslint/scope-manager" "5.62.0"
+    "@typescript-eslint/types" "5.62.0"
+    "@typescript-eslint/typescript-estree" "5.62.0"
+    eslint-scope "^5.1.1"
+    semver "^7.3.7"
+
 "@typescript-eslint/visitor-keys@3.10.1":
   version "3.10.1"
   resolved "https://registry.yarnpkg.com/@typescript-eslint/visitor-keys/-/visitor-keys-3.10.1.tgz#cd4274773e3eb63b2e870ac602274487ecd1e931"
@@ -5406,6 +5453,14 @@
     "@typescript-eslint/types" "5.48.0"
     eslint-visitor-keys "^3.3.0"
 
+"@typescript-eslint/visitor-keys@5.62.0":
+  version "5.62.0"
+  resolved "https://registry.yarnpkg.com/@typescript-eslint/visitor-keys/-/visitor-keys-5.62.0.tgz#2174011917ce582875954ffe2f6912d5931e353e"
+  integrity sha512-07ny+LHRzQXepkGg6w0mFY41fVUNBrL2Roj/++7V1txKugfjm/Ci/qSND03r2RhlJhJYMcTn9AhhSSqQp0Ysyw==
+  dependencies:
+    "@typescript-eslint/types" "5.62.0"
+    eslint-visitor-keys "^3.3.0"
+
 "@vitest/coverage-c8@^0.29.2":
   version "0.29.2"
   resolved "https://registry.yarnpkg.com/@vitest/coverage-c8/-/coverage-c8-0.29.2.tgz#30b81e32ff11c20e2f3ab78c84e21b4c6c08190c"
@@ -12491,6 +12546,13 @@ eslint-plugin-import@^2.22.0:
     resolve "^1.17.0"
     tsconfig-paths "^3.9.0"
 
+eslint-plugin-jest@^27.2.2:
+  version "27.2.2"
+  resolved "https://registry.yarnpkg.com/eslint-plugin-jest/-/eslint-plugin-jest-27.2.2.tgz#be4ded5f91905d9ec89aa8968d39c71f3b072c0c"
+  integrity sha512-euzbp06F934Z7UDl5ZUaRPLAc9MKjh0rMPERrHT7UhlCEwgb25kBj37TvMgWeHZVkR5I9CayswrpoaqZU1RImw==
+  dependencies:
+    "@typescript-eslint/utils" "^5.10.0"
+
 eslint-plugin-jsdoc@^30.0.3:
   version "30.7.13"
   resolved "https://registry.yarnpkg.com/eslint-plugin-jsdoc/-/eslint-plugin-jsdoc-30.7.13.tgz#52e5c74fb806d3bbeb51d04a0c829508c3c6b563"
