Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests(similarity): Simplify tests #82031

Merged
merged 13 commits into from
Dec 13, 2024
Merged

Conversation

armenzg
Copy link
Member

@armenzg armenzg commented Dec 12, 2024

This removes a lot of the assertion for mocked logs.

@armenzg armenzg self-assigned this Dec 12, 2024
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Dec 12, 2024
@@ -682,16 +681,6 @@ def _retry_operation(operation, *args, retries, delay, exceptions, **kwargs):
raise


# TODO: delete this and its tests
def lookup_event(project_id: int, event_id: str, group_id: int) -> Event:
Copy link
Member Author

Choose a reason for hiding this comment

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

Not in use.

bulk_data["rows"],
bulk_data["events"],
)
self.event = self.store_event(
Copy link
Member Author

Choose a reason for hiding this comment

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

This gets replaced by self.event = bulk_data["events"][0].

self.event.group.times_seen = 5
self.event.group.save()

self.wait_for_event_count(self.project.id, 6)
Copy link
Member Author

Choose a reason for hiding this comment

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

This is not necessary and it speeds up a tiny bit the execution of the tests.

"similarity_model_version": SEER_SIMILARITY_MODEL_VERSION,
"request_hash": self.group_hashes[group.id],
}
self.assert_groups_metadata_updated(groups)
Copy link
Member Author

Choose a reason for hiding this comment

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

Testing the final state rather than the output of the logger.

tests/sentry/tasks/test_backfill_seer_grouping_records.py Outdated Show resolved Hide resolved
@@ -1199,13 +882,13 @@ def test_backfill_seer_grouping_records_multiple_batches(
"backfill_seer_grouping_records.batch",
extra={
"project_id": self.project.id,
"batch_len": 6,
"batch_len": 5,
Copy link
Member Author

Choose a reason for hiding this comment

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

This changes because in the set up we don't create an extra event.

@@ -1268,15 +951,14 @@ def test_backfill_seer_grouping_records_only_delete(self, mock_project_delete_gr
assert group.data["metadata"] == default_metadata

@patch("sentry.tasks.embeddings_grouping.utils.delete_project_grouping_records")
@patch("sentry.tasks.embeddings_grouping.backfill_seer_grouping_records_for_project.logger")
@patch("sentry.tasks.embeddings_grouping.utils.post_bulk_grouping_records")
Copy link
Member Author

Choose a reason for hiding this comment

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

We're going to call the task first to populate the metadata.

@armenzg armenzg marked this pull request as ready for review December 12, 2024 20:21
@armenzg armenzg requested a review from a team as a code owner December 12, 2024 20:21
@armenzg armenzg enabled auto-merge (squash) December 13, 2024 12:01
@armenzg armenzg merged commit 63d940e into master Dec 13, 2024
48 checks passed
@armenzg armenzg deleted the ref/similarity/backfill/armenzg branch December 13, 2024 12:28
Copy link

sentry-io bot commented Dec 13, 2024

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ AssertionError: assert {'request_has...ersion': 'v0'} == {'request_has...ersion': 'v0'} pytest.runtest.protocol tests/sentry/tasks/test... View Issue
  • ‼️ AssertionError: assert {'request_has...ersion': 'v0'} == {'request_has...ersion': 'v0'} pytest.runtest.protocol tests/sentry/tasks/test... View Issue
  • ‼️ AssertionError: assert {'request_has...ersion': 'v0'} == {'request_has...ersion': 'v0'} pytest.runtest.protocol tests/sentry/tasks/test... View Issue
  • ‼️ AssertionError: assert {'request_has...ersion': 'v0'} == {'request_has...ersion': 'v0'} pytest.runtest.protocol tests/sentry/tasks/test... View Issue
  • ‼️ AssertionError: assert {'request_has...ersion': 'v0'} == {'request_has...ersion': 'v0'} pytest.runtest.protocol tests/sentry/tasks/test... View Issue

Did you find this useful? React with a 👍 or 👎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants