From c5bd419a434229695a940b17b4f193492d1d01a4 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Thu, 27 Aug 2026 22:55:52 +0000 Subject: [PATCH 1/3] ci(selective): force Seer public API matrix on publish_status edits Selective testing only follows coverage and static imports, so flipping an endpoint to ApiPublishStatus.PUBLIC can land green on PRs while master fails the Seer agent-token public-mutation matrix. Detect publish_status in changed source files and always include that matrix test. Co-Authored-By: Dan Fuller --- .../scripts/compute-sentry-selected-tests.py | 38 +++++++ .../test_compute_sentry_selected_tests.py | 99 ++++++++++++++++++- 2 files changed, 136 insertions(+), 1 deletion(-) diff --git a/.github/workflows/scripts/compute-sentry-selected-tests.py b/.github/workflows/scripts/compute-sentry-selected-tests.py index d790a3103d1c..99de7144a779 100644 --- a/.github/workflows/scripts/compute-sentry-selected-tests.py +++ b/.github/workflows/scripts/compute-sentry-selected-tests.py @@ -157,6 +157,15 @@ "tests/sentry/backup/test_validate.py", } +# Seer Code Mode's public API auth matrix discovers every endpoint with +# publish_status = PUBLIC at collection time. Coverage and static imports only +# see direct callers of the endpoint module, so flipping an endpoint to PUBLIC +# (or editing publish_status at all) can break master while PR selective testing +# stays green. Force-include the matrix whenever a changed source file declares +# publish_status. +PUBLIC_API_MATRIX_TEST = "tests/sentry/seer/endpoints/test_organization_agent_token.py" +_PUBLISH_STATUS_DECL = re.compile(r"\bpublish_status\b") + def _is_test(path: str) -> bool: return any(path.startswith(d) for d in TEST_DIRS) @@ -168,6 +177,24 @@ def _matches_trigger(file_path: str, trigger: str | re.Pattern[str]) -> bool: return file_path == trigger +def _source_declares_publish_status(file_path: str, *, repo_root: Path | None = None) -> bool: + """True when a non-test Python source file declares endpoint publish_status.""" + if not file_path.endswith(".py") or _is_test(file_path): + return False + path = (repo_root or Path.cwd()) / file_path + try: + contents = path.read_text(encoding="utf-8") + except OSError: + return False + return _PUBLISH_STATUS_DECL.search(contents) is not None + + +def _changed_files_declare_publish_status( + changed_files: list[str], *, repo_root: Path | None = None +) -> list[str]: + return [f for f in changed_files if _source_declares_publish_status(f, repo_root=repo_root)] + + def _query_coverage(coverage_db_path: str, db_file_paths: list[str]) -> set[str]: """Query coverage DB for test contexts covering the given source files.""" conn = sqlite3.connect(coverage_db_path) @@ -313,6 +340,17 @@ def main() -> int: # Always run these tests affected_test_files.update(ALWAYS_RUN_TESTS) + # publish_status edits aren't attributed to the Seer public-API matrix + # via coverage/static imports — force it in when any changed source + # file still declares publish_status after the change. + publish_status_sources = _changed_files_declare_publish_status(changed) + if publish_status_sources: + print( + "Including public API matrix test due to publish_status in: " + + ", ".join(publish_status_sources) + ) + affected_test_files.add(PUBLIC_API_MATRIX_TEST) + # 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)} diff --git a/.github/workflows/scripts/test_compute_sentry_selected_tests.py b/.github/workflows/scripts/test_compute_sentry_selected_tests.py index e9336f0037d0..556c18b805f1 100644 --- a/.github/workflows/scripts/test_compute_sentry_selected_tests.py +++ b/.github/workflows/scripts/test_compute_sentry_selected_tests.py @@ -24,7 +24,10 @@ EXTRA_DIR_TO_TEST_MAPPING, EXTRA_FILE_TO_TEST_MAPPING, FULL_SUITE_TRIGGERS, + PUBLIC_API_MATRIX_TEST, + _changed_files_declare_publish_status, _query_coverage, + _source_declares_publish_status, main, ) @@ -427,6 +430,100 @@ def test_missing_db_returns_error(self): ret = _run(["--coverage-db", "/nonexistent/coverage.db", "--changed-files", "foo.py"]) assert ret == 1 + def test_publish_status_source_force_includes_public_api_matrix(self, tmp_path): + """Endpoints that declare publish_status must select the Seer public-API matrix.""" + db_path = tmp_path / "coverage.db" + _create_coverage_db(str(db_path), {}) + output = tmp_path / "output.txt" + gh_output = tmp_path / "gh_output" + gh_output.write_text("") + + endpoint = tmp_path / "src" / "sentry" / "api" / "endpoints" / "views.py" + endpoint.parent.mkdir(parents=True) + endpoint.write_text( + "class ViewsEndpoint:\n" + " publish_status = {\n" + ' "POST": ApiPublishStatus.PUBLIC,\n' + " }\n" + ) + + with mock.patch("compute_sentry_selected_tests.Path.cwd", return_value=tmp_path): + with mock.patch("compute_sentry_selected_tests.Path.exists", return_value=True): + _run( + [ + "--coverage-db", + str(db_path), + "--changed-files", + "src/sentry/api/endpoints/views.py", + "--output", + str(output), + "--github-output", + ], + {"GITHUB_OUTPUT": str(gh_output)}, + ) + + selected = set(output.read_text().splitlines()) + assert PUBLIC_API_MATRIX_TEST in selected + assert selected == ALWAYS_RUN_TESTS | {PUBLIC_API_MATRIX_TEST} + assert "has-selected-tests=true" in gh_output.read_text() + + def test_non_publish_status_source_does_not_force_public_api_matrix(self, tmp_path): + db_path = tmp_path / "coverage.db" + _create_coverage_db(str(db_path), {}) + output = tmp_path / "output.txt" + gh_output = tmp_path / "gh_output" + gh_output.write_text("") + + source = tmp_path / "src" / "sentry" / "utils" / "thing.py" + source.parent.mkdir(parents=True) + source.write_text("VALUE = 1\n") + + with mock.patch("compute_sentry_selected_tests.Path.cwd", return_value=tmp_path): + with mock.patch("compute_sentry_selected_tests.Path.exists", return_value=True): + _run( + [ + "--coverage-db", + str(db_path), + "--changed-files", + "src/sentry/utils/thing.py", + "--output", + str(output), + "--github-output", + ], + {"GITHUB_OUTPUT": str(gh_output)}, + ) + + assert set(output.read_text().splitlines()) == ALWAYS_RUN_TESTS + assert PUBLIC_API_MATRIX_TEST not in output.read_text() + + +class TestPublishStatusDetection: + def test_detects_publish_status_declaration(self, tmp_path): + path = tmp_path / "endpoint.py" + path.write_text('publish_status = {"GET": ApiPublishStatus.PUBLIC}\n') + assert _source_declares_publish_status("endpoint.py", repo_root=tmp_path) + + def test_ignores_test_files_and_missing_paths(self, tmp_path): + test_path = tmp_path / "tests" / "sentry" / "test_endpoint.py" + test_path.parent.mkdir(parents=True) + test_path.write_text("publish_status = {}\n") + assert not _source_declares_publish_status( + "tests/sentry/test_endpoint.py", repo_root=tmp_path + ) + assert not _source_declares_publish_status("missing.py", repo_root=tmp_path) + + def test_changed_files_filter(self, tmp_path): + endpoint = tmp_path / "src" / "sentry" / "api" / "endpoint.py" + other = tmp_path / "src" / "sentry" / "utils" / "x.py" + endpoint.parent.mkdir(parents=True) + other.parent.mkdir(parents=True) + endpoint.write_text("publish_status = {}\n") + other.write_text("x = 1\n") + assert _changed_files_declare_publish_status( + ["src/sentry/api/endpoint.py", "src/sentry/utils/x.py"], + repo_root=tmp_path, + ) == ["src/sentry/api/endpoint.py"] + class TestConfigPaths: """Assert every literal path in the selective testing config still exists on disk. @@ -440,7 +537,7 @@ class TestConfigPaths: def test_full_suite_triggers_exist(self, trigger: str) -> None: assert (_REPO_ROOT / trigger).exists(), _stale_msg(trigger) - @pytest.mark.parametrize("path", sorted(ALWAYS_RUN_TESTS)) + @pytest.mark.parametrize("path", sorted(ALWAYS_RUN_TESTS | {PUBLIC_API_MATRIX_TEST})) def test_always_run_tests_exist(self, path: str) -> None: assert (_REPO_ROOT / path).exists(), _stale_msg(path, "test file") From 5b4133a97fb012c62e1ee5531e6acabb91ff9c39 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Thu, 27 Aug 2026 23:03:42 +0000 Subject: [PATCH 2/3] ci(selective): trim comments on publish_status matrix force-include Co-Authored-By: Dan Fuller --- .../scripts/compute-sentry-selected-tests.py | 12 ++---------- .../scripts/test_compute_sentry_selected_tests.py | 1 - 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/.github/workflows/scripts/compute-sentry-selected-tests.py b/.github/workflows/scripts/compute-sentry-selected-tests.py index 99de7144a779..0d8260500702 100644 --- a/.github/workflows/scripts/compute-sentry-selected-tests.py +++ b/.github/workflows/scripts/compute-sentry-selected-tests.py @@ -157,12 +157,8 @@ "tests/sentry/backup/test_validate.py", } -# Seer Code Mode's public API auth matrix discovers every endpoint with -# publish_status = PUBLIC at collection time. Coverage and static imports only -# see direct callers of the endpoint module, so flipping an endpoint to PUBLIC -# (or editing publish_status at all) can break master while PR selective testing -# stays green. Force-include the matrix whenever a changed source file declares -# publish_status. +# Seer public-API matrix is discovered at collection time, not via coverage/ +# static imports of endpoint modules. Include it when publish_status is touched. PUBLIC_API_MATRIX_TEST = "tests/sentry/seer/endpoints/test_organization_agent_token.py" _PUBLISH_STATUS_DECL = re.compile(r"\bpublish_status\b") @@ -178,7 +174,6 @@ def _matches_trigger(file_path: str, trigger: str | re.Pattern[str]) -> bool: def _source_declares_publish_status(file_path: str, *, repo_root: Path | None = None) -> bool: - """True when a non-test Python source file declares endpoint publish_status.""" if not file_path.endswith(".py") or _is_test(file_path): return False path = (repo_root or Path.cwd()) / file_path @@ -340,9 +335,6 @@ def main() -> int: # Always run these tests affected_test_files.update(ALWAYS_RUN_TESTS) - # publish_status edits aren't attributed to the Seer public-API matrix - # via coverage/static imports — force it in when any changed source - # file still declares publish_status after the change. publish_status_sources = _changed_files_declare_publish_status(changed) if publish_status_sources: print( diff --git a/.github/workflows/scripts/test_compute_sentry_selected_tests.py b/.github/workflows/scripts/test_compute_sentry_selected_tests.py index 556c18b805f1..b96ea2cd1afe 100644 --- a/.github/workflows/scripts/test_compute_sentry_selected_tests.py +++ b/.github/workflows/scripts/test_compute_sentry_selected_tests.py @@ -431,7 +431,6 @@ def test_missing_db_returns_error(self): assert ret == 1 def test_publish_status_source_force_includes_public_api_matrix(self, tmp_path): - """Endpoints that declare publish_status must select the Seer public-API matrix.""" db_path = tmp_path / "coverage.db" _create_coverage_db(str(db_path), {}) output = tmp_path / "output.txt" From d53784f0895a446462c6006e96446deda9c5ddb8 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Thu, 27 Aug 2026 23:11:31 +0000 Subject: [PATCH 3/3] ci(selective): path-filter endpoint edits into Seer public API matrix Replace content scanning for publish_status with a path trigger on src/sentry/**/endpoints/**.py so selection stays path-based like the rest of this script. Co-Authored-By: Dan Fuller --- .../scripts/compute-sentry-selected-tests.py | 38 +++--- .../test_compute_sentry_selected_tests.py | 113 +++++++----------- 2 files changed, 58 insertions(+), 93 deletions(-) diff --git a/.github/workflows/scripts/compute-sentry-selected-tests.py b/.github/workflows/scripts/compute-sentry-selected-tests.py index 0d8260500702..4066423106d1 100644 --- a/.github/workflows/scripts/compute-sentry-selected-tests.py +++ b/.github/workflows/scripts/compute-sentry-selected-tests.py @@ -157,10 +157,13 @@ "tests/sentry/backup/test_validate.py", } -# Seer public-API matrix is discovered at collection time, not via coverage/ -# static imports of endpoint modules. Include it when publish_status is touched. +# Seer public-API matrix discovers PUBLIC mutations at collection time, so +# endpoint module edits (including publish_status flips) need an explicit include. PUBLIC_API_MATRIX_TEST = "tests/sentry/seer/endpoints/test_organization_agent_token.py" -_PUBLISH_STATUS_DECL = re.compile(r"\bpublish_status\b") +PUBLIC_API_MATRIX_PATH_TRIGGERS: list[re.Pattern[str]] = [ + # Endpoint modules live under */endpoints/ across product areas. + re.compile(r"^src/sentry/.*/endpoints/.*\.py$"), +] def _is_test(path: str) -> bool: @@ -173,21 +176,12 @@ def _matches_trigger(file_path: str, trigger: str | re.Pattern[str]) -> bool: return file_path == trigger -def _source_declares_publish_status(file_path: str, *, repo_root: Path | None = None) -> bool: - if not file_path.endswith(".py") or _is_test(file_path): - return False - path = (repo_root or Path.cwd()) / file_path - try: - contents = path.read_text(encoding="utf-8") - except OSError: - return False - return _PUBLISH_STATUS_DECL.search(contents) is not None - - -def _changed_files_declare_publish_status( - changed_files: list[str], *, repo_root: Path | None = None -) -> list[str]: - return [f for f in changed_files if _source_declares_publish_status(f, repo_root=repo_root)] +def _changed_files_match_public_api_matrix_paths(changed_files: list[str]) -> list[str]: + return [ + f + for f in changed_files + if any(_matches_trigger(f, t) for t in PUBLIC_API_MATRIX_PATH_TRIGGERS) + ] def _query_coverage(coverage_db_path: str, db_file_paths: list[str]) -> set[str]: @@ -335,11 +329,11 @@ def main() -> int: # Always run these tests affected_test_files.update(ALWAYS_RUN_TESTS) - publish_status_sources = _changed_files_declare_publish_status(changed) - if publish_status_sources: + endpoint_sources = _changed_files_match_public_api_matrix_paths(changed) + if endpoint_sources: print( - "Including public API matrix test due to publish_status in: " - + ", ".join(publish_status_sources) + "Including public API matrix test due to endpoint path(s): " + + ", ".join(endpoint_sources) ) affected_test_files.add(PUBLIC_API_MATRIX_TEST) diff --git a/.github/workflows/scripts/test_compute_sentry_selected_tests.py b/.github/workflows/scripts/test_compute_sentry_selected_tests.py index b96ea2cd1afe..937ab8047c13 100644 --- a/.github/workflows/scripts/test_compute_sentry_selected_tests.py +++ b/.github/workflows/scripts/test_compute_sentry_selected_tests.py @@ -25,9 +25,8 @@ EXTRA_FILE_TO_TEST_MAPPING, FULL_SUITE_TRIGGERS, PUBLIC_API_MATRIX_TEST, - _changed_files_declare_publish_status, + _changed_files_match_public_api_matrix_paths, _query_coverage, - _source_declares_publish_status, main, ) @@ -430,98 +429,70 @@ def test_missing_db_returns_error(self): ret = _run(["--coverage-db", "/nonexistent/coverage.db", "--changed-files", "foo.py"]) assert ret == 1 - def test_publish_status_source_force_includes_public_api_matrix(self, tmp_path): + def test_endpoint_path_force_includes_public_api_matrix(self, tmp_path): db_path = tmp_path / "coverage.db" _create_coverage_db(str(db_path), {}) output = tmp_path / "output.txt" gh_output = tmp_path / "gh_output" gh_output.write_text("") - endpoint = tmp_path / "src" / "sentry" / "api" / "endpoints" / "views.py" - endpoint.parent.mkdir(parents=True) - endpoint.write_text( - "class ViewsEndpoint:\n" - " publish_status = {\n" - ' "POST": ApiPublishStatus.PUBLIC,\n' - " }\n" - ) - - with mock.patch("compute_sentry_selected_tests.Path.cwd", return_value=tmp_path): - with mock.patch("compute_sentry_selected_tests.Path.exists", return_value=True): - _run( - [ - "--coverage-db", - str(db_path), - "--changed-files", - "src/sentry/api/endpoints/views.py", - "--output", - str(output), - "--github-output", - ], - {"GITHUB_OUTPUT": str(gh_output)}, - ) + with mock.patch("compute_sentry_selected_tests.Path.exists", return_value=True): + _run( + [ + "--coverage-db", + str(db_path), + "--changed-files", + "src/sentry/api/endpoints/views.py", + "--output", + str(output), + "--github-output", + ], + {"GITHUB_OUTPUT": str(gh_output)}, + ) selected = set(output.read_text().splitlines()) assert PUBLIC_API_MATRIX_TEST in selected assert selected == ALWAYS_RUN_TESTS | {PUBLIC_API_MATRIX_TEST} assert "has-selected-tests=true" in gh_output.read_text() - def test_non_publish_status_source_does_not_force_public_api_matrix(self, tmp_path): + def test_non_endpoint_source_does_not_force_public_api_matrix(self, tmp_path): db_path = tmp_path / "coverage.db" _create_coverage_db(str(db_path), {}) output = tmp_path / "output.txt" gh_output = tmp_path / "gh_output" gh_output.write_text("") - source = tmp_path / "src" / "sentry" / "utils" / "thing.py" - source.parent.mkdir(parents=True) - source.write_text("VALUE = 1\n") - - with mock.patch("compute_sentry_selected_tests.Path.cwd", return_value=tmp_path): - with mock.patch("compute_sentry_selected_tests.Path.exists", return_value=True): - _run( - [ - "--coverage-db", - str(db_path), - "--changed-files", - "src/sentry/utils/thing.py", - "--output", - str(output), - "--github-output", - ], - {"GITHUB_OUTPUT": str(gh_output)}, - ) + with mock.patch("compute_sentry_selected_tests.Path.exists", return_value=True): + _run( + [ + "--coverage-db", + str(db_path), + "--changed-files", + "src/sentry/utils/thing.py", + "--output", + str(output), + "--github-output", + ], + {"GITHUB_OUTPUT": str(gh_output)}, + ) assert set(output.read_text().splitlines()) == ALWAYS_RUN_TESTS assert PUBLIC_API_MATRIX_TEST not in output.read_text() -class TestPublishStatusDetection: - def test_detects_publish_status_declaration(self, tmp_path): - path = tmp_path / "endpoint.py" - path.write_text('publish_status = {"GET": ApiPublishStatus.PUBLIC}\n') - assert _source_declares_publish_status("endpoint.py", repo_root=tmp_path) - - def test_ignores_test_files_and_missing_paths(self, tmp_path): - test_path = tmp_path / "tests" / "sentry" / "test_endpoint.py" - test_path.parent.mkdir(parents=True) - test_path.write_text("publish_status = {}\n") - assert not _source_declares_publish_status( - "tests/sentry/test_endpoint.py", repo_root=tmp_path - ) - assert not _source_declares_publish_status("missing.py", repo_root=tmp_path) - - def test_changed_files_filter(self, tmp_path): - endpoint = tmp_path / "src" / "sentry" / "api" / "endpoint.py" - other = tmp_path / "src" / "sentry" / "utils" / "x.py" - endpoint.parent.mkdir(parents=True) - other.parent.mkdir(parents=True) - endpoint.write_text("publish_status = {}\n") - other.write_text("x = 1\n") - assert _changed_files_declare_publish_status( - ["src/sentry/api/endpoint.py", "src/sentry/utils/x.py"], - repo_root=tmp_path, - ) == ["src/sentry/api/endpoint.py"] +class TestPublicApiMatrixPathTriggers: + def test_matches_endpoint_paths(self): + assert _changed_files_match_public_api_matrix_paths( + [ + "src/sentry/api/endpoints/views.py", + "src/sentry/issues/endpoints/organization_group_search_views.py", + "src/sentry/utils/thing.py", + "tests/sentry/api/endpoints/test_views.py", + ] + ) == [ + "src/sentry/api/endpoints/views.py", + "src/sentry/issues/endpoints/organization_group_search_views.py", + ] class TestConfigPaths: