Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rbro112 merged 1 commit into from
Apr 14, 2026
+25 −14
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .github/workflows/scripts/compute-sentry-selected-tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@
re.compile(r"^tests/(acceptance|apidocs|js|tools)/"),
]

# Tests that should always be run even if not explicitly selected.
ALWAYS_RUN_TESTS: set[str] = {
"tests/sentry/taskworker/test_config.py",
}


def _is_test(path: str) -> bool:
return any(path.startswith(d) for d in TEST_DIRS)
Expand Down Expand Up @@ -225,6 +230,9 @@ def main() -> int:
print(f"Including {len(existing_changed)} directly changed test files")
affected_test_files.update(existing_changed)

# Always run these tests
affected_test_files.update(ALWAYS_RUN_TESTS)

Comment on lines 232 to +235
Copy link
Copy Markdown
Contributor

@sentry sentry bot Apr 14, 2026

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.

# Filter to sentry tests only (drop any getsentry tests from coverage)
affected_test_files = {f for f in affected_test_files if _is_test(f)}

Expand Down
31 changes: 17 additions & 14 deletions .github/workflows/scripts/test_compute_sentry_selected_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
sys.modules["compute_sentry_selected_tests"] = _mod
_spec.loader.exec_module(_mod)

from compute_sentry_selected_tests import _query_coverage, main
from compute_sentry_selected_tests import ALWAYS_RUN_TESTS, _query_coverage, main


def _create_coverage_db(path: str, file_to_contexts: dict[str, list[str]]) -> None:
Expand Down Expand Up @@ -178,8 +178,9 @@ def test_selective_returns_matched_tests(self, tmp_path):

gh = gh_output.read_text()
assert "has-selected-tests=true" in gh
assert "test-count=1" in gh
assert output.read_text().strip() == "tests/sentry/test_org.py"
assert f"test-count={1 + len(ALWAYS_RUN_TESTS)}" in gh
expected_output = sorted({"tests/sentry/test_org.py"} | ALWAYS_RUN_TESTS)
assert output.read_text().splitlines() == expected_output

def test_getsentry_tests_filtered_out(self, tmp_path):
"""Coverage may return getsentry tests — they should be filtered."""
Expand Down Expand Up @@ -211,8 +212,9 @@ def test_getsentry_tests_filtered_out(self, tmp_path):
{"GITHUB_OUTPUT": str(gh_output)},
)

assert "test-count=1" in gh_output.read_text()
assert output.read_text().strip() == "tests/sentry/test_org.py"
assert f"test-count={1 + len(ALWAYS_RUN_TESTS)}" in gh_output.read_text()
expected_output = sorted({"tests/sentry/test_org.py"} | ALWAYS_RUN_TESTS)
assert output.read_text().splitlines() == expected_output

def test_changed_test_file_included(self, tmp_path):
db_path = tmp_path / "coverage.db"
Expand All @@ -236,7 +238,7 @@ def test_changed_test_file_included(self, tmp_path):

gh = gh_output.read_text()
assert "has-selected-tests=true" in gh
assert "test-count=1" in gh
assert f"test-count={1 + len(ALWAYS_RUN_TESTS)}" in gh

def test_excluded_test_dirs_skipped(self, tmp_path):
"""Tests in acceptance/apidocs/js/tools should not be selected."""
Expand All @@ -257,10 +259,10 @@ def test_excluded_test_dirs_skipped(self, tmp_path):
{"GITHUB_OUTPUT": str(gh_output)},
)

assert "test-count=0" in gh_output.read_text()
assert f"test-count={len(ALWAYS_RUN_TESTS)}" in gh_output.read_text()

def test_zero_tests_signals_selective_applied(self, tmp_path):
"""0 tests after filtering should signal 'run nothing', not full suite."""
def test_zero_coverage_matches_still_runs_always_run_tests(self, tmp_path):
"""No coverage matches should still run ALWAYS_RUN_TESTS, not the full suite."""
db_path = tmp_path / "coverage.db"
_create_coverage_db(str(db_path), {})
output = tmp_path / "output.txt"
Expand All @@ -282,8 +284,8 @@ def test_zero_tests_signals_selective_applied(self, tmp_path):

gh = gh_output.read_text()
assert "has-selected-tests=true" in gh
assert "test-count=0" in gh
assert output.read_text() == ""
assert f"test-count={len(ALWAYS_RUN_TESTS)}" in gh
assert set(output.read_text().splitlines()) == ALWAYS_RUN_TESTS

def test_renamed_file_queries_old_path(self, tmp_path):
"""When a file is renamed, the old path should be queried against the coverage DB."""
Expand Down Expand Up @@ -319,8 +321,9 @@ def test_renamed_file_queries_old_path(self, tmp_path):

gh = gh_output.read_text()
assert "has-selected-tests=true" in gh
assert "test-count=1" in gh
assert output.read_text().strip() == "tests/sentry/test_old_name.py"
assert f"test-count={1 + len(ALWAYS_RUN_TESTS)}" in gh
expected_output = sorted({"tests/sentry/test_old_name.py"} | ALWAYS_RUN_TESTS)
assert output.read_text().splitlines() == expected_output

def test_renamed_file_without_previous_misses_coverage(self, tmp_path):
"""Without --previous-filenames, a renamed file gets no coverage hits."""
Expand Down Expand Up @@ -349,7 +352,7 @@ def test_renamed_file_without_previous_misses_coverage(self, tmp_path):

gh = gh_output.read_text()
assert "has-selected-tests=true" in gh
assert "test-count=0" in gh
assert f"test-count={len(ALWAYS_RUN_TESTS)}" in gh

def test_missing_db_returns_error(self):
ret = _run(["--coverage-db", "/nonexistent/coverage.db", "--changed-files", "foo.py"])
Expand Down
Loading