Repository navigation
chore(ACI): Add back subscription processor test coverage - #105113
Merged
ceorourke merged 9 commits intoDec 19, 2025
Merged
Conversation
ceorourke
commented
Dec 17, 2025
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #105113 +/- ##
========================================
Coverage 80.61% 80.61%
========================================
Files 9422 9422
Lines 404017 404017
Branches 25654 25654
========================================
+ Hits 325711 325717 +6
+ Misses 77837 77831 -6
Partials 469 469 |
ceorourke
force-pushed
the
ceorourke/add-back-subscription-processor-test-coverage
branch
from
December 17, 2025 19:01
839436e to
9fa6aae
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| "sentry.seer.anomaly_detection.get_anomaly_data.SEER_ANOMALY_DETECTION_CONNECTION_POOL.urlopen" | ||
| ) | ||
| def test_triggers_performance_detector(self, mock_seer_request: mock.MagicMock) -> None: | ||
| self.snuba_query.update(time_window=15 * 60, dataset=Dataset.Transactions) |
Contributor
There was a problem hiding this comment.
Bug: Missing .value when passing Dataset enum to update
The dataset=Dataset.Transactions is passed without .value. The SnubaQuery.dataset field is a TextField expecting a string like "transactions". Without .value, Python's str() converts the enum to "Dataset.Transactions" instead. Other similar calls in test_subscription_processor_aci.py correctly use Dataset.Transactions.value. This will cause incorrect data to be stored in the database, potentially making the test behave unexpectedly or test the wrong scenario.
This was referenced Dec 18, 2025
ceorourke
deleted the
ceorourke/add-back-subscription-processor-test-coverage
branch
December 19, 2025 17:41
ceorourke
added a commit
that referenced
this pull request
Dec 19, 2025
Another companion to #105113 to add back the upsampled alerts subscription processor tests but updates them for ACI.
This branch was successfully deployed
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
Add missing test coverage for the subscription processor. Many tests we still need were deleted and need to be recovered but updated for ACI.
Add missing tests for early returns from here
Add missing anomaly detection test coverage from here
TODO in future PRs:
Add missing crash rate alert tests from here chore(ACI): Add back subscription processor crash rate detector test coverage #105205
Add missing upsample tests from here chore(ACI): Add back upsampled tests #105272
Nonefromget_eap_aggregation_valueso I will ask Shruthi post holidays) from here