ref(ci): Run more of sentry's tests when we change the API in snuba by pbhandari · Pull Request #8237 · getsentry/snuba

pbhandari · 2026-07-29T15:48:48Z

What

The sentry-tests job maintained a hand-curated list of tests/sentry/... paths to decide which of Sentry's tests to run against a Snuba change. That list is brittle: as Snuba-dependent tests are added, moved, or renamed in the
Sentry repo, the list silently goes stale and we stop covering them.

The lack of coverage can lead to a snuba deploy that is incompatible with Sentry which can take up to 2+

This replaces the path lists with marker-based selection; that marker is associated with the base class SnubaTestCase, and is now run over the whole tests tree:

Always-run step (api_changes == false): -m snuba_ci now scans all of tests/ instead of three hardcoded directories.
API-change step (api_changes == true): collapses the old "Run full tests" (explicit path list) steps into a single -m snuba tests run.
Run CI module tests (api_changes == true): is identical to the api_changes == false path and has been collapsed.

So which tests run is now driven by how tests are marked in Sentry, not by a path list in this repo that has to be kept in sync by hand.

Tests collected: before → after

Collected with pytest --collect-only -k 'not __In' in the Sentry repo.

Scenario	Before	After
Always-run (`-m snuba_ci`)	173 (3 dirs)	173 (whole `tests/`)
On API change	4,716 + 173¹ (path list + `snuba_ci`)	5,555 (`-m snuba` whole `tests/`)

Takeaways:

The snuba_ci set is unchanged (173) — widening collection to the whole tree discovers the same marked tests, just without the stale hardcoded dirs.
On an API change we now collect 5,555 snuba-marked tests vs the previous 4,716 path-listed tests — ~840 more, and no longer dependent on the path list staying current.

(Local collection reports 1 error for tests/acceptance, which needs a webpack/pnpm frontend build that isn't set up locally; it carries no snuba-marked tests and is present in CI, so it doesn't affect these counts.)

🤖 Generated with Claude Code

pbhandari · 2026-07-29T16:02:35Z

        if: needs.files-changed.outputs.api_changes == 'false'
        working-directory: sentry
        run: |
-          # Only these dirs carry the `snuba_ci` marker; collecting all of tests/ just to find them is wasteful.


This comment is valid; but given that pytest collection is pretty fast, and reverting a change can take up to 2+ hours if we miss something on the CI ... better to just collect everything

pbhandari · 2026-07-29T16:04:05Z

+              -m snuba \
+              tests \


Theoretically we are missing some tests that used to be covered before. But I don't think that's an actual concern since we care more about how our API changes interact with the monolith, only running the SnubaTestCase test cases should be sufficient.

pbhandari · 2026-07-29T16:06:27Z


-      - name: Run CI module tests
-        if: needs.files-changed.outputs.api_changes == 'true'
-        working-directory: sentry
-        run: |
-          pytest -k 'not __In' \
-            tests/sentry/snuba \
-            tests/sentry/workflow_engine/endpoints \
-            tests/snuba/rules/conditions \
-            -m snuba_ci \
-            -n "$XDIST_WORKERS" --dist=loadfile --reuse-db \
-            -vv
-


This was identical to the Run snuba tests step; with the only exception being the if gate.

cursor · 2026-07-29T16:12:56Z

-            tests/sentry/workflow_engine/endpoints \
-            tests/snuba/rules/conditions \
            -m snuba_ci \
+            tests \


Acceptance collection can fail CI

High Severity

Widening collection to all of tests now pulls in tests/acceptance, which this PR already notes fails to collect without a webpack/pnpm frontend build. This job uses mode: minimal and does not build frontend assets, and pytest treats collection errors as failures even when no snuba/snuba_ci tests live there. The previous path lists never included acceptance, so this can fail the sentry job on every PR.

Additional Locations (1)

.github/workflows/ci.yml#L483-L484

^{Reviewed by Cursor Bugbot for commit bb56d76. Configure here.}

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 5150e13. Configure here.}

cursor · 2026-07-29T16:31:10Z

            -vv

      - name: Run full tests
-        if: needs.files-changed.outputs.api_changes == 'true'


Full tests missing API-change gate

Medium Severity

The Run full tests step dropped its if: needs.files-changed.outputs.api_changes == 'true' guard, so the large -m snuba suite now runs on every PR. The PR description still treats that suite as API-change-only, and api_changes is computed but unused. Non-API PRs therefore pay for ~5,555 tests instead of the intended ~173 snuba_ci set.

^{Reviewed by Cursor Bugbot for commit 5150e13. Configure here.}

sentry · 2026-07-29T16:31:23Z

        working-directory: sentry
        run: |
          pytest -k 'not __In' \
-              tests/snuba \
-              tests/sentry/snuba \
-              tests/sentry/eventstream/kafka \
-              tests/sentry/post_process_forwarder \
-              tests/sentry/services/eventstore/snuba \
-              tests/sentry/search/events \
-              tests/sentry/event_manager \
-              tests/sentry/api/endpoints/test_organization_profiling_functions.py \
-              tests/sentry/integrations/slack/test_unfurl.py \
-              tests/sentry/uptime/endpoints/test_project_uptime_alert_check_index.py \
-              tests/sentry/uptime/endpoints/test_organization_uptime_stats.py \
-              tests/sentry/api/endpoints/test_organization_traces.py \
-              tests/sentry/api/endpoints/test_organization_spans_fields.py \
-              tests/sentry/api/endpoints/test_organization_ai_conversations.py \
-              tests/sentry/api/endpoints/test_organization_ai_conversation_details.py \
-              tests/sentry/sentry_metrics/querying \
+              -m snuba \
+              tests \
              -n "$XDIST_WORKERS" --dist=loadfile --reuse-db \
              -vv


Bug: The if conditions gating the "Run snuba CI tests" and "Run full tests" steps have been removed, causing both to run unconditionally on every PR.
_{Severity: MEDIUM}

Suggested Fix

Restore the if conditions to the respective test steps. The "Run snuba CI tests" step should have if: needs.files-changed.outputs.api_changes == 'false', and the "Run full tests" step should have if: needs.files-changed.outputs.api_changes == 'true'. This will ensure the expensive full test suite only runs when necessary.

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/ci.yml#L478-L484 Potential issue: The `if` conditions that were supposed to conditionally execute test steps based on `needs.files-changed.outputs.api_changes` have been removed from the CI workflow. As a result, both the "Run snuba CI tests" step and the expensive "Run full tests" step now run on every pull request, regardless of whether API changes were detected. This contradicts the PR's stated intent to only run the full test suite when API changes are present, leading to significant CI resource waste by unnecessarily executing a large test suite.

pbhandari requested a review from a team as a code owner July 29, 2026 15:48

cursor Bot reviewed Jul 29, 2026

View reviewed changes

Comment thread .github/workflows/ci.yml

sentry Bot reviewed Jul 29, 2026

View reviewed changes

Comment thread .github/workflows/ci.yml Outdated

ref(ci): Run more of sentry's tests when we change the API in snuba

bb56d76

pbhandari force-pushed the ref/more-tests-in-ci branch from e9b30da to bb56d76 Compare July 29, 2026 16:00

pbhandari commented Jul 29, 2026

View reviewed changes

cursor Bot reviewed Jul 29, 2026

View reviewed changes

Run full test suite on CI to see times

5150e13

cursor Bot reviewed Jul 29, 2026

View reviewed changes

sentry Bot reviewed Jul 29, 2026

View reviewed changes

pbhandari marked this pull request as draft July 29, 2026 16:36

pbhandari added 2 commits July 30, 2026 09:51

Ignore acceptance tests

7ecbb74

Use Makefile instead of hardcoded pytest to run tests

299ff7c

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

ref(ci): Run more of sentry's tests when we change the API in snuba - #8237

ref(ci): Run more of sentry's tests when we change the API in snuba#8237
pbhandari wants to merge 4 commits into
masterfrom
ref/more-tests-in-ci

pbhandari commented Jul 29, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

pbhandari Jul 29, 2026 •

edited

Loading

Uh oh!

pbhandari Jul 29, 2026

Uh oh!

pbhandari Jul 29, 2026

Uh oh!

cursor Bot Jul 29, 2026

Uh oh!

cursor Bot left a comment

Uh oh!

cursor Bot Jul 29, 2026

Uh oh!

sentry Bot Jul 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

pbhandari commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Tests collected: before → after

Uh oh!

Uh oh!

Uh oh!

pbhandari Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pbhandari Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

pbhandari Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot Jul 29, 2026

Choose a reason for hiding this comment

Acceptance collection can fail CI

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor Bot Jul 29, 2026

Choose a reason for hiding this comment

Full tests missing API-change gate

Uh oh!

sentry Bot Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

pbhandari commented Jul 29, 2026 •

edited

Loading

pbhandari Jul 29, 2026 •

edited

Loading