From 2a34f7d4f5f9149b32c6ae08ca4956d8901d2d5b Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 14 Dec 2022 15:58:24 +0100 Subject: [PATCH 1/6] chore(test): Allow to run node/browser unit tests separately --- package.json | 2 ++ scripts/test.ts | 79 ++++++++++++++++++++++++++++++++----------------- 2 files changed, 54 insertions(+), 27 deletions(-) diff --git a/package.json b/package.json index 9bbd68de81d3..cbd1022f346c 100644 --- a/package.json +++ b/package.json @@ -26,6 +26,8 @@ "postpublish": "lerna run --stream --concurrency 1 postpublish", "test": "lerna run --ignore @sentry-internal/browser-integration-tests --ignore @sentry-internal/node-integration-tests --stream --concurrency 1 --sort test", "test-ci": "ts-node ./scripts/test.ts", + "test-ci-browser": "TESTS_SKIP=node ts-node ./scripts/test.ts", + "test-ci-node": "TESTS_SKIP=browser ts-node ./scripts/test.ts", "postinstall": "patch-package" }, "volta": { diff --git a/scripts/test.ts b/scripts/test.ts index ddc5cde2489d..36442f4a2fbb 100644 --- a/scripts/test.ts +++ b/scripts/test.ts @@ -5,9 +5,9 @@ const CURRENT_NODE_VERSION = process.version.replace('v', '').split('.')[0]; // We run ember tests in their own job. const DEFAULT_SKIP_TESTS_PACKAGES = ['@sentry/ember']; + // These packages don't support Node 8 for syntax or dependency reasons. const NODE_8_SKIP_TESTS_PACKAGES = [ - ...DEFAULT_SKIP_TESTS_PACKAGES, '@sentry-internal/eslint-plugin-sdk', '@sentry/react', '@sentry/wasm', @@ -20,6 +20,26 @@ const NODE_8_SKIP_TESTS_PACKAGES = [ '@sentry/replay', ]; +// These can be skipped when running tests in different Node environments. +const SKIP_BROWSER_TESTS_PACKAGES = [ + '@sentry/browser', + '@sentry/vue', + '@sentry/react', + '@sentry/angular', + '@sentry/svelte', + '@sentry/replay', +]; + +// These can be skipped when running tests independently of the Node version. +const SKIP_NODE_TESTS_PACKAGES = [ + '@sentry/node', + '@sentry/opentelemetry-node', + '@sentry/serverless', + '@sentry/nextjs', + '@sentry/remix', + '@sentry/gatsby', +]; + // We have to downgrade some of our dependencies in order to run tests in Node 8 and 10. const NODE_8_LEGACY_DEPENDENCIES = [ 'jsdom@15.x', @@ -91,13 +111,10 @@ const es6ifyTestTSConfig = (pkg: string): void => { }; /** - * Skip tests which don't apply to Node and therefore don't need to run in older Node versions. - * - * TODO We're foreced to skip these tests for compatibility reasons (right now this function only gets called in Node - * 8), but we could be skipping a lot more tests in Node 8-14 - anything where compatibility with different Node - * versions is irrelevant - and only running them in Node 16. + * Skip tests which don't run in Node 8. + * We're forced to skip these tests for compatibility reasons. */ -function skipNonNodeTests(): void { +function skipNodeV8Tests(): void { run('rm -rf packages/tracing/test/browser'); } @@ -113,29 +130,37 @@ function runWithIgnores(skipPackages: string[] = []): void { * Run the tests, accounting for compatibility problems in older versions of Node. */ function runTests(): void { - if (CURRENT_NODE_VERSION === '8') { - installLegacyDeps(NODE_8_LEGACY_DEPENDENCIES); - // TODO Right now, this just skips incompatible tests, but it could be skipping more (hence the aspirational name), - // and not just in Node 8. See `skipNonNodeTests`'s docstring. - skipNonNodeTests(); - es6ifyTestTSConfig('utils'); - runWithIgnores(NODE_8_SKIP_TESTS_PACKAGES); - } - // - else if (CURRENT_NODE_VERSION === '10') { - installLegacyDeps(NODE_10_LEGACY_DEPENDENCIES); - es6ifyTestTSConfig('utils'); - runWithIgnores(NODE_10_SKIP_TESTS_PACKAGES); + const ignores = new Set(); + + DEFAULT_SKIP_TESTS_PACKAGES.forEach(dep => ignores.add(dep)); + + if (process.env.TESTS_SKIP === 'browser') { + SKIP_BROWSER_TESTS_PACKAGES.forEach(dep => ignores.add(dep)); } - // - else if (CURRENT_NODE_VERSION === '12') { - es6ifyTestTSConfig('utils'); - runWithIgnores(NODE_12_SKIP_TESTS_PACKAGES); + + if (process.env.TESTS_SKIP === 'node') { + SKIP_NODE_TESTS_PACKAGES.forEach(dep => ignores.add(dep)); } - // - else { - runWithIgnores(DEFAULT_SKIP_TESTS_PACKAGES); + + switch (CURRENT_NODE_VERSION) { + case '8': + NODE_8_SKIP_TESTS_PACKAGES.forEach(dep => ignores.add(dep)); + installLegacyDeps(NODE_8_LEGACY_DEPENDENCIES); + skipNodeV8Tests(); + es6ifyTestTSConfig('utils'); + break; + case '10': + NODE_10_SKIP_TESTS_PACKAGES.forEach(dep => ignores.add(dep)); + installLegacyDeps(NODE_10_LEGACY_DEPENDENCIES); + es6ifyTestTSConfig('utils'); + break; + case '12': + NODE_12_SKIP_TESTS_PACKAGES.forEach(dep => ignores.add(dep)); + es6ifyTestTSConfig('utils'); + break; } + + runWithIgnores(Array.from(ignores)); } runTests(); From 17da4928d83a4715d47dfcc57825b81b9884f84b Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 14 Dec 2022 16:10:27 +0100 Subject: [PATCH 2/6] chore(ci): Run node & browser unit tests separately on CI --- .github/workflows/build.yml | 40 +++++++++++++++++++++++++++++++++---- 1 file changed, 36 insertions(+), 4 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 524832a01a44..65f09d0f096e 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -351,8 +351,39 @@ jobs: ${{ github.workspace }}/packages/replay/build/bundles/** ${{ github.workspace }}/packages/**/*.tgz - job_unit_test: - name: Test (Node ${{ matrix.node }}) + job_browser_unit_tests: + name: Browser Unit Tests + needs: [job_get_metadata, job_build] + timeout-minutes: 10 + runs-on: ubuntu-latest + steps: + - name: Check out current commit (${{ needs.job_get_metadata.outputs.commit_label }}) + uses: actions/checkout@v3 + with: + ref: ${{ env.HEAD_COMMIT }} + - name: Set up Node + uses: actions/setup-node@v3 + with: + node-version: ${{ env.DEFAULT_NODE_VERSION }} + - name: Check dependency cache + uses: actions/cache@v3 + with: + path: ${{ env.CACHED_DEPENDENCY_PATHS }} + key: ${{ needs.job_build.outputs.dependency_cache_key }} + - name: Check build cache + uses: actions/cache@v3 + with: + path: ${{ env.CACHED_BUILD_PATHS }} + key: ${{ env.BUILD_CACHE_KEY }} + - name: Run tests + env: + NODE_VERSION: 16 + run: yarn test-ci-browser + - name: Compute test coverage + uses: codecov/codecov-action@v3 + + job_node_unit_tests: + name: Node (${{ matrix.node }}) Unit Tests needs: [job_get_metadata, job_build] timeout-minutes: 30 runs-on: ubuntu-20.04 @@ -384,7 +415,7 @@ jobs: NODE_VERSION: ${{ matrix.node }} run: | [[ $NODE_VERSION == 8 ]] && yarn add --dev --ignore-engines --ignore-scripts --ignore-workspace-root-check ts-node@8.10.2 - yarn test-ci + yarn test-ci-node - name: Compute test coverage uses: codecov/codecov-action@v3 @@ -707,7 +738,8 @@ jobs: [ job_build, job_browser_build_tests, - job_unit_test, + job_browser_unit_test, + job_node_unit_test, job_nextjs_integration_test, job_node_integration_tests, job_browser_playwright_tests, From 0cbfc2547031ed9e854676dd56673b279685cf2f Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 14 Dec 2022 16:19:22 +0100 Subject: [PATCH 3/6] fix --- .github/workflows/build.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 65f09d0f096e..ad04bec5d7d9 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -738,8 +738,8 @@ jobs: [ job_build, job_browser_build_tests, - job_browser_unit_test, - job_node_unit_test, + job_browser_unit_tests, + job_node_unit_tests, job_nextjs_integration_test, job_node_integration_tests, job_browser_playwright_tests, From bd89029013754a568149aa6c0e3b76d71079ba46 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 15 Dec 2022 09:25:11 +0100 Subject: [PATCH 4/6] ci: Align test naming --- .github/workflows/build.yml | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index ad04bec5d7d9..eb6a41f5c85e 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -32,7 +32,6 @@ env: CACHED_BUILD_PATHS: | ${{ github.workspace }}/packages/*/build ${{ github.workspace }}/packages/ember/*.d.ts - ${{ github.workspace }}/packages/ember/instance-initializers ${{ github.workspace }}/packages/gatsby/*.d.ts ${{ github.workspace }}/packages/core/src/version.ts ${{ github.workspace }}/packages/serverless @@ -420,7 +419,7 @@ jobs: uses: codecov/codecov-action@v3 job_nextjs_integration_test: - name: Test @sentry/nextjs on (Node ${{ matrix.node }}) + name: Nextjs (Node ${{ matrix.node }}) Tests needs: [job_get_metadata, job_build] if: needs.job_get_metadata.outputs.changed_nextjs == 'true' || github.event_name != 'pull_request' timeout-minutes: 30 @@ -458,7 +457,7 @@ jobs: # Ember tests are separate from the rest because they are the slowest part of the test suite, and making them a # separate job allows them to run in parallel with the other tests. job_ember_tests: - name: Test @sentry/ember + name: Ember (${{ matrix.scenario }}) Tests needs: [job_get_metadata, job_build] if: needs.job_get_metadata.outputs.changed_ember == 'true' || github.event_name != 'pull_request' timeout-minutes: 10 @@ -500,7 +499,7 @@ jobs: uses: codecov/codecov-action@v3 job_browser_playwright_tests: - name: Playwright - ${{ (matrix.tracing_only && 'Browser + Tracing') || 'Browser' }} (${{ matrix.bundle }}) + name: Playwright (${{ matrix.bundle }})${{ (matrix.tracing_only && ' tracing only') || '' }} Tests needs: [job_get_metadata, job_build] if: needs.job_get_metadata.outputs.changed_browser_integration == 'true' || github.event_name != 'pull_request' runs-on: ubuntu-20.04 @@ -550,7 +549,7 @@ jobs: yarn test:ci job_browser_integration_tests: - name: Old Browser Integration Tests (${{ matrix.browser }}) + name: Browser (${{ matrix.browser }}) Tests needs: [job_get_metadata, job_build] if: needs.job_get_metadata.outputs.changed_browser == 'true' || github.event_name != 'pull_request' runs-on: ubuntu-20.04 @@ -619,7 +618,7 @@ jobs: yarn test:package job_node_integration_tests: - name: Node SDK Integration Tests (${{ matrix.node }}) + name: Node (${{ matrix.node }}) Integration Tests needs: [job_get_metadata, job_build] if: needs.job_get_metadata.outputs.changed_node == 'true' || github.event_name != 'pull_request' runs-on: ubuntu-20.04 @@ -655,7 +654,7 @@ jobs: yarn test job_remix_integration_tests: - name: Remix SDK Integration Tests (${{ matrix.node }}) + name: Remix (Node ${{ matrix.node }}) Tests needs: [job_get_metadata, job_build] if: needs.job_get_metadata.outputs.changed_remix == 'true' || github.event_name != 'pull_request' runs-on: ubuntu-20.04 From 02e30aad343635c6a9be1f6038a1d6bfe7b6af0e Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 15 Dec 2022 09:33:41 +0100 Subject: [PATCH 5/6] test: Streamline test ignoring --- scripts/test.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/scripts/test.ts b/scripts/test.ts index 36442f4a2fbb..b1af2f918ee8 100644 --- a/scripts/test.ts +++ b/scripts/test.ts @@ -28,6 +28,7 @@ const SKIP_BROWSER_TESTS_PACKAGES = [ '@sentry/angular', '@sentry/svelte', '@sentry/replay', + '@sentry/wasm', ]; // These can be skipped when running tests independently of the Node version. @@ -49,10 +50,10 @@ const NODE_8_LEGACY_DEPENDENCIES = [ 'ts-jest@25.x', ]; -const NODE_10_SKIP_TESTS_PACKAGES = [...DEFAULT_SKIP_TESTS_PACKAGES, '@sentry/remix', '@sentry/replay']; +const NODE_10_SKIP_TESTS_PACKAGES = ['@sentry/remix', '@sentry/replay']; const NODE_10_LEGACY_DEPENDENCIES = ['jsdom@16.x']; -const NODE_12_SKIP_TESTS_PACKAGES = [...DEFAULT_SKIP_TESTS_PACKAGES, '@sentry/remix']; +const NODE_12_SKIP_TESTS_PACKAGES = ['@sentry/remix']; type JSONValue = string | number | boolean | null | JSONArray | JSONObject; From 076b13389530af82ab5cf54631e7a0ed380125c8 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 15 Dec 2022 13:17:28 +0100 Subject: [PATCH 6/6] fix: Use cross-env for env variables --- package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index cbd1022f346c..b7ab59c0c862 100644 --- a/package.json +++ b/package.json @@ -26,8 +26,8 @@ "postpublish": "lerna run --stream --concurrency 1 postpublish", "test": "lerna run --ignore @sentry-internal/browser-integration-tests --ignore @sentry-internal/node-integration-tests --stream --concurrency 1 --sort test", "test-ci": "ts-node ./scripts/test.ts", - "test-ci-browser": "TESTS_SKIP=node ts-node ./scripts/test.ts", - "test-ci-node": "TESTS_SKIP=browser ts-node ./scripts/test.ts", + "test-ci-browser": "cross-env TESTS_SKIP=node ts-node ./scripts/test.ts", + "test-ci-node": "cross-env TESTS_SKIP=browser ts-node ./scripts/test.ts", "postinstall": "patch-package" }, "volta": {