Skip to content

Conversation

mydea
Copy link
Member

@mydea mydea commented Sep 8, 2025

Noticed here: https://github.com/getsentry/sentry-javascript/actions/runs/17542916888/job/49818463117 that this was not actually working 🤔

I played around a bit with this locally, and this change made it work for me. Not sure why the path part is not working as expected, but we filter for the correct changes anyhow below, so this should be fine IMHO.

@mydea mydea self-assigned this Sep 8, 2025
Comment on lines +177 to 178
const changedFiles = execSync(`git diff --name-only ${base}${head ? `..${head}` : ''} -- .`, {
encoding: 'utf-8',
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential bug: The git diff scope was broadened, but the file filtering logic was not updated, causing all E2E tests to run unnecessarily for most infrastructure changes.
  • Description: The git diff command in getChangedTestApps was changed to scan for file changes across the entire repository (-- .) instead of being limited to the dev-packages/e2e-tests/ directory. However, the subsequent logic still checks if every changed file starts with the dev-packages/e2e-tests/test-applications/ prefix. If any file outside this specific subdirectory is changed (e.g., a change to the test infrastructure file getTestMatrix.ts itself), the function returns false. This causes the CI pipeline to run all E2E tests, defeating the purpose of the selective test matrix optimization and leading to significantly longer CI execution times.

  • Suggested fix: Revert the path specifier in the git diff command from -- . back to the more specific -- dev-packages/e2e-tests/. This ensures the command only considers file changes within the E2E tests package, which aligns with the existing filtering logic and restores the intended selective test execution.
    severity: 0.65, confidence: 0.98

Did we get this right? 👍 / 👎 to inform future reviews.

@mydea mydea merged commit dcb4d23 into develop Sep 8, 2025
163 checks passed
@mydea mydea deleted the fn/run-changes-e2e-tests branch September 8, 2025 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants