Skip to content

ref(ci): Re-add ALWAYS_RUN_TESTS to selective test computing#112930

Merged
rbro112 merged 1 commit intomasterfrom
ryan/04-14-re-add_always_run_tests_to_selective_test_computing
Apr 14, 2026
Merged

ref(ci): Re-add ALWAYS_RUN_TESTS to selective test computing#112930
rbro112 merged 1 commit intomasterfrom
ryan/04-14-re-add_always_run_tests_to_selective_test_computing

Conversation

@rbro112
Copy link
Copy Markdown
Member

@rbro112 rbro112 commented Apr 14, 2026

I had added an ALWAYS_RUN_TESTS config to compute-selective-tests.py in #112154 per Mark's request. At some point, compute-selective-tests.py was deleted and compute-sentry-selected-tests.py was added, but this ALWAYS_RUN_TESTS was not included.

Today, a test breakage slipped through on master that this ALWAYS_RUN_TESTS would've caught (Slack), this re-adds the tests to always run.

@rbro112 rbro112 requested a review from a team as a code owner April 14, 2026 16:06
Copy link
Copy Markdown
Member Author

rbro112 commented Apr 14, 2026

This stack of pull requests is managed by Graphite. Learn more about stacking.

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Apr 14, 2026
@rbro112 rbro112 changed the title Re-add ALWAYS_RUN_TESTS to selective test computing ref(ci): Re-add ALWAYS_RUN_TESTS to selective test computing Apr 14, 2026
Comment on lines 232 to +235

# Always run these tests
affected_test_files.update(ALWAYS_RUN_TESTS)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: Tests in ALWAYS_RUN_TESTS can be silently skipped if the file path is invalid, as they are filtered with Path.exists() without raising an error on failure.
Severity: HIGH

Suggested Fix

Either exempt files in ALWAYS_RUN_TESTS from the Path.exists() check, or modify the logic to raise an error and fail the CI job if a test from this list is not found. This ensures that missing critical tests do not go unnoticed.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: .github/workflows/scripts/compute-sentry-selected-tests.py#L232-L235

Potential issue: The script ensures tests in `ALWAYS_RUN_TESTS` are always executed.
However, these tests are subjected to an existence check using `Path(f).exists()`. If a
test file from this list is renamed, deleted, or its relative path becomes invalid, it
is silently removed from the test suite. A debug message is logged, but the CI job does
not fail. This defeats the purpose of the `ALWAYS_RUN_TESTS` mechanism, as it could
allow breakages in critical tests to go unnoticed.

@rbro112 rbro112 force-pushed the ryan/04-14-re-add_always_run_tests_to_selective_test_computing branch from d9ad596 to d5874b0 Compare April 14, 2026 17:39
@rbro112 rbro112 merged commit aded5c4 into master Apr 14, 2026
55 checks passed
@rbro112 rbro112 deleted the ryan/04-14-re-add_always_run_tests_to_selective_test_computing branch April 14, 2026 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants