Repository navigation
test: isolate viewer context between pytest tests - #129727
Merged
Merged
Conversation
cmanallen
approved these changes
Nov 3, 2026
sehr-m
added a commit
that referenced
this pull request
Nov 3, 2026
…ion (#129729) Stacked on #129727. Merge the test-isolation fix first. Reapply #129261, which was reverted after full backend CI observed UNKNOWN viewer actors in statistical-detector tests. Slack validation assigns the authenticated linked human as the USER actor; bot and unlinked requests remain UNKNOWN, preserving already resolved scope and respecting SENTRY_VIEWER_CONTEXT_ENABLED. Knowledge indexing preserves a valid USER actor only within the target organization, otherwise scopes the background operation to a SYSTEM actor and restores the incoming context afterward. The original failure is now reproduced locally: running SlackRequestTest::test_validate_using_sdk followed by test_detect_function_change_points_groups_viewer_context_by_organization fails with UNKNOWN instead of SYSTEM and organization_id=None when #129727's isolation fixture is disabled. The same pair passes with it enabled. Validation: - Both changed test files plus the two originally failing statistical-detector tests: 76 passed, 1 xfailed. - Original two-test reproduction: 1 failed / 1 passed without isolation; 2 passed with isolation. - Required prek checks passed. - Current SCM and protocol packages were loaded from a temporary directory for local testing without changing the shared virtual environment. Only adjustments from the original patch are resolving conflicts with newer test imports/type annotations. This PR intentionally retains the original runtime behavior so full backend CI can validate it on top of the test-isolation fix. The draft requests the full suite via Trigger: Override Selective Testing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Tests that directly call request handlers can update the ViewerContext ContextVar without entering the middleware scope that restores it. The value then survives into later tests in the same worker. After #129261, a full backend run observed UNKNOWN actors in statistical-detector tests that expected organization-scoped SYSTEM actors; the PR's selective run did not include those detector tests. That change was subsequently reverted.
Add an autouse fixture that starts each test with no ViewerContext and restores the incoming value using its ContextVar token in a finally block. A subprocess regression checks isolation after both a successful UNKNOWN-context mutation and a failing USER-context mutation, and verifies restoration of an outer SYSTEM context at session exit. This prepares test isolation without reintroducing the reverted Slack/indexing change or changing production behavior.
Validation:
Production assessment: normal HTTP handling encloses Slack validation in ViewerContextMiddleware's restoring scope, so the cross-test contamination is not evidence of cross-request production leakage. Direct calls outside a restoring scope can still retain context, and the detector's existing handling of inherited UNKNOWN contexts is unchanged. This PR addresses test isolation only.