Skip to content

Commit c71177b

Browse files
authored
ci: Improve CI dependency checks (#13175)
This PR updates the way we detect changed packages to rely on Nx under the hood, which should always be in sync. Previously, we hard-coded paths in the GH workflow to determine which packages have been changed, so we can make sure to run tests accordingly. Now, we use Nx to detect this for PRs - this takes the dependency graph into consideration and should always be up-to-date. We just need to make sure to have correct dependencies defined, also for dev packages like node-integration-tests (see addition I made there). Note: For profiling-node, we still check the old way, because we want to avoid re-running this every time a dependency of profiling-node changes - because that depends on e.g. core and utils, and we don't want to/need to re-run this all the time. This PR does two other things: 1. Enable global yarn cache - this may help us reduce install time on CI 2. Merge the install & build CI steps - these were run in parallel, which in reality only ate up about 50s, because this is how long it takes to restore the dependency cache, which had to happen in the build step. By merging this, min. time for install + build for a fully cached scenario is down to ~1:15 minutes, where previously it was >2 minutes across the two steps. Example runs: * Change in packages/browser: https://github.com/getsentry/sentry-javascript/actions/runs/10215948246 * Change in packages/core: https://github.com/getsentry/sentry-javascript/actions/runs/10215944443 * Change in packages/profiling-node: https://github.com/getsentry/sentry-javascript/actions/runs/10216003595
1 parent c1052ab commit c71177b

File tree

2 files changed

+67
-136
lines changed

2 files changed

+67
-136
lines changed

.github/workflows/build.yml

Lines changed: 65 additions & 136 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ jobs:
7373
# We need to check out not only the fake merge commit between the PR and the base branch which GH creates, but
7474
# also its parents, so that we can pull the commit message from the head commit of the PR
7575
fetch-depth: 2
76+
7677
- name: Get metadata
7778
id: get_metadata
7879
# We need to try a number of different options for finding the head commit, because each kind of trigger event
@@ -82,101 +83,32 @@ jobs:
8283
echo "COMMIT_SHA=$COMMIT_SHA" >> $GITHUB_ENV
8384
echo "COMMIT_MESSAGE=$(git log -n 1 --pretty=format:%s $COMMIT_SHA)" >> $GITHUB_ENV
8485
86+
# Most changed packages are determined in job_build via Nx
87+
# However, for profiling-node we only want to run certain things when in this specific package
88+
# something changed, not in any of the dependencies (which include core, utils, ...)
8589
- name: Determine changed packages
8690
uses: dorny/[email protected]
8791
id: changed
8892
with:
8993
filters: |
90-
workflow: &workflow
94+
workflow:
9195
- '.github/**'
92-
shared: &shared
93-
- *workflow
94-
- '*.{js,ts,json,yml,lock}'
95-
- 'CHANGELOG.md'
96-
- 'jest/**'
97-
- 'scripts/**'
98-
- 'packages/core/**'
99-
- 'packages/rollup-utils/**'
100-
- 'packages/utils/**'
101-
- 'packages/types/**'
102-
- 'dev-packages/test-utils/**'
103-
browser: &browser
104-
- *shared
105-
- 'packages/browser/**'
106-
- 'packages/browser-utils/**'
107-
- 'packages/replay-internal/**'
108-
- 'packages/replay-worker/**'
109-
- 'packages/replay-canvas/**'
110-
- 'packages/feedback/**'
111-
- 'packages/wasm/**'
112-
node: &node
113-
- *shared
114-
- 'packages/node/**'
115-
- 'packages/opentelemetry/**'
116-
browser_integration:
117-
- *shared
118-
- *browser
119-
- 'dev-packages/browser-integration-tests/**'
120-
ember:
121-
- *shared
122-
- *browser
123-
- 'packages/ember/**'
124-
node_integration:
125-
- *shared
126-
- *node
127-
- 'dev-packages/node-integration-tests/**'
128-
- 'packages/nestjs/**'
129-
nextjs:
130-
- *shared
131-
- *browser
132-
- *node
133-
- 'packages/nextjs/**'
134-
- 'packages/react/**'
135-
- 'packages/vercel-edge/**'
136-
remix:
137-
- *shared
138-
- *browser
139-
- *node
140-
- 'packages/remix/**'
141-
- 'packages/react/**'
14296
profiling_node:
143-
- *shared
144-
- 'packages/node/**'
145-
- 'packages/profiling-node/**'
146-
- 'dev-packages/e2e-tests/test-applications/node-profiling/**'
147-
profiling_node_bindings:
14897
- 'packages/profiling-node/**'
14998
- 'dev-packages/e2e-tests/test-applications/node-profiling/**'
150-
deno:
151-
- *shared
152-
- 'packages/deno/**'
153-
bun:
154-
- *shared
155-
- 'packages/bun/**'
156-
any_code:
157-
- '!**/*.md'
99+
158100
159101
- name: Get PR labels
160102
id: pr-labels
161103
uses: mydea/pr-labels-action@fn/bump-node20
162104

163105
outputs:
164106
commit_label: '${{ env.COMMIT_SHA }}: ${{ env.COMMIT_MESSAGE }}'
165-
changed_nextjs: ${{ steps.changed.outputs.nextjs }}
166-
changed_ember: ${{ steps.changed.outputs.ember }}
167-
changed_remix: ${{ steps.changed.outputs.remix }}
168-
changed_node: ${{ steps.changed.outputs.node }}
169-
changed_node_integration: ${{ steps.changed.outputs.node_integration }}
170-
changed_profiling_node: ${{ steps.changed.outputs.profiling_node }}
171-
changed_profiling_node_bindings: ${{ steps.changed.outputs.profiling_node_bindings }}
172-
changed_deno: ${{ steps.changed.outputs.deno }}
173-
changed_bun: ${{ steps.changed.outputs.bun }}
174-
changed_browser: ${{ steps.changed.outputs.browser }}
175-
changed_browser_integration: ${{ steps.changed.outputs.browser_integration }}
176-
changed_any_code: ${{ steps.changed.outputs.any_code }}
177107
# Note: These next three have to be checked as strings ('true'/'false')!
178108
is_develop: ${{ github.ref == 'refs/heads/develop' }}
179109
is_release: ${{ startsWith(github.ref, 'refs/heads/release/') }}
110+
changed_profiling_node: ${{ steps.changed.outputs.profiling_node == 'true' }}
111+
changed_ci: ${{ steps.changed.outputs.workflow == 'true' }}
180112
# When merging into master, or from master
181113
is_gitflow_sync: ${{ github.head_ref == 'master' || github.ref == 'refs/heads/master' }}
182114
has_gitflow_label:
@@ -185,22 +117,30 @@ jobs:
185117
${{ github.event_name == 'schedule' || (github.event_name == 'pull_request' &&
186118
contains(steps.pr-labels.outputs.labels, ' ci-skip-cache ')) }}
187119

188-
job_install_deps:
189-
name: Install Dependencies
120+
job_build:
121+
name: Build
190122
needs: job_get_metadata
191123
runs-on: ubuntu-20.04
192124
timeout-minutes: 15
193125
if: |
194126
(needs.job_get_metadata.outputs.is_gitflow_sync == 'false' && needs.job_get_metadata.outputs.has_gitflow_label == 'false')
195127
steps:
128+
- name: Check out base commit (${{ github.event.pull_request.base.sha }})
129+
uses: actions/checkout@v4
130+
if: github.event_name == 'pull_request'
131+
with:
132+
ref: ${{ github.event.pull_request.base.sha }}
133+
196134
- name: 'Check out current commit (${{ needs.job_get_metadata.outputs.commit_label }})'
197135
uses: actions/checkout@v4
198136
with:
199137
ref: ${{ env.HEAD_COMMIT }}
138+
200139
- name: Set up Node
201140
uses: actions/setup-node@v4
202141
with:
203142
node-version-file: 'package.json'
143+
204144
# we use a hash of yarn.lock as our cache key, because if it hasn't changed, our dependencies haven't changed,
205145
# so no need to reinstall them
206146
- name: Compute dependency cache key
@@ -217,46 +157,14 @@ jobs:
217157
- name: Install dependencies
218158
if: steps.cache_dependencies.outputs.cache-hit != 'true'
219159
run: yarn install --ignore-engines --frozen-lockfile
220-
outputs:
221-
dependency_cache_key: ${{ steps.compute_lockfile_hash.outputs.hash }}
222-
223-
job_check_branches:
224-
name: Check PR branches
225-
needs: job_get_metadata
226-
runs-on: ubuntu-20.04
227-
if: github.event_name == 'pull_request'
228-
permissions:
229-
pull-requests: write
230-
steps:
231-
- name: PR is opened against master
232-
uses: mshick/add-pr-comment@dd126dd8c253650d181ad9538d8b4fa218fc31e8
233-
if: ${{ github.base_ref == 'master' && !startsWith(github.head_ref, 'prepare-release/') }}
234-
with:
235-
message: |
236-
⚠️ This PR is opened against **master**. You probably want to open it against **develop**.
237160

238-
job_build:
239-
name: Build
240-
needs: [job_get_metadata, job_install_deps]
241-
runs-on: ubuntu-20.04-large-js
242-
timeout-minutes: 30
243-
if: |
244-
(needs.job_get_metadata.outputs.changed_any_code == 'true' || github.event_name != 'pull_request')
245-
steps:
246-
- name: Check out current commit (${{ needs.job_get_metadata.outputs.commit_label }})
247-
uses: actions/checkout@v4
248-
with:
249-
ref: ${{ env.HEAD_COMMIT }}
250-
- name: Set up Node
251-
uses: actions/setup-node@v4
252-
with:
253-
node-version-file: 'package.json'
254-
- name: Check dependency cache
255-
uses: actions/cache/restore@v4
161+
- name: Check for Affected Nx Projects
162+
uses: dkhunt27/[email protected]
163+
id: checkForAffected
164+
if: github.event_name == 'pull_request'
256165
with:
257-
path: ${{ env.CACHED_DEPENDENCY_PATHS }}
258-
key: ${{ needs.job_install_deps.outputs.dependency_cache_key }}
259-
fail-on-cache-miss: true
166+
base: ${{ github.event.pull_request.base.sha }}
167+
head: ${{ env.HEAD_COMMIT }}
260168

261169
- name: Check build cache
262170
uses: actions/cache@v4
@@ -285,10 +193,31 @@ jobs:
285193
env:
286194
CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
287195
run: yarn build
196+
288197
outputs:
289-
# this needs to be passed on, because the `needs` context only looks at direct ancestors (so steps which depend on
290-
# `job_build` can't see `job_install_deps` and what it returned)
291-
dependency_cache_key: ${{ needs.job_install_deps.outputs.dependency_cache_key }}
198+
dependency_cache_key: ${{ steps.compute_lockfile_hash.outputs.hash }}
199+
changed_node_integration: ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, '@sentry-internal/node-integration-tests') }}
200+
changed_remix: ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, '@sentry/remix') }}
201+
changed_node: ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, '@sentry/node') }}
202+
changed_deno: ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, '@sentry/deno') }}
203+
changed_bun: ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, '@sentry/bun') }}
204+
changed_browser_integration: ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, '@sentry-internal/browser-integration-tests') }}
205+
# If you are looking for changed_profiling_node, this is defined in job_get_metadata
206+
207+
job_check_branches:
208+
name: Check PR branches
209+
needs: job_get_metadata
210+
runs-on: ubuntu-20.04
211+
if: github.event_name == 'pull_request'
212+
permissions:
213+
pull-requests: write
214+
steps:
215+
- name: PR is opened against master
216+
uses: mshick/add-pr-comment@dd126dd8c253650d181ad9538d8b4fa218fc31e8
217+
if: ${{ github.base_ref == 'master' && !startsWith(github.head_ref, 'prepare-release/') }}
218+
with:
219+
message: |
220+
⚠️ This PR is opened against **master**. You probably want to open it against **develop**.
292221
293222
job_size_check:
294223
name: Size Check
@@ -345,7 +274,7 @@ jobs:
345274

346275
job_check_format:
347276
name: Check file formatting
348-
needs: [job_get_metadata, job_install_deps]
277+
needs: [job_get_metadata, job_build]
349278
timeout-minutes: 10
350279
runs-on: ubuntu-20.04
351280
steps:
@@ -361,7 +290,7 @@ jobs:
361290
uses: actions/cache/restore@v4
362291
with:
363292
path: ${{ env.CACHED_DEPENDENCY_PATHS }}
364-
key: ${{ needs.job_install_deps.outputs.dependency_cache_key }}
293+
key: ${{ needs.job_build.outputs.dependency_cache_key }}
365294
fail-on-cache-miss: true
366295
- name: Check file formatting
367296
run: yarn lint:prettier && yarn lint:biome
@@ -437,6 +366,7 @@ jobs:
437366
steps:
438367
- name: Check out base commit (${{ github.event.pull_request.base.sha }})
439368
uses: actions/checkout@v4
369+
if: github.event_name == 'pull_request'
440370
with:
441371
ref: ${{ github.event.pull_request.base.sha }}
442372

@@ -469,7 +399,7 @@ jobs:
469399
job_bun_unit_tests:
470400
name: Bun Unit Tests
471401
needs: [job_get_metadata, job_build]
472-
if: needs.job_get_metadata.outputs.changed_bun == 'true' || github.event_name != 'pull_request'
402+
if: needs.job_build.outputs.changed_bun == 'true' || github.event_name != 'pull_request'
473403
timeout-minutes: 10
474404
runs-on: ubuntu-20.04
475405
strategy:
@@ -496,7 +426,7 @@ jobs:
496426
job_deno_unit_tests:
497427
name: Deno Unit Tests
498428
needs: [job_get_metadata, job_build]
499-
if: needs.job_get_metadata.outputs.changed_deno == 'true' || github.event_name != 'pull_request'
429+
if: needs.job_build.outputs.changed_deno == 'true' || github.event_name != 'pull_request'
500430
timeout-minutes: 10
501431
runs-on: ubuntu-20.04
502432
strategy:
@@ -536,6 +466,7 @@ jobs:
536466
steps:
537467
- name: Check out base commit (${{ github.event.pull_request.base.sha }})
538468
uses: actions/checkout@v4
469+
if: github.event_name == 'pull_request'
539470
with:
540471
ref: ${{ github.event.pull_request.base.sha }}
541472
- name: Check out current commit (${{ needs.job_get_metadata.outputs.commit_label }})
@@ -571,7 +502,7 @@ jobs:
571502
job_profiling_node_unit_tests:
572503
name: Node Profiling Unit Tests
573504
needs: [job_get_metadata, job_build]
574-
if: needs.job_get_metadata.outputs.changed_node == 'true' || needs.job_get_metadata.outputs.changed_profiling_node == 'true' || github.event_name != 'pull_request'
505+
if: needs.job_build.outputs.changed_node == 'true' || needs.job_get_metadata.outputs.changed_profiling_node == 'true' || github.event_name != 'pull_request'
575506
runs-on: ubuntu-latest
576507
timeout-minutes: 10
577508
steps:
@@ -599,7 +530,7 @@ jobs:
599530
job_browser_playwright_tests:
600531
name: Playwright (${{ matrix.bundle }}${{ matrix.shard && format(' {0}/{1}', matrix.shard, matrix.shards) || ''}}) Tests
601532
needs: [job_get_metadata, job_build]
602-
if: needs.job_get_metadata.outputs.changed_browser_integration == 'true' || github.event_name != 'pull_request'
533+
if: needs.job_build.outputs.changed_browser_integration == 'true' || github.event_name != 'pull_request'
603534
runs-on: ubuntu-20.04-large-js
604535
timeout-minutes: 25
605536
strategy:
@@ -674,11 +605,10 @@ jobs:
674605
name: playwright-traces
675606
path: dev-packages/browser-integration-tests/test-results
676607

677-
678608
job_browser_loader_tests:
679609
name: Playwright Loader (${{ matrix.bundle }}) Tests
680610
needs: [job_get_metadata, job_build]
681-
if: needs.job_get_metadata.outputs.changed_browser_integration == 'true' || github.event_name != 'pull_request'
611+
if: needs.job_build.outputs.changed_browser_integration == 'true' || github.event_name != 'pull_request'
682612
runs-on: ubuntu-20.04
683613
timeout-minutes: 15
684614
strategy:
@@ -748,13 +678,12 @@ jobs:
748678
exit 1
749679
fi
750680
751-
752681
job_node_integration_tests:
753682
name:
754683
Node (${{ matrix.node }})${{ (matrix.typescript && format(' (TS {0})', matrix.typescript)) || '' }} Integration
755684
Tests
756685
needs: [job_get_metadata, job_build]
757-
if: needs.job_get_metadata.outputs.changed_node_integration == 'true' || github.event_name != 'pull_request'
686+
if: needs.job_build.outputs.changed_node_integration == 'true' || github.event_name != 'pull_request'
758687
runs-on: ubuntu-20.04
759688
timeout-minutes: 15
760689
strategy:
@@ -796,7 +725,7 @@ jobs:
796725
job_remix_integration_tests:
797726
name: Remix v${{ matrix.remix }} (Node ${{ matrix.node }}) Tests
798727
needs: [job_get_metadata, job_build]
799-
if: needs.job_get_metadata.outputs.changed_remix == 'true' || github.event_name != 'pull_request'
728+
if: needs.job_build.outputs.changed_remix == 'true' || github.event_name != 'pull_request'
800729
runs-on: ubuntu-20.04
801730
timeout-minutes: 10
802731
strategy:
@@ -866,14 +795,14 @@ jobs:
866795
# Rebuild profiling by compiling TS and pull the precompiled binary artifacts
867796
- name: Build Profiling Node
868797
if: |
869-
(needs.job_get_metadata.outputs.changed_profiling_node_bindings == 'true') ||
798+
(needs.job_get_metadata.outputs.changed_profiling_node == 'true') ||
870799
(needs.job_get_metadata.outputs.is_release == 'true') ||
871800
(github.event_name != 'pull_request')
872801
run: yarn lerna run build:lib --scope @sentry/profiling-node
873802

874803
- name: Extract Profiling Node Prebuilt Binaries
875804
if: |
876-
(needs.job_get_metadata.outputs.changed_profiling_node_bindings == 'true') ||
805+
(needs.job_get_metadata.outputs.changed_profiling_node == 'true') ||
877806
(needs.job_get_metadata.outputs.is_release == 'true') ||
878807
(github.event_name != 'pull_request')
879808
uses: actions/download-artifact@v4
@@ -1167,7 +1096,7 @@ jobs:
11671096
always() && needs.job_e2e_prepare.result == 'success' &&
11681097
(github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository) &&
11691098
(
1170-
(needs.job_get_metadata.outputs.changed_profiling_node_bindings == 'true') ||
1099+
(needs.job_get_metadata.outputs.changed_profiling_node == 'true') ||
11711100
(needs.job_get_metadata.outputs.is_release == 'true') ||
11721101
(github.event_name != 'pull_request')
11731102
)
@@ -1320,11 +1249,11 @@ jobs:
13201249

13211250
job_compile_bindings_profiling_node:
13221251
name: Compile & Test Profiling Bindings (v${{ matrix.node }}) ${{ matrix.target_platform || matrix.os }}, ${{ matrix.node || matrix.container }}, ${{ matrix.arch || matrix.container }}, ${{ contains(matrix.container, 'alpine') && 'musl' || 'glibc' }}
1323-
needs: [job_get_metadata, job_install_deps, job_build]
1252+
needs: [job_get_metadata, job_build]
13241253
# Compiling bindings can be very slow (especially on windows), so only run precompile
13251254
# Skip precompile unless we are on a release branch as precompile slows down CI times.
13261255
if: |
1327-
(needs.job_get_metadata.outputs.changed_profiling_node_bindings == 'true') ||
1256+
(needs.job_get_metadata.outputs.changed_profiling_node == 'true') ||
13281257
(needs.job_get_metadata.outputs.is_release == 'true') ||
13291258
(github.event_name != 'pull_request')
13301259
runs-on: ${{ matrix.os }}
@@ -1481,7 +1410,7 @@ jobs:
14811410
id: restore-dependencies
14821411
with:
14831412
path: ${{ env.CACHED_DEPENDENCY_PATHS }}
1484-
key: ${{ needs.job_install_deps.outputs.dependency_cache_key }}
1413+
key: ${{ needs.job_build.outputs.dependency_cache_key }}
14851414
enableCrossOsArchive: true
14861415

14871416
- name: Restore build cache

0 commit comments

Comments
 (0)