ci: Streamline browser & node unit tests (#13307)

Instead of having to keep to separate lists of include/excludes, we now
keep this in a single list and invert it when necessary.

This way, we should no longer have problems where tests are either run
multiple times, or not in the correct env - just add the test to the
`browser` list in `ci-unit-tests.ts` to make sure it is not run in
multiple node versions unnecessarily. I also added this step to the new
package checklist.

docs/new-sdk-release-checklist.md

@@ -45,8 +45,8 @@ differ slightly for other SDKs depending on how they are structured and how they
 
 - [ ] Make sure `build.yml` CI script is correctly set up to cover tests for the new package
 
-  - [ ] Ensure dependent packages are correctly set for “Determine changed packages”
   - [ ] Ensure unit tests run correctly
+  - [ ] If it is a browser SDK, add it to `BROWSER_TEST_PACKAGES` in `scripts/ci-unit-tests.ts`
 
 - [ ] Make sure the file paths in the
       ["Upload Artifacts" job](https://github.com/getsentry/sentry-javascript/blob/e5c1486eed236b878f2c49d6a04be86093816ac9/.github/workflows/build.yml#L314-L349)

package.json

@@ -35,10 +35,10 @@
     "test:unit": "lerna run --ignore \"@sentry-internal/{browser-integration-tests,e2e-tests,integration-shims,node-integration-tests,overhead-metrics}\" test:unit",
     "test:update-snapshots": "lerna run test:update-snapshots",
     "test:pr": "nx affected -t test --exclude \"@sentry-internal/{browser-integration-tests,e2e-tests,integration-shims,node-integration-tests,overhead-metrics}\"",
-    "test:pr:browser": "yarn test:pr --exclude \"@sentry/{core,utils,opentelemetry,bun,deno,node,profiling-node,aws-serverless,google-cloud-serverless,nextjs,nestjs,astro,cloudflare,solidstart,nuxt,remix,gatsby,sveltekit,vercel-edge}\"",
-    "test:pr:node": "ts-node ./scripts/node-unit-tests.ts --affected",
-    "test:ci:browser": "lerna run test --ignore \"@sentry/{core,utils,opentelemetry,bun,deno,node,profiling-node,aws-serverless,google-cloud-serverless,nextjs,nestjs,astro,cloudflare,solidstart,nuxt,remix,gatsby,sveltekit,vercel-edge}\" --ignore \"@sentry-internal/{browser-integration-tests,e2e-tests,integration-shims,node-integration-tests,overhead-metrics}\"",
-    "test:ci:node": "ts-node ./scripts/node-unit-tests.ts",
+    "test:pr:browser": "UNIT_TEST_ENV=browser ts-node ./scripts/ci-unit-tests.ts --affected",
+    "test:pr:node": "UNIT_TEST_ENV=node ts-node ./scripts/ci-unit-tests.ts --affected",
+    "test:ci:browser": "UNIT_TEST_ENV=browser ts-node ./scripts/ci-unit-tests.ts",
+    "test:ci:node": "UNIT_TEST_ENV=node ts-node ./scripts/ci-unit-tests.ts",
     "test:ci:bun": "lerna run test --scope @sentry/bun",
     "yalc:publish": "lerna run yalc:publish"
   },

scripts/node-unit-tests.ts → scripts/ci-unit-tests.ts

@@ -1,17 +1,24 @@
 import * as childProcess from 'child_process';
+import * as fs from 'fs';
+import * as path from 'path';
 
 type NodeVersion = '14' | '16' | '18' | '20' | '21';
 
 interface VersionConfig {
   ignoredPackages: Array<`@${'sentry' | 'sentry-internal'}/${string}`>;
 }
 
+const UNIT_TEST_ENV = process.env.UNIT_TEST_ENV as 'node' | 'browser' | undefined;
+
 const CURRENT_NODE_VERSION = process.version.replace('v', '').split('.')[0] as NodeVersion;
 
 const RUN_AFFECTED = process.argv.includes('--affected');
 
-const DEFAULT_SKIP_TESTS_PACKAGES = [
-  '@sentry-internal/eslint-plugin-sdk',
+// These packages are tested separately in CI, so no need to run them here
+const DEFAULT_SKIP_PACKAGES = ['@sentry/profiling-node', '@sentry/bun', '@sentry/deno'];
+
+// All other packages are run for multiple node versions
+const BROWSER_TEST_PACKAGES = [
   '@sentry/ember',
   '@sentry/browser',
   '@sentry/vue',
@@ -26,10 +33,9 @@ const DEFAULT_SKIP_TESTS_PACKAGES = [
   '@sentry-internal/replay-worker',
   '@sentry-internal/feedback',
   '@sentry/wasm',
-  '@sentry/bun',
-  '@sentry/deno',
 ];
 
+// These are Node-version specific tests that need to be skipped because of support
 const SKIP_TEST_PACKAGES: Record<NodeVersion, VersionConfig> = {
   '14': {
     ignoredPackages: [
@@ -40,6 +46,7 @@ const SKIP_TEST_PACKAGES: Record<NodeVersion, VersionConfig> = {
       '@sentry/astro',
       '@sentry/nuxt',
       '@sentry/nestjs',
+      '@sentry-internal/eslint-plugin-sdk',
     ],
   },
   '16': {
@@ -56,6 +63,50 @@ const SKIP_TEST_PACKAGES: Record<NodeVersion, VersionConfig> = {
   },
 };
 
+function getAllPackages(): string[] {
+  const { workspaces }: { workspaces: string[] } = JSON.parse(
+    fs.readFileSync(path.join(process.cwd(), 'package.json'), 'utf-8'),
+  );
+
+  return workspaces.map(workspacePath => {
+    const { name }: { name: string } = JSON.parse(
+      fs.readFileSync(path.join(process.cwd(), workspacePath, 'package.json'), 'utf-8'),
+    );
+    return name;
+  });
+}
+
+/**
+ * Run the tests, accounting for compatibility problems in older versions of Node.
+ */
+function runTests(): void {
+  const ignores = new Set<string>(DEFAULT_SKIP_PACKAGES);
+
+  const packages = getAllPackages();
+
+  if (UNIT_TEST_ENV === 'browser') {
+    // Since we cannot "include" for affected mode, we instead exclude all other packages
+    packages.forEach(pkg => {
+      if (!BROWSER_TEST_PACKAGES.includes(pkg)) {
+        ignores.add(pkg);
+      }
+    });
+  } else if (UNIT_TEST_ENV === 'node') {
+    BROWSER_TEST_PACKAGES.forEach(pkg => ignores.add(pkg));
+  }
+
+  const versionConfig = SKIP_TEST_PACKAGES[CURRENT_NODE_VERSION];
+  if (versionConfig) {
+    versionConfig.ignoredPackages.forEach(dep => ignores.add(dep));
+  }
+
+  if (RUN_AFFECTED) {
+    runAffectedTests(ignores);
+  } else {
+    runAllTests(ignores);
+  }
+}
+
 /**
  * Run the given shell command, piping the shell process's `stdin`, `stdout`, and `stderr` to that of the current
  * process. Returns contents of `stdout`.
@@ -67,41 +118,28 @@ function run(cmd: string, options?: childProcess.ExecSyncOptions): void {
 /**
  * Run tests, ignoring the given packages
  */
-function runWithIgnores(skipPackages: string[] = []): void {
-  const ignoreFlags = skipPackages.map(dep => `--ignore="${dep}"`).join(' ');
+function runAllTests(ignorePackages: Set<string>): void {
+  const ignoreFlags = Array.from(ignorePackages)
+    .map(dep => `--ignore="${dep}"`)
+    .join(' ');
+
   run(`yarn test ${ignoreFlags}`);
 }
 
 /**
  * Run affected tests, ignoring the given packages
  */
-function runAffectedWithIgnores(skipPackages: string[] = []): void {
+function runAffectedTests(ignorePackages: Set<string>): void {
   const additionalArgs = process.argv
     .slice(2)
     .filter(arg => arg !== '--affected')
     .join(' ');
-  const ignoreFlags = skipPackages.map(dep => `--exclude="${dep}"`).join(' ');
-  run(`yarn test:pr ${ignoreFlags} ${additionalArgs}`);
-}
-
-/**
- * Run the tests, accounting for compatibility problems in older versions of Node.
- */
-function runTests(): void {
-  const ignores = new Set<string>();
-
-  DEFAULT_SKIP_TESTS_PACKAGES.forEach(pkg => ignores.add(pkg));
 
-  const versionConfig = SKIP_TEST_PACKAGES[CURRENT_NODE_VERSION];
-  if (versionConfig) {
-    versionConfig.ignoredPackages.forEach(dep => ignores.add(dep));
-  }
+  const excludeFlags = Array.from(ignorePackages)
+    .map(dep => `--exclude="${dep}"`)
+    .join(' ');
 
-  if (RUN_AFFECTED) {
-    runAffectedWithIgnores(Array.from(ignores));
-  } else {
-    runWithIgnores(Array.from(ignores));
-  }
+  run(`yarn test:pr ${excludeFlags} ${additionalArgs}`);
 }
 
 runTests();