Skip to content

Conversation

@thetruecpaul
Copy link
Contributor

Hotlined the fix in #103177 because it was causing problems. Best practice is to add a test here to ensure we don't regress in the future.

Hotlined the fix in #103177 because it was causing problems. Best practice is to add a test here to ensure we don't regress in the future.
@thetruecpaul thetruecpaul requested review from a team and scttcper November 11, 2025 21:09
@thetruecpaul thetruecpaul enabled auto-merge (squash) November 11, 2025 21:09
conditions=conditions,
)
assert filter_keys == {"group_id": {g4.id}}
assert conditions == [["group_id", "IN", [g1.id, g4.id]]]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Test Fails to Verify Preprocessing Logic

The test doesn't enable the snuba.preprocess-group-redirects option before creating SnubaQueryParams, so _preprocess_group_id_redirects won't execute. This means the test doesn't actually verify that inputs aren't mutated during group redirect preprocessing, which is its intended purpose based on the comment and PR description.

Fix in Cursor Fix in Web

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line 329

options.set("snuba.preprocess-group-redirects", True)

check yourself.

@thetruecpaul thetruecpaul merged commit a692a69 into master Nov 11, 2025
64 checks passed
@thetruecpaul thetruecpaul deleted the cpaul/111125/snubanomutatetest branch November 11, 2025 21:32
Jesse-Box pushed a commit that referenced this pull request Nov 12, 2025
Hotlined the fix in #103177 because it was causing problems. Best
practice is to add a test here to ensure we don't regress in the future.
andrewshie-sentry pushed a commit that referenced this pull request Nov 13, 2025
Hotlined the fix in #103177 because it was causing problems. Best
practice is to add a test here to ensure we don't regress in the future.
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.

3 participants