From 79946949c7f889d5c729d05fb0c2c5dbbf2e5159 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josh=20Goldberg=20=E2=9C=A8?= Date: Thu, 19 Mar 2026 08:52:40 -0400 Subject: [PATCH 1/4] chore: add --changedSince as MERGE_BASE to Jest in PR CI Made-with: Cursor --- .github/workflows/frontend.yml | 25 ++++++++++++++++ eslint.config.ts | 1 + jest.config.ts | 30 +++++++++++++++---- tests/js/sentry-test/jest-environment.js | 28 +++++++++++++++++ .../sentry-test/sentry-jest-environment.d.ts | 7 +++++ 5 files changed, 86 insertions(+), 5 deletions(-) create mode 100644 tests/js/sentry-test/jest-environment.js create mode 100644 tests/js/sentry-test/sentry-jest-environment.d.ts diff --git a/.github/workflows/frontend.yml b/.github/workflows/frontend.yml index c0cf8b6737f854..6aeb2cbfc8e7e1 100644 --- a/.github/workflows/frontend.yml +++ b/.github/workflows/frontend.yml @@ -127,6 +127,10 @@ jobs: steps: - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7 name: Checkout sentry + with: + # PRs need history so we can compute merge base for Jest --changedSince. + # 100 is an arbitrary depth that will get most reasonable PRs' commits. + fetch-depth: ${{ github.event_name == 'pull_request' && '100' || '1' }} - uses: ./.github/actions/setup-node-pnpm @@ -143,6 +147,26 @@ jobs: search_artifacts: true # Search for the last workflow run whose stored the artifact we're looking for if_no_artifact_found: warn # Can be one of: "fail", "warn", "ignore" + # On PRs, HEAD is the merge commit; its parents (HEAD^1, HEAD^2) are base and head. + # Merge base of those two is what Jest --changedSince needs. Checkout uses fetch-depth: 100 for PRs. + - name: Get merge base for changedSince (PRs only) + id: merge_base + if: github.event_name == 'pull_request' + run: | + MERGE_BASE=$(git merge-base HEAD^1 HEAD^2 2>/dev/null) || true + if [ -n "$MERGE_BASE" ]; then + CHANGED=$(git diff --name-only "$MERGE_BASE" HEAD) + if echo "$CHANGED" | grep -qv '/'; then + echo "Root-level file changed — running full test suite" + MERGE_BASE="" + else + echo "Merge base: $MERGE_BASE (Jest will use --changedSince)" + fi + else + echo "Merge base: could not compute, will run full test suite" + fi + echo "merge_base=${MERGE_BASE:-}" >> "$GITHUB_OUTPUT" + - name: jest env: GITHUB_PR_SHA: ${{ github.event.pull_request.head.sha || github.sha }} @@ -158,6 +182,7 @@ jobs: # # This quiets up the logs quite a bit. DEBUG_PRINT_LIMIT: 0 + MERGE_BASE: ${{ steps.merge_base.outputs.merge_base }} run: pnpm run test-ci --forceExit form-field-registry: diff --git a/eslint.config.ts b/eslint.config.ts index 3058c903290e26..d632d8b2b6da84 100644 --- a/eslint.config.ts +++ b/eslint.config.ts @@ -928,6 +928,7 @@ export default typescript.config([ name: 'files/jest related', files: [ 'tests/js/jest-pegjs-transform.js', + 'tests/js/sentry-test/jest-environment.js', 'tests/js/sentry-test/mocks/*', 'tests/js/sentry-test/loadFixtures.ts', 'tests/js/setup.ts', diff --git a/jest.config.ts b/jest.config.ts index b859f88264e964..04d12ff49b3685 100644 --- a/jest.config.ts +++ b/jest.config.ts @@ -69,7 +69,18 @@ let JEST_TESTS: string[] | undefined; // to reexec itself here if (CI && !process.env.JEST_LIST_TESTS_INNER) { try { - const stdout = execFileSync('pnpm', ['exec', 'jest', '--listTests', '--json'], { + const listTestArguments = ['exec', 'jest', '--listTests', '--json']; + + if (process.env.MERGE_BASE) { + console.log('MERGE_BASE detected:', process.env.MERGE_BASE); + listTestArguments.push( + '--changedSince', + process.env.MERGE_BASE, + '--passWithNoTests' + ); + } + + const stdout = execFileSync('pnpm', listTestArguments, { stdio: 'pipe', encoding: 'utf-8', env: {...process.env, JEST_LIST_TESTS_INNER: '1'}, @@ -108,6 +119,10 @@ function getTestsForGroup( allTests: ReadonlyArray, testStats: Record ): string[] { + if (allTests.length === 0) { + return []; + } + const speculatedSuiteDuration = Object.values(testStats).reduce((a, b) => a + b, 0); const targetDuration = speculatedSuiteDuration / nodeTotal; @@ -122,8 +137,13 @@ function getTestsForGroup( const tests = new Map(); const SUITE_P50_DURATION_MS = 1500; + const allTestsSet = new Set(allTests); + // First, iterate over all of the tests we have stats for. Object.entries(testStats).forEach(([test, duration]) => { + if (!allTestsSet.has(test)) { + return; + } if (duration <= 0) { throw new Error(`Test duration is <= 0 for ${test}`); } @@ -199,8 +219,8 @@ function getTestsForGroup( } } - if (!groups[nodeIndex]) { - throw new Error(`No tests found for node ${nodeIndex}`); + if (!groups[nodeIndex]?.length) { + return ['/__no_tests_for_this_shard__']; } return groups[nodeIndex].map(test => `/${test}`); } @@ -285,6 +305,7 @@ const config: Config.InitialOptions = { // window/cookies state. '@sentry/toolbar': '/tests/js/sentry-test/mocks/sentryToolbarMock.js', }, + passWithNoTests: !!process.env.MERGE_BASE, setupFiles: [ '/static/app/utils/silence-react-unsafe-warnings.ts', 'jest-canvas-mock', @@ -333,8 +354,7 @@ const config: Config.InitialOptions = { */ clearMocks: true, - // To disable the sentry jest integration, set this to 'jsdom' - testEnvironment: '@sentry/jest-environment/jsdom', + testEnvironment: '/tests/js/sentry-test/jest-environment.js', testEnvironmentOptions: { globalsCleanup: 'on', sentryConfig: { diff --git a/tests/js/sentry-test/jest-environment.js b/tests/js/sentry-test/jest-environment.js new file mode 100644 index 00000000000000..93083f2ae59491 --- /dev/null +++ b/tests/js/sentry-test/jest-environment.js @@ -0,0 +1,28 @@ +const SentryEnvironment = require('@sentry/jest-environment/jsdom'); + +// @sentry/jest-environment mutates config.projectConfig.testEnvironmentOptions +// .sentryConfig.init in-place (pushing integrations and calling Sentry.init). +// When Jest runs in-band (≤1 test, e.g. via --changedSince), those mutations +// create circular references that crash ScriptTransformer's config serialisation. +// Deep-cloning sentryConfig isolates the mutation from the original config object. +class SafeSentryEnvironment extends SentryEnvironment { + /** @param {import('@jest/environment').JestEnvironmentConfig} config @param {import('@jest/environment').EnvironmentContext} context */ + constructor(config, context) { + const sentryConfig = config.projectConfig.testEnvironmentOptions?.sentryConfig; + if (sentryConfig) { + config = { + ...config, + projectConfig: { + ...config.projectConfig, + testEnvironmentOptions: { + ...config.projectConfig.testEnvironmentOptions, + sentryConfig: structuredClone(sentryConfig), + }, + }, + }; + } + super(config, context); + } +} + +module.exports = SafeSentryEnvironment; diff --git a/tests/js/sentry-test/sentry-jest-environment.d.ts b/tests/js/sentry-test/sentry-jest-environment.d.ts new file mode 100644 index 00000000000000..7ebeac108507d8 --- /dev/null +++ b/tests/js/sentry-test/sentry-jest-environment.d.ts @@ -0,0 +1,7 @@ +declare module '@sentry/jest-environment/jsdom' { + // eslint-disable-next-line import/no-extraneous-dependencies -- transitive dep of jest + import type {JestEnvironment} from '@jest/environment'; + + const SentryEnvironment: typeof JestEnvironment; + export = SentryEnvironment; +} From b5c9931818799242d3151fdab7d49a500b7ace14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josh=20Goldberg=20=E2=9C=A8?= Date: Wed, 25 Mar 2026 12:37:20 -0400 Subject: [PATCH 2/4] ref: moved to frontend-optional workflow --- .github/workflows/frontend-optional.yml | 78 +++++++++++++++++++++++++ .github/workflows/frontend.yml | 25 -------- 2 files changed, 78 insertions(+), 25 deletions(-) diff --git a/.github/workflows/frontend-optional.yml b/.github/workflows/frontend-optional.yml index 24b1ed2f46a2b5..63272ffeb8146a 100644 --- a/.github/workflows/frontend-optional.yml +++ b/.github/workflows/frontend-optional.yml @@ -36,6 +36,84 @@ jobs: filters: .github/file-filters.yml list-files: shell + frontend-jest-tests-changed-only: + if: needs.files-changed.outputs.testable_rules_changed == 'true' || needs.files-changed.outputs.testable_modified == 'true' + needs: [files-changed] + name: Jest + # If you change the runs-on image, you must also change the runner in jest-balance.yml + # so that the balancer runs in the same environment as the tests. + runs-on: ubuntu-24.04 + timeout-minutes: 30 + strategy: + # This helps not having to run multiple jobs because one fails, thus, reducing resource usage + # and reducing the risk that one of many runs would turn red again (read: intermittent tests) + fail-fast: false + matrix: + # XXX: When updating this, make sure you also update CI_NODE_TOTAL. + + instance: [0, 1, 2, 3] + + steps: + - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7 + name: Checkout sentry + with: + # PRs need history so we can compute merge base for Jest --changedSince. + # 100 is an arbitrary depth that will get most reasonable PRs' commits. + fetch-depth: ${{ github.event_name == 'pull_request' && '100' || '1' }} + + - uses: ./.github/actions/setup-node-pnpm + + - name: Download jest-balance.json + id: download-artifact + uses: dawidd6/action-download-artifact@ac66b43f0e6a346234dd65d4d0c8fbb31cb316e5 # v11 + with: + workflow: 38531594 # jest-balancer.yml + workflow_conclusion: success # The conclusion of the workflow we're looking for + branch: master # The branch we're looking for + name: jest-balance.json # Artifact name + name_is_regexp: false + path: tests/js/test-balancer/ # Directory where to extract artifact(s), defaults to the current directory + search_artifacts: true # Search for the last workflow run whose stored the artifact we're looking for + if_no_artifact_found: warn # Can be one of: "fail", "warn", "ignore" + + # On PRs, HEAD is the merge commit; its parents (HEAD^1, HEAD^2) are base and head. + # Merge base of those two is what Jest --changedSince needs. Checkout uses fetch-depth: 100 for PRs. + - name: Get merge base for changedSince (PRs only) + id: merge_base + if: github.event_name == 'pull_request' + run: | + MERGE_BASE=$(git merge-base HEAD^1 HEAD^2 2>/dev/null) || true + if [ -n "$MERGE_BASE" ]; then + CHANGED=$(git diff --name-only "$MERGE_BASE" HEAD) + if echo "$CHANGED" | grep -qvE '^static/'; then + echo "Non-frontend file changed — running full test suite" + MERGE_BASE="" + else + echo "Merge base: $MERGE_BASE (Jest will use --changedSince)" + fi + else + echo "Merge base: could not compute, will run full test suite" + fi + echo "merge_base=${MERGE_BASE:-}" >> "$GITHUB_OUTPUT" + + - name: jest + env: + GITHUB_PR_SHA: ${{ github.event.pull_request.head.sha || github.sha }} + GITHUB_PR_REF: ${{ github.event.pull_request.head.ref || github.ref }} + # XXX: CI_NODE_TOTAL must be hardcoded to the length of strategy.matrix.instance. + # Otherwise, if there are other things in the matrix, using strategy.job-total + # wouldn't be correct. + CI_NODE_TOTAL: 4 + CI_NODE_INDEX: ${{ matrix.instance }} + # Disable testing-library from printing out any of of the DOM to + # stdout. No one actually looks through this in CI, they're just + # going to run it locally. + # + # This quiets up the logs quite a bit. + DEBUG_PRINT_LIMIT: 0 + MERGE_BASE: ${{ steps.merge_base.outputs.merge_base }} + run: pnpm run test-ci --forceExit + typescript-native: if: needs.files-changed.outputs.frontend_all == 'true' needs: files-changed diff --git a/.github/workflows/frontend.yml b/.github/workflows/frontend.yml index 6aeb2cbfc8e7e1..c0cf8b6737f854 100644 --- a/.github/workflows/frontend.yml +++ b/.github/workflows/frontend.yml @@ -127,10 +127,6 @@ jobs: steps: - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7 name: Checkout sentry - with: - # PRs need history so we can compute merge base for Jest --changedSince. - # 100 is an arbitrary depth that will get most reasonable PRs' commits. - fetch-depth: ${{ github.event_name == 'pull_request' && '100' || '1' }} - uses: ./.github/actions/setup-node-pnpm @@ -147,26 +143,6 @@ jobs: search_artifacts: true # Search for the last workflow run whose stored the artifact we're looking for if_no_artifact_found: warn # Can be one of: "fail", "warn", "ignore" - # On PRs, HEAD is the merge commit; its parents (HEAD^1, HEAD^2) are base and head. - # Merge base of those two is what Jest --changedSince needs. Checkout uses fetch-depth: 100 for PRs. - - name: Get merge base for changedSince (PRs only) - id: merge_base - if: github.event_name == 'pull_request' - run: | - MERGE_BASE=$(git merge-base HEAD^1 HEAD^2 2>/dev/null) || true - if [ -n "$MERGE_BASE" ]; then - CHANGED=$(git diff --name-only "$MERGE_BASE" HEAD) - if echo "$CHANGED" | grep -qv '/'; then - echo "Root-level file changed — running full test suite" - MERGE_BASE="" - else - echo "Merge base: $MERGE_BASE (Jest will use --changedSince)" - fi - else - echo "Merge base: could not compute, will run full test suite" - fi - echo "merge_base=${MERGE_BASE:-}" >> "$GITHUB_OUTPUT" - - name: jest env: GITHUB_PR_SHA: ${{ github.event.pull_request.head.sha || github.sha }} @@ -182,7 +158,6 @@ jobs: # # This quiets up the logs quite a bit. DEBUG_PRINT_LIMIT: 0 - MERGE_BASE: ${{ steps.merge_base.outputs.merge_base }} run: pnpm run test-ci --forceExit form-field-registry: From 7e2b9e258e16821f0c71cc5462d2e89afd28da65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josh=20Goldberg=20=E2=9C=A8?= Date: Wed, 25 Mar 2026 12:54:20 -0400 Subject: [PATCH 3/4] fix(ci): Compare diff against PR head instead of merge commit The git diff for the non-frontend file guard was comparing against HEAD (the synthetic merge commit) instead of HEAD^2 (the PR branch tip). This included base branch changes in the diff, causing false positives that unnecessarily forced full test runs. Made-with: Cursor --- .github/workflows/frontend-optional.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/frontend-optional.yml b/.github/workflows/frontend-optional.yml index 63272ffeb8146a..8e1e5b72df749a 100644 --- a/.github/workflows/frontend-optional.yml +++ b/.github/workflows/frontend-optional.yml @@ -84,7 +84,7 @@ jobs: run: | MERGE_BASE=$(git merge-base HEAD^1 HEAD^2 2>/dev/null) || true if [ -n "$MERGE_BASE" ]; then - CHANGED=$(git diff --name-only "$MERGE_BASE" HEAD) + CHANGED=$(git diff --name-only "$MERGE_BASE" HEAD^2) if echo "$CHANGED" | grep -qvE '^static/'; then echo "Non-frontend file changed — running full test suite" MERGE_BASE="" From 5c9ae7e0c29bff62abf578dc144139ed3c4cf5aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josh=20Goldberg=20=E2=9C=A8?= Date: Thu, 26 Mar 2026 15:03:24 -0400 Subject: [PATCH 4/4] fix: touchups to frontend-optional.yml; don't run if no merge base --- .github/workflows/frontend-optional.yml | 53 ++++++++++++++----------- 1 file changed, 30 insertions(+), 23 deletions(-) diff --git a/.github/workflows/frontend-optional.yml b/.github/workflows/frontend-optional.yml index 8e1e5b72df749a..e018750130c229 100644 --- a/.github/workflows/frontend-optional.yml +++ b/.github/workflows/frontend-optional.yml @@ -25,8 +25,11 @@ jobs: testable_rules_changed: ${{ steps.changes.outputs.testable_rules_changed }} typecheckable_rules_changed: ${{ steps.changes.outputs.typecheckable_rules_changed }} frontend_all: ${{ steps.changes.outputs.frontend_all }} + merge_base: ${{ steps.merge_base.outputs.merge_base }} steps: - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7 + with: + fetch-depth: 100 - name: Check for frontend file changes uses: dorny/paths-filter@0bc4621a3135347011ad047f9ecf449bf72ce2bd # v3.0.0 @@ -36,8 +39,33 @@ jobs: filters: .github/file-filters.yml list-files: shell + # On PRs, HEAD is the merge commit; its parents (HEAD^1, HEAD^2) are base and head. + # Merge base of those two is what Jest --changedSince needs. + # If merge base can't be computed or non-frontend files changed, output is empty + # and the optional Jest job will be skipped entirely. + - name: Get merge base for changedSince + id: merge_base + run: | + MERGE_BASE=$(git merge-base HEAD^1 HEAD^2 2>/dev/null) || true + if [ -n "$MERGE_BASE" ]; then + CHANGED=$(git diff --name-only "$MERGE_BASE" HEAD^2) + if echo "$CHANGED" | grep -qvE '^static/'; then + echo "Non-frontend file changed — skipping optional Jest" + MERGE_BASE="" + else + echo "Merge base: $MERGE_BASE (Jest will use --changedSince)" + fi + else + echo "Could not compute merge base — skipping optional Jest" + fi + echo "merge_base=${MERGE_BASE:-}" >> "$GITHUB_OUTPUT" + + # This job intentionally mirrors `frontend-jest-tests` in frontend.yml. + # Our intent is to try it out for a few weeks and see if it's stable. frontend-jest-tests-changed-only: - if: needs.files-changed.outputs.testable_rules_changed == 'true' || needs.files-changed.outputs.testable_modified == 'true' + if: >- + needs.files-changed.outputs.merge_base != '' && + (needs.files-changed.outputs.testable_rules_changed == 'true' || needs.files-changed.outputs.testable_modified == 'true') needs: [files-changed] name: Jest # If you change the runs-on image, you must also change the runner in jest-balance.yml @@ -50,7 +78,6 @@ jobs: fail-fast: false matrix: # XXX: When updating this, make sure you also update CI_NODE_TOTAL. - instance: [0, 1, 2, 3] steps: @@ -76,26 +103,6 @@ jobs: search_artifacts: true # Search for the last workflow run whose stored the artifact we're looking for if_no_artifact_found: warn # Can be one of: "fail", "warn", "ignore" - # On PRs, HEAD is the merge commit; its parents (HEAD^1, HEAD^2) are base and head. - # Merge base of those two is what Jest --changedSince needs. Checkout uses fetch-depth: 100 for PRs. - - name: Get merge base for changedSince (PRs only) - id: merge_base - if: github.event_name == 'pull_request' - run: | - MERGE_BASE=$(git merge-base HEAD^1 HEAD^2 2>/dev/null) || true - if [ -n "$MERGE_BASE" ]; then - CHANGED=$(git diff --name-only "$MERGE_BASE" HEAD^2) - if echo "$CHANGED" | grep -qvE '^static/'; then - echo "Non-frontend file changed — running full test suite" - MERGE_BASE="" - else - echo "Merge base: $MERGE_BASE (Jest will use --changedSince)" - fi - else - echo "Merge base: could not compute, will run full test suite" - fi - echo "merge_base=${MERGE_BASE:-}" >> "$GITHUB_OUTPUT" - - name: jest env: GITHUB_PR_SHA: ${{ github.event.pull_request.head.sha || github.sha }} @@ -111,7 +118,7 @@ jobs: # # This quiets up the logs quite a bit. DEBUG_PRINT_LIMIT: 0 - MERGE_BASE: ${{ steps.merge_base.outputs.merge_base }} + MERGE_BASE: ${{ needs.files-changed.outputs.merge_base }} run: pnpm run test-ci --forceExit typescript-native: