Skip to content

test: Fix leaked attachment response and expired seer replay fixture - #127952

Merged
joshuarli merged 2 commits into
masterfrom
test/fix-flaky-backend-tests-followup
Oct 18, 2026
Merged

joshuarli merged 2 commits into
masterfrom
test/fix-flaky-backend-tests-followup

Conversation

@sentry-junior

@sentry-junior sentry-junior Bot commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

Follow-up to the earlier flaky backend test fixes. Two test-only commits, no src/ changes. Neither commit changes assertions or test ordering, and neither adds skips, sleeps, or reruns.

Close the second superuser attachment download response

test_superuser_can_access in tests/sentry/api/endpoints/test_event_attachment_details.py left its second streaming attachment response unconsumed and unclosed. When garbage collection finalized it later, the ResourceWarning showed up as a PytestUnraisableExceptionWarning in whatever unrelated test happened to be running. Over the last 14 days pytest-sentry recorded 592 of these warning events across 13 tests.

The fix consumes and closes the second response the same way the first one already is.

  • Before: reproduced 5/5 locally
  • After: 20/20 passes

Store the 89-day replay with 90-day retention

test_get_replay_metadata_short_id in tests/sentry/seer/agent/test_tools.py stores replay rows timestamped 89 days ago, but mock_replay defaults to retention_days=30. The ClickHouse TTL is timestamp + retention_days, so those rows are already expired when they're inserted. Locally, system.part_log showed a TTL-delete merge (TTLDeleteMerge) dropping them about 6 seconds after insertion. The test passes today only because its lookup runs before that merge.

The fix passes retention_days=90 on the two 89-day rows, so they stay inside retention like real data would. This is preventive: 0 CI failures in the last 14 days. With the fix the relevant tests pass 2/2, and the TTL probe finds no expired rows.

Investigated, not changed

  • TTL sweep: I swept about 13.2k Snuba-backed tests. No other test uses a fixed timestamp that's still inside retention and will age out soon. The other rows that were already expired at insert either don't affect assertions or are expired on purpose.
  • Readthrough cache: a synthetic probe reproduced a stale read. In the sweep, though, only 3 tests had confirmed cache hits after a write, and all 3 assert on Postgres, not on the affected Snuba data. I didn't find a Sentry-side fix that keeps test fidelity.
  • Snuba read timeouts: I saw occasional 30s Snuba read timeouts but didn't confirm a root cause, so there's no code change for them here.

via @joshuarli.

--

View Junior Session [Sentry]

sentry-junior Bot and others added 2 commits October 17, 2026 23:15
test_superuser_can_access downloads the attachment a second time under
SENTRY_SELF_HOSTED=False without consuming the streaming response, so the
attachment file stays open. Garbage collection closes it later, during
whichever test runs next on the worker. pytest reports the ResourceWarning
there as a PytestUnraisableExceptionWarning, which fails that test and
passes on rerun. This was the most common rerun cause in the API endpoint
tests, landing on test_oauth_userinfo, test_event_committers,
test_organization_access_request_details and others.

Co-Authored-By: Joshua Li <14209781+joshuarli@users.noreply.github.com>
test_get_replay_metadata_short_id stores a replay 89 days old to check
lookups at the start of the 90-day query window, but mock_replay defaults
to retention_days=30. ClickHouse's TTL is timestamp + retention_days, so
those rows are already expired when inserted and the next TTL merge on
replays_local deletes them. Locally that merge ran about 6s after the
insert, after the test's lookup, so it has not failed in CI yet; a slower
runner or a later lookup would lose the replay.

Co-Authored-By: Joshua Li <14209781+joshuarli@users.noreply.github.com>
@joshuarli
joshuarli marked this pull request as ready for review October 18, 2026 16:03
@joshuarli
joshuarli requested review from a team as code owners October 18, 2026 16:03
@joshuarli
joshuarli merged commit c893ba7 into master Oct 18, 2026
64 checks passed
@joshuarli
joshuarli deleted the test/fix-flaky-backend-tests-followup branch October 18, 2026 16:03
RudraPatel2003 pushed a commit that referenced this pull request Oct 19, 2026
…127952)

Follow-up to the earlier flaky backend test fixes. Two test-only
commits, no `src/` changes. Neither commit changes assertions or test
ordering, and neither adds skips, sleeps, or reruns.

## Close the second superuser attachment download response

`test_superuser_can_access` in
`tests/sentry/api/endpoints/test_event_attachment_details.py` left its
second streaming attachment response unconsumed and unclosed. When
garbage collection finalized it later, the `ResourceWarning` showed up
as a `PytestUnraisableExceptionWarning` in whatever unrelated test
happened to be running. Over the last 14 days pytest-sentry recorded 592
of these warning events across 13 tests.

The fix consumes and closes the second response the same way the first
one already is.

- Before: reproduced 5/5 locally
- After: 20/20 passes

## Store the 89-day replay with 90-day retention

`test_get_replay_metadata_short_id` in
`tests/sentry/seer/agent/test_tools.py` stores replay rows timestamped
89 days ago, but `mock_replay` defaults to `retention_days=30`. The
ClickHouse TTL is `timestamp + retention_days`, so those rows are
already expired when they're inserted. Locally, `system.part_log` showed
a TTL-delete merge (`TTLDeleteMerge`) dropping them about 6 seconds
after insertion. The test passes today only because its lookup runs
before that merge.

The fix passes `retention_days=90` on the two 89-day rows, so they stay
inside retention like real data would. This is preventive: 0 CI failures
in the last 14 days. With the fix the relevant tests pass 2/2, and the
TTL probe finds no expired rows.

## Investigated, not changed

- **TTL sweep:** I swept about 13.2k Snuba-backed tests. No other test
uses a fixed timestamp that's still inside retention and will age out
soon. The other rows that were already expired at insert either don't
affect assertions or are expired on purpose.
- **Readthrough cache:** a synthetic probe reproduced a stale read. In
the sweep, though, only 3 tests had confirmed cache hits after a write,
and all 3 assert on Postgres, not on the affected Snuba data. I didn't
find a Sentry-side fix that keeps test fidelity.
- **Snuba read timeouts:** I saw occasional 30s Snuba read timeouts but
didn't confirm a root cause, so there's no code change for them here.

<!-- junior-request-attribution:start -->
via @joshuarli.
<!-- junior-request-attribution:end -->

<!-- junior-session-footer:start -->
<!-- junior-conversation-id:agent%3Af1c2ed6b2adb46ac0acce53c705b727e -->

--

[View Junior
Session](https://junior-prod.sentry.dev/conversations/agent%3Af1c2ed6b2adb46ac0acce53c705b727e)
[[Sentry]](https://sentry.sentry.io/explore/conversations/agent%3Af1c2ed6b2adb46ac0acce53c705b727e/?project=4510944073809921)

<!-- junior-session-footer:end -->

---------

Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com>
Co-authored-by: Joshua Li <14209781+joshuarli@users.noreply.github.com>
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.

1 participant