Skip to content

Commit a5a214c

Browse files
authored
ci: Only run E2E test apps that are affected on PRs (#14254)
This PR aims to solve two things: 1. Do not require us to keep a list of test-applications to run on CI. This is prone to be forgotten, and then certain tests do not run on CI and we may not notice (this has happened more than once in the past 😬 ) 2. Do not run E2E test applications that are unrelated to anything that was changed. With this PR, instead of keeping a list of E2E test jobs we want to run manually in the build.yml file, this is now inferred (on CI) by a script based on the nx dependency graph and some config in the test apps package.json. This is how it works: 1. Pick all folders in test-applications, by default all of them should run. --> This will "fix" the problem we had sometimes that we forgot to add test apps to the build.yml so they don't actually run on CI :grimacing: 3. For each test app, look at it's dependencies (and devDependencies), and see if any of them has changed in the current PR (based on nx affected projects). So e.g. if you change something in browser, only E2E test apps that have any dependency that uses browser will run, so e.g. node-express will be skipped. 4. Additionally, you can configure variants in the test apps package.json - instead of defining this in the workflow file, it has to be defined in the test app itself now. 5. Finally, a test app can be marked as optional in the package.json, and can also have optional variants to run. 6. The E2E test builds the required & optional matrix on the fly based on these things. 7. In pushes (=on develop/release branches) we just run everything. 8. If anything inside of the e2e-tests package changed, run everything. --> This could be further optimized to only run changed test apps, but this was the easy solution for now. ## Example test runs * In a PR with no changes related to the E2E tests, nothing is run: https://github.com/getsentry/sentry-javascript/actions/runs/11838604965 * In a CI run for a push, all E2E tests are run: https://github.com/getsentry/sentry-javascript/actions/runs/11835834748 * In a PR with a change in e2e-tests itself, all E2E tests are run: https://github.com/getsentry/sentry-javascript/actions/runs/11836668035 * In a PR with a change in the node package, only related E2E tests are run: https://github.com/getsentry/sentry-javascript/actions/runs/11838274483
1 parent 12902c5 commit a5a214c

File tree

21 files changed

+326
-192
lines changed

21 files changed

+326
-192
lines changed

.github/workflows/build.yml

+22-152
Original file line numberDiff line numberDiff line change
@@ -801,7 +801,15 @@ jobs:
801801
needs: [job_get_metadata, job_build, job_compile_bindings_profiling_node]
802802
runs-on: ubuntu-20.04-large-js
803803
timeout-minutes: 15
804+
outputs:
805+
matrix: ${{ steps.matrix.outputs.matrix }}
806+
matrix-optional: ${{ steps.matrix-optional.outputs.matrix }}
804807
steps:
808+
- name: Check out base commit (${{ github.event.pull_request.base.sha }})
809+
uses: actions/checkout@v4
810+
if: github.event_name == 'pull_request'
811+
with:
812+
ref: ${{ github.event.pull_request.base.sha }}
805813
- name: Check out current commit (${{ needs.job_get_metadata.outputs.commit_label }})
806814
uses: actions/checkout@v4
807815
with:
@@ -851,11 +859,21 @@ jobs:
851859
path: ${{ github.workspace }}/packages/*/*.tgz
852860
key: ${{ env.BUILD_CACHE_TARBALL_KEY }}
853861

862+
- name: Determine which E2E test applications should be run
863+
id: matrix
864+
run: yarn --silent ci:build-matrix --base=${{ (github.event_name == 'pull_request' && github.event.pull_request.base.sha) || '' }} >> $GITHUB_OUTPUT
865+
working-directory: dev-packages/e2e-tests
866+
867+
- name: Determine which optional E2E test applications should be run
868+
id: matrix-optional
869+
run: yarn --silent ci:build-matrix-optional --base=${{ (github.event_name == 'pull_request' && github.event.pull_request.base.sha) || '' }} >> $GITHUB_OUTPUT
870+
working-directory: dev-packages/e2e-tests
871+
854872
job_e2e_tests:
855873
name: E2E ${{ matrix.label || matrix.test-application }} Test
856874
# We need to add the `always()` check here because the previous step has this as well :(
857875
# See: https://github.com/actions/runner/issues/2205
858-
if: always() && needs.job_e2e_prepare.result == 'success'
876+
if: always() && needs.job_e2e_prepare.result == 'success' && needs.job_e2e_prepare.outputs.matrix != '{"include":[]}'
859877
needs: [job_get_metadata, job_build, job_e2e_prepare]
860878
runs-on: ubuntu-20.04
861879
timeout-minutes: 15
@@ -870,105 +888,7 @@ jobs:
870888
E2E_TEST_SENTRY_PROJECT: 'sentry-javascript-e2e-tests'
871889
strategy:
872890
fail-fast: false
873-
matrix:
874-
is_dependabot:
875-
- ${{ github.actor == 'dependabot[bot]' }}
876-
test-application:
877-
[
878-
'angular-17',
879-
'angular-18',
880-
'astro-4',
881-
'aws-lambda-layer-cjs',
882-
'aws-serverless-esm',
883-
'node-express',
884-
'create-react-app',
885-
'create-next-app',
886-
'create-remix-app',
887-
'create-remix-app-legacy',
888-
'create-remix-app-v2',
889-
'create-remix-app-v2-legacy',
890-
'create-remix-app-express',
891-
'create-remix-app-express-legacy',
892-
'create-remix-app-express-vite-dev',
893-
'default-browser',
894-
'node-express-esm-loader',
895-
'node-express-esm-preload',
896-
'node-express-esm-without-loader',
897-
'node-express-cjs-preload',
898-
'node-otel-sdk-node',
899-
'node-otel-custom-sampler',
900-
'node-otel-without-tracing',
901-
'ember-classic',
902-
'ember-embroider',
903-
'nextjs-app-dir',
904-
'nextjs-13',
905-
'nextjs-14',
906-
'nextjs-15',
907-
'nextjs-turbo',
908-
'nextjs-t3',
909-
'react-17',
910-
'react-19',
911-
'react-create-hash-router',
912-
'react-router-6-use-routes',
913-
'react-router-5',
914-
'react-router-6',
915-
'solid',
916-
'solidstart',
917-
'solidstart-spa',
918-
'svelte-5',
919-
'sveltekit',
920-
'sveltekit-2',
921-
'sveltekit-2-svelte-5',
922-
'sveltekit-2-twp',
923-
'tanstack-router',
924-
'generic-ts3.8',
925-
'node-fastify',
926-
'node-fastify-5',
927-
'node-hapi',
928-
'node-nestjs-basic',
929-
'node-nestjs-distributed-tracing',
930-
'nestjs-basic',
931-
'nestjs-8',
932-
'nestjs-distributed-tracing',
933-
'nestjs-with-submodules',
934-
'nestjs-with-submodules-decorator',
935-
'nestjs-basic-with-graphql',
936-
'nestjs-graphql',
937-
'node-exports-test-app',
938-
'node-koa',
939-
'node-connect',
940-
'nuxt-3',
941-
'nuxt-3-min',
942-
'nuxt-4',
943-
'vue-3',
944-
'webpack-4',
945-
'webpack-5'
946-
]
947-
build-command:
948-
- false
949-
label:
950-
- false
951-
# Add any variations of a test app here
952-
# You should provide an alternate build-command as well as a matching label
953-
include:
954-
- test-application: 'create-react-app'
955-
build-command: 'test:build-ts3.8'
956-
label: 'create-react-app (TS 3.8)'
957-
- test-application: 'react-router-6'
958-
build-command: 'test:build-ts3.8'
959-
label: 'react-router-6 (TS 3.8)'
960-
- test-application: 'create-next-app'
961-
build-command: 'test:build-13'
962-
label: 'create-next-app (next@13)'
963-
- test-application: 'nextjs-app-dir'
964-
build-command: 'test:build-13'
965-
label: 'nextjs-app-dir (next@13)'
966-
exclude:
967-
- is_dependabot: true
968-
test-application: 'cloudflare-astro'
969-
- is_dependabot: true
970-
test-application: 'cloudflare-workers'
971-
891+
matrix: ${{ fromJson(needs.job_e2e_prepare.outputs.matrix) }}
972892
steps:
973893
- name: Check out current commit (${{ needs.job_get_metadata.outputs.commit_label }})
974894
uses: actions/checkout@v4
@@ -1069,6 +989,7 @@ jobs:
1069989
# See: https://github.com/actions/runner/issues/2205
1070990
if:
1071991
always() && needs.job_e2e_prepare.result == 'success' &&
992+
needs.job_e2e_prepare.outputs.matrix-optional != '{"include":[]}' &&
1072993
(github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository) &&
1073994
github.actor != 'dependabot[bot]'
1074995
needs: [job_get_metadata, job_build, job_e2e_prepare]
@@ -1085,58 +1006,7 @@ jobs:
10851006
E2E_TEST_SENTRY_PROJECT: 'sentry-javascript-e2e-tests'
10861007
strategy:
10871008
fail-fast: false
1088-
matrix:
1089-
test-application:
1090-
[
1091-
'cloudflare-astro',
1092-
'cloudflare-workers',
1093-
'react-send-to-sentry',
1094-
'node-express-send-to-sentry',
1095-
'debug-id-sourcemaps',
1096-
]
1097-
build-command:
1098-
- false
1099-
assert-command:
1100-
- false
1101-
label:
1102-
- false
1103-
include:
1104-
- test-application: 'create-remix-app'
1105-
assert-command: 'test:assert-sourcemaps'
1106-
label: 'create-remix-app (sourcemaps)'
1107-
- test-application: 'create-remix-app-legacy'
1108-
assert-command: 'test:assert-sourcemaps'
1109-
label: 'create-remix-app-legacy (sourcemaps)'
1110-
- test-application: 'nextjs-app-dir'
1111-
build-command: 'test:build-canary'
1112-
label: 'nextjs-app-dir (canary)'
1113-
- test-application: 'nextjs-app-dir'
1114-
build-command: 'test:build-latest'
1115-
label: 'nextjs-app-dir (latest)'
1116-
- test-application: 'nextjs-13'
1117-
build-command: 'test:build-canary'
1118-
label: 'nextjs-13 (canary)'
1119-
- test-application: 'nextjs-13'
1120-
build-command: 'test:build-latest'
1121-
label: 'nextjs-13 (latest)'
1122-
- test-application: 'nextjs-14'
1123-
build-command: 'test:build-canary'
1124-
label: 'nextjs-14 (canary)'
1125-
- test-application: 'nextjs-14'
1126-
build-command: 'test:build-latest'
1127-
label: 'nextjs-14 (latest)'
1128-
- test-application: 'nextjs-15'
1129-
build-command: 'test:build-canary'
1130-
label: 'nextjs-15 (canary)'
1131-
- test-application: 'nextjs-15'
1132-
build-command: 'test:build-latest'
1133-
label: 'nextjs-15 (latest)'
1134-
- test-application: 'nextjs-turbo'
1135-
build-command: 'test:build-canary'
1136-
label: 'nextjs-turbo (canary)'
1137-
- test-application: 'nextjs-turbo'
1138-
build-command: 'test:build-latest'
1139-
label: 'nextjs-turbo (latest)'
1009+
matrix: ${{ fromJson(needs.job_e2e_prepare.outputs.matrix-optional) }}
11401010

11411011
steps:
11421012
- name: Check out current commit (${{ needs.job_get_metadata.outputs.commit_label }})
+155
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
import { execSync } from 'child_process';
2+
import * as fs from 'fs';
3+
import * as path from 'path';
4+
import { dirname } from 'path';
5+
import { parseArgs } from 'util';
6+
import { sync as globSync } from 'glob';
7+
8+
interface MatrixInclude {
9+
/** The test application (directory) name. */
10+
'test-application': string;
11+
/** Optional override for the build command to run. */
12+
'build-command'?: string;
13+
/** Optional override for the assert command to run. */
14+
'assert-command'?: string;
15+
/** Optional label for the test run. If not set, defaults to value of `test-application`. */
16+
label?: string;
17+
}
18+
19+
interface PackageJsonSentryTestConfig {
20+
/** If this is true, the test app is optional. */
21+
optional?: boolean;
22+
/** Variant configs that should be run in non-optional test runs. */
23+
variants?: Partial<MatrixInclude>[];
24+
/** Variant configs that should be run in optional test runs. */
25+
optionalVariants?: Partial<MatrixInclude>[];
26+
/** Skip this test app for matrix generation. */
27+
skip?: boolean;
28+
}
29+
30+
/**
31+
* This methods generates a matrix for the GitHub Actions workflow to run the E2E tests.
32+
* It checks which test applications are affected by the current changes in the PR and then generates a matrix
33+
* including all test apps that have at least one dependency that was changed in the PR.
34+
* If no `--base=xxx` is provided, it will output all test applications.
35+
*
36+
* If `--optional=true` is set, it will generate a matrix of optional test applications only.
37+
* Otherwise, these will be skipped.
38+
*/
39+
function run(): void {
40+
const { values } = parseArgs({
41+
args: process.argv.slice(2),
42+
options: {
43+
base: { type: 'string' },
44+
head: { type: 'string' },
45+
optional: { type: 'string', default: 'false' },
46+
},
47+
});
48+
49+
const { base, head, optional } = values;
50+
51+
const testApplications = globSync('*/package.json', {
52+
cwd: `${__dirname}/../test-applications`,
53+
}).map(filePath => dirname(filePath));
54+
55+
// If `--base=xxx` is defined, we only want to get test applications changed since that base
56+
// Else, we take all test applications (e.g. on push)
57+
const includedTestApplications = base
58+
? getAffectedTestApplications(testApplications, { base, head })
59+
: testApplications;
60+
61+
const optionalMode = optional === 'true';
62+
const includes: MatrixInclude[] = [];
63+
64+
includedTestApplications.forEach(testApp => {
65+
addIncludesForTestApp(testApp, includes, { optionalMode });
66+
});
67+
68+
// We print this to the output, so the GHA can use it for the matrix
69+
// eslint-disable-next-line no-console
70+
console.log(`matrix=${JSON.stringify({ include: includes })}`);
71+
}
72+
73+
function addIncludesForTestApp(
74+
testApp: string,
75+
includes: MatrixInclude[],
76+
{ optionalMode }: { optionalMode: boolean },
77+
): void {
78+
const packageJson = getPackageJson(testApp);
79+
80+
const shouldSkip = packageJson.sentryTest?.skip || false;
81+
const isOptional = packageJson.sentryTest?.optional || false;
82+
const variants = (optionalMode ? packageJson.sentryTest?.optionalVariants : packageJson.sentryTest?.variants) || [];
83+
84+
if (shouldSkip) {
85+
return;
86+
}
87+
88+
// Add the basic test-application itself, if it is in the current mode
89+
if (optionalMode === isOptional) {
90+
includes.push({
91+
'test-application': testApp,
92+
});
93+
}
94+
95+
variants.forEach(variant => {
96+
includes.push({
97+
'test-application': testApp,
98+
...variant,
99+
});
100+
});
101+
}
102+
103+
function getSentryDependencies(appName: string): string[] {
104+
const packageJson = getPackageJson(appName) || {};
105+
106+
const dependencies = {
107+
...packageJson.devDependencies,
108+
...packageJson.dependencies,
109+
};
110+
111+
return Object.keys(dependencies).filter(key => key.startsWith('@sentry'));
112+
}
113+
114+
function getPackageJson(appName: string): {
115+
dependencies?: { [key: string]: string };
116+
devDependencies?: { [key: string]: string };
117+
sentryTest?: PackageJsonSentryTestConfig;
118+
} {
119+
const fullPath = path.resolve(__dirname, '..', 'test-applications', appName, 'package.json');
120+
121+
if (!fs.existsSync(fullPath)) {
122+
throw new Error(`Could not find package.json for ${appName}`);
123+
}
124+
125+
return JSON.parse(fs.readFileSync(fullPath, 'utf8'));
126+
}
127+
128+
run();
129+
130+
function getAffectedTestApplications(
131+
testApplications: string[],
132+
{ base = 'develop', head }: { base?: string; head?: string },
133+
): string[] {
134+
const additionalArgs = [`--base=${base}`];
135+
136+
if (head) {
137+
additionalArgs.push(`--head=${head}`);
138+
}
139+
140+
const affectedProjects = execSync(`yarn --silent nx show projects --affected ${additionalArgs.join(' ')}`)
141+
.toString()
142+
.split('\n')
143+
.map(line => line.trim())
144+
.filter(Boolean);
145+
146+
// If something in e2e tests themselves are changed, just run everything
147+
if (affectedProjects.includes('@sentry-internal/e2e-tests')) {
148+
return testApplications;
149+
}
150+
151+
return testApplications.filter(testApp => {
152+
const sentryDependencies = getSentryDependencies(testApp);
153+
return sentryDependencies.some(dep => affectedProjects.includes(dep));
154+
});
155+
}

dev-packages/e2e-tests/package.json

+3-1
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,13 @@
1414
"test:prepare": "ts-node prepare.ts",
1515
"test:validate": "run-s test:validate-configuration test:validate-test-app-setups",
1616
"clean": "rimraf tmp node_modules pnpm-lock.yaml && yarn clean:test-applications",
17+
"ci:build-matrix": "ts-node ./lib/getTestMatrix.ts",
18+
"ci:build-matrix-optional": "ts-node ./lib/getTestMatrix.ts --optional=true",
1719
"clean:test-applications": "rimraf --glob test-applications/**/{node_modules,dist,build,.next,.sveltekit,pnpm-lock.yaml} .last-run.json && pnpm store prune"
1820
},
1921
"devDependencies": {
2022
"@types/glob": "8.0.0",
21-
"@types/node": "^14.18.0",
23+
"@types/node": "^18.0.0",
2224
"dotenv": "16.0.3",
2325
"esbuild": "0.20.0",
2426
"glob": "8.0.3",

dev-packages/e2e-tests/test-applications/cloudflare-astro/package.json

+3
Original file line numberDiff line numberDiff line change
@@ -26,5 +26,8 @@
2626
},
2727
"volta": {
2828
"extends": "../../package.json"
29+
},
30+
"sentryTest": {
31+
"optional": true
2932
}
3033
}

0 commit comments

Comments
 (0)