Skip to content

Commit 2306458

Browse files
authored
ci: Only run affected unit tests on PRs (#13052)
This PR updates our unit test runner on CI to only run unit tests for packages that have been changed. In addition, it also updates the list of packages to run in node/browser only unit test envs - we've been running some in both. We only actually need to run the fully-browser-only unit tests in the "Browser Unit Tests" job, the rest (including e.g. core, ...) runs in the node unit tests in all node versions. In a follow up step, maybe we can further streamline this and simply run only unit tests and ensure that e.g. the browser ones only run in one of the node versions, not all of them. Or find another way that does not require us to keep 2 lists of packages in separate places. But for now, this should be an improvement. I opened a test PR to show this in action, where only something in the react package was changed: * Browser Unit Tests: https://github.com/getsentry/sentry-javascript/actions/runs/10180894649/job/28160138970 * Node Unit Tests: https://github.com/getsentry/sentry-javascript/actions/runs/10180894649/job/28160141152
1 parent 34708fd commit 2306458

File tree

3 files changed

+55
-9
lines changed

3 files changed

+55
-9
lines changed

.github/workflows/build.yml

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -434,6 +434,11 @@ jobs:
434434
timeout-minutes: 10
435435
runs-on: ubuntu-20.04
436436
steps:
437+
- name: Check out base commit (${{ github.event.pull_request.base.sha }})
438+
uses: actions/checkout@v4
439+
with:
440+
ref: ${{ github.event.pull_request.base.sha }}
441+
437442
- name: Check out current commit (${{ needs.job_get_metadata.outputs.commit_label }})
438443
uses: actions/checkout@v4
439444
with:
@@ -446,8 +451,15 @@ jobs:
446451
uses: ./.github/actions/restore-cache
447452
env:
448453
DEPENDENCY_CACHE_KEY: ${{ needs.job_build.outputs.dependency_cache_key }}
449-
- name: Run tests
450-
run: yarn test-ci-browser
454+
455+
- name: Run affected tests
456+
run: yarn test:pr:browser --base=${{ github.event.pull_request.base.sha }}
457+
if: github.event_name == 'pull_request'
458+
459+
- name: Run all tests
460+
run: yarn test:ci:browser
461+
if: github.event_name != 'pull_request'
462+
451463
- name: Compute test coverage
452464
uses: codecov/codecov-action@v4
453465
with:
@@ -478,7 +490,7 @@ jobs:
478490
DEPENDENCY_CACHE_KEY: ${{ needs.job_build.outputs.dependency_cache_key }}
479491
- name: Run tests
480492
run: |
481-
yarn test-ci-bun
493+
yarn test:ci:bun
482494
483495
job_deno_unit_tests:
484496
name: Deno Unit Tests
@@ -521,6 +533,10 @@ jobs:
521533
matrix:
522534
node: [14, 16, 18, 20, 22]
523535
steps:
536+
- name: Check out base commit (${{ github.event.pull_request.base.sha }})
537+
uses: actions/checkout@v4
538+
with:
539+
ref: ${{ github.event.pull_request.base.sha }}
524540
- name: Check out current commit (${{ needs.job_get_metadata.outputs.commit_label }})
525541
uses: actions/checkout@v4
526542
with:
@@ -533,10 +549,19 @@ jobs:
533549
uses: ./.github/actions/restore-cache
534550
env:
535551
DEPENDENCY_CACHE_KEY: ${{ needs.job_build.outputs.dependency_cache_key }}
536-
- name: Run tests
552+
553+
- name: Run affected tests
554+
run: yarn test:pr:node --base=${{ github.event.pull_request.base.sha }}
555+
if: github.event_name == 'pull_request'
537556
env:
538557
NODE_VERSION: ${{ matrix.node }}
539-
run: yarn test-ci-node
558+
559+
- name: Run all tests
560+
run: yarn test:ci:node
561+
if: github.event_name != 'pull_request'
562+
env:
563+
NODE_VERSION: ${{ matrix.node }}
564+
540565
- name: Compute test coverage
541566
uses: codecov/codecov-action@v4
542567
with:

package.json

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,13 @@
3333
"postpublish": "lerna run --stream --concurrency 1 postpublish",
3434
"test": "lerna run --ignore \"@sentry-internal/{browser-integration-tests,e2e-tests,integration-shims,node-integration-tests,overhead-metrics}\" test",
3535
"test:unit": "lerna run --ignore \"@sentry-internal/{browser-integration-tests,e2e-tests,integration-shims,node-integration-tests,overhead-metrics}\" test:unit",
36-
"test-ci-browser": "lerna run test --ignore \"@sentry/{bun,deno,node,profiling-node,serverless,google-cloud,nextjs,remix,gatsby,sveltekit,vercel-edge}\" --ignore \"@sentry-internal/{browser-integration-tests,e2e-tests,integration-shims,node-integration-tests,overhead-metrics}\"",
37-
"test-ci-node": "ts-node ./scripts/node-unit-tests.ts",
38-
"test-ci-bun": "lerna run test --scope @sentry/bun",
3936
"test:update-snapshots": "lerna run test:update-snapshots",
37+
"test:pr": "nx affected -t test --exclude \"@sentry-internal/{browser-integration-tests,e2e-tests,integration-shims,node-integration-tests,overhead-metrics}\"",
38+
"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}\"",
39+
"test:pr:node": "ts-node ./scripts/node-unit-tests.ts --affected",
40+
"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}\"",
41+
"test:ci:node": "ts-node ./scripts/node-unit-tests.ts",
42+
"test:ci:bun": "lerna run test --scope @sentry/bun",
4043
"yalc:publish": "lerna run yalc:publish"
4144
},
4245
"volta": {

scripts/node-unit-tests.ts

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ interface VersionConfig {
88

99
const CURRENT_NODE_VERSION = process.version.replace('v', '').split('.')[0] as NodeVersion;
1010

11+
const RUN_AFFECTED = process.argv.includes('--affected');
12+
1113
const DEFAULT_SKIP_TESTS_PACKAGES = [
1214
'@sentry-internal/eslint-plugin-sdk',
1315
'@sentry/ember',
@@ -70,6 +72,18 @@ function runWithIgnores(skipPackages: string[] = []): void {
7072
run(`yarn test ${ignoreFlags}`);
7173
}
7274

75+
/**
76+
* Run affected tests, ignoring the given packages
77+
*/
78+
function runAffectedWithIgnores(skipPackages: string[] = []): void {
79+
const additionalArgs = process.argv
80+
.slice(2)
81+
.filter(arg => arg !== '--affected')
82+
.join(' ');
83+
const ignoreFlags = skipPackages.map(dep => `--exclude="${dep}"`).join(' ');
84+
run(`yarn test:pr ${ignoreFlags} ${additionalArgs}`);
85+
}
86+
7387
/**
7488
* Run the tests, accounting for compatibility problems in older versions of Node.
7589
*/
@@ -83,7 +97,11 @@ function runTests(): void {
8397
versionConfig.ignoredPackages.forEach(dep => ignores.add(dep));
8498
}
8599

86-
runWithIgnores(Array.from(ignores));
100+
if (RUN_AFFECTED) {
101+
runAffectedWithIgnores(Array.from(ignores));
102+
} else {
103+
runWithIgnores(Array.from(ignores));
104+
}
87105
}
88106

89107
runTests();

0 commit comments

Comments
 (0)