ci: Split node & browser unit tests (#6535)

Author: mydea

.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
@@ -352,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
@@ -385,12 +415,12 @@ jobs:
           NODE_VERSION: ${{ matrix.node }}
         run: |
           [[ $NODE_VERSION == 8 ]] && yarn add --dev --ignore-engines --ignore-scripts --ignore-workspace-root-check [email protected]
-          yarn test-ci
+          yarn test-ci-node
       - name: Compute test coverage
         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
@@ -428,7 +458,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
@@ -470,7 +500,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
@@ -520,7 +550,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
@@ -589,7 +619,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
@@ -625,7 +655,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
@@ -708,7 +738,8 @@ jobs:
       [
         job_build,
         job_browser_build_tests,
-        job_unit_test,
+        job_browser_unit_tests,
+        job_node_unit_tests,
         job_nextjs_integration_test,
         job_node_integration_tests,
         job_browser_playwright_tests,

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": "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": {

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,27 @@ 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',
+  '@sentry/wasm',
+];
+
+// 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 = [
   '[email protected]',
@@ -29,10 +50,10 @@ const NODE_8_LEGACY_DEPENDENCIES = [
   '[email protected]',
 ];
 
-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 = ['[email protected]'];
 
-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;
 
@@ -91,13 +112,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 +131,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<string>();
+
+  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();