From ad2e0c429becee2efad7f2467e03d620afbdd771 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 11 Jul 2023 09:47:54 +0200 Subject: [PATCH 1/5] chore(eslint): Add lint rules for disabled or focused tests --- packages/eslint-config-sdk/package.json | 1 + packages/eslint-config-sdk/src/index.js | 11 ++++- yarn.lock | 62 +++++++++++++++++++++++++ 3 files changed, 73 insertions(+), 1 deletion(-) diff --git a/packages/eslint-config-sdk/package.json b/packages/eslint-config-sdk/package.json index 8a5c8b770d95..4ab60f10ac1a 100644 --- 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 index c070195ed083..114270bbcc95 100644 --- a/packages/eslint-config-sdk/src/index.js +++ b/packages/eslint-config-sdk/src/index.js @@ -4,7 +4,7 @@ module.exports = { node: true, }, extends: ['prettier', 'eslint:recommended', 'plugin:import/errors', 'plugin:import/warnings'], - plugins: ['@sentry-internal/eslint-plugin-sdk', 'simple-import-sort'], + plugins: ['@sentry-internal/eslint-plugin-sdk', 'simple-import-sort', 'jest'], overrides: [ { // Configuration for JavaScript files @@ -184,6 +184,15 @@ module.exports = { '@typescript-eslint/no-empty-function': 'off', '@sentry-internal/sdk/no-optional-chaining': 'off', '@sentry-internal/sdk/no-nullish-coalescing': 'off', + + // 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", }, }, { diff --git a/yarn.lock b/yarn.lock index 8431c6928ee7..5dc640b26163 100644 --- 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" From b0835e500c8be2135d5bad44cbfa0440c00a8486 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 11 Jul 2023 10:45:06 +0200 Subject: [PATCH 2/5] ignore purposefully skipped offline transport test --- packages/core/test/lib/transports/offline.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/core/test/lib/transports/offline.test.ts b/packages/core/test/lib/transports/offline.test.ts index 6a6474d51ef9..e7174cb7c2e0 100644 --- 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 () => { From 7262c58f2a08bac16bae05d31be8b19a24633944 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 11 Jul 2023 10:54:11 +0200 Subject: [PATCH 3/5] move rules to more specific override --- packages/eslint-config-sdk/src/index.js | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/packages/eslint-config-sdk/src/index.js b/packages/eslint-config-sdk/src/index.js index 114270bbcc95..5be0333d8990 100644 --- 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, }, @@ -195,6 +195,23 @@ module.exports = { "jest/no-disabled-tests": "error", }, }, + { + // Configuration only for test files (this won't apply to utils or other files in test directories) + 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'], From 771b695d3a64002d19cdca7bb1d8971cf16222b0 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 11 Jul 2023 11:36:08 +0200 Subject: [PATCH 4/5] remove duplicated rules, scope plugin just to test files --- packages/eslint-config-sdk/src/index.js | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/packages/eslint-config-sdk/src/index.js b/packages/eslint-config-sdk/src/index.js index 5be0333d8990..ffdc8b8ba0af 100644 --- a/packages/eslint-config-sdk/src/index.js +++ b/packages/eslint-config-sdk/src/index.js @@ -4,7 +4,7 @@ module.exports = { node: true, }, extends: ['prettier', 'eslint:recommended', 'plugin:import/errors', 'plugin:import/warnings'], - plugins: ['@sentry-internal/eslint-plugin-sdk', 'simple-import-sort', 'jest'], + plugins: ['@sentry-internal/eslint-plugin-sdk', 'simple-import-sort'], overrides: [ { // Configuration for JavaScript files @@ -184,19 +184,11 @@ module.exports = { '@typescript-eslint/no-empty-function': 'off', '@sentry-internal/sdk/no-optional-chaining': 'off', '@sentry-internal/sdk/no-nullish-coalescing': 'off', - - // 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 only for test files (this won't apply to utils or other files in test directories) + plugins: ['jest'], env: { jest: true, }, @@ -204,12 +196,12 @@ module.exports = { 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", + '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", + 'jest/no-disabled-tests': 'error', }, }, { From dd2dffb3c0891860f8126ea1a6a5e4c73db5196b Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 13 Jul 2023 15:19:06 +0200 Subject: [PATCH 5/5] disable rule in skipped undici tests --- packages/node/test/integrations/undici.test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/node/test/integrations/undici.test.ts b/packages/node/test/integrations/undici.test.ts index b30ac92c0695..e72f7ce70212 100644 --- 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);