Skip to content

Commit

Permalink
ci: Only run affected unit tests on PRs (#13052)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
mydea authored Aug 1, 2024
1 parent 34708fd commit 2306458
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 9 deletions.
35 changes: 30 additions & 5 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,11 @@ jobs:
timeout-minutes: 10
runs-on: ubuntu-20.04
steps:
- name: Check out base commit (${{ github.event.pull_request.base.sha }})
uses: actions/checkout@v4
with:
ref: ${{ github.event.pull_request.base.sha }}

- name: Check out current commit (${{ needs.job_get_metadata.outputs.commit_label }})
uses: actions/checkout@v4
with:
Expand All @@ -446,8 +451,15 @@ jobs:
uses: ./.github/actions/restore-cache
env:
DEPENDENCY_CACHE_KEY: ${{ needs.job_build.outputs.dependency_cache_key }}
- name: Run tests
run: yarn test-ci-browser

- name: Run affected tests
run: yarn test:pr:browser --base=${{ github.event.pull_request.base.sha }}
if: github.event_name == 'pull_request'

- name: Run all tests
run: yarn test:ci:browser
if: github.event_name != 'pull_request'

- name: Compute test coverage
uses: codecov/codecov-action@v4
with:
Expand Down Expand Up @@ -478,7 +490,7 @@ jobs:
DEPENDENCY_CACHE_KEY: ${{ needs.job_build.outputs.dependency_cache_key }}
- name: Run tests
run: |
yarn test-ci-bun
yarn test:ci:bun
job_deno_unit_tests:
name: Deno Unit Tests
Expand Down Expand Up @@ -521,6 +533,10 @@ jobs:
matrix:
node: [14, 16, 18, 20, 22]
steps:
- name: Check out base commit (${{ github.event.pull_request.base.sha }})
uses: actions/checkout@v4
with:
ref: ${{ github.event.pull_request.base.sha }}
- name: Check out current commit (${{ needs.job_get_metadata.outputs.commit_label }})
uses: actions/checkout@v4
with:
Expand All @@ -533,10 +549,19 @@ jobs:
uses: ./.github/actions/restore-cache
env:
DEPENDENCY_CACHE_KEY: ${{ needs.job_build.outputs.dependency_cache_key }}
- name: Run tests

- name: Run affected tests
run: yarn test:pr:node --base=${{ github.event.pull_request.base.sha }}
if: github.event_name == 'pull_request'
env:
NODE_VERSION: ${{ matrix.node }}
run: yarn test-ci-node

- name: Run all tests
run: yarn test:ci:node
if: github.event_name != 'pull_request'
env:
NODE_VERSION: ${{ matrix.node }}

- name: Compute test coverage
uses: codecov/codecov-action@v4
with:
Expand Down
9 changes: 6 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,13 @@
"postpublish": "lerna run --stream --concurrency 1 postpublish",
"test": "lerna run --ignore \"@sentry-internal/{browser-integration-tests,e2e-tests,integration-shims,node-integration-tests,overhead-metrics}\" test",
"test:unit": "lerna run --ignore \"@sentry-internal/{browser-integration-tests,e2e-tests,integration-shims,node-integration-tests,overhead-metrics}\" test:unit",
"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}\"",
"test-ci-node": "ts-node ./scripts/node-unit-tests.ts",
"test-ci-bun": "lerna run test --scope @sentry/bun",
"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}\"",
"test:ci:node": "ts-node ./scripts/node-unit-tests.ts",
"test:ci:bun": "lerna run test --scope @sentry/bun",
"yalc:publish": "lerna run yalc:publish"
},
"volta": {
Expand Down
20 changes: 19 additions & 1 deletion scripts/node-unit-tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ interface VersionConfig {

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',
'@sentry/ember',
Expand Down Expand Up @@ -70,6 +72,18 @@ function runWithIgnores(skipPackages: string[] = []): void {
run(`yarn test ${ignoreFlags}`);
}

/**
* Run affected tests, ignoring the given packages
*/
function runAffectedWithIgnores(skipPackages: 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.
*/
Expand All @@ -83,7 +97,11 @@ function runTests(): void {
versionConfig.ignoredPackages.forEach(dep => ignores.add(dep));
}

runWithIgnores(Array.from(ignores));
if (RUN_AFFECTED) {
runAffectedWithIgnores(Array.from(ignores));
} else {
runWithIgnores(Array.from(ignores));
}
}

runTests();

0 comments on commit 2306458

Please sign in to comment.