Skip to content

tests(resolve_groups): Clean up the tests #91779

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

Merged
merged 5 commits into from
May 20, 2025
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions tests/sentry/integrations/github/test_webhooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ def setUp(self):
self.secret = "b3002c3e321d4b7880360d397db2ccfd"
options.set("github-app.webhook-secret", self.secret)

def _setup_repo_test(self, project):
def _create_integration_and_send_push_event(self):
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 makes it clearer what this helper function does.

future_expires = datetime.now().replace(microsecond=0) + timedelta(minutes=5)
with assume_test_silo_mode(SiloMode.CONTROL):
integration = self.create_integration(
Expand Down Expand Up @@ -305,7 +305,7 @@ def test_simple(self, mock_record):
name="baxterthehacker/repo",
)

self._setup_repo_test(self.project)
self._create_integration_and_send_push_event()

commit_list = list(
Commit.objects.filter(
Expand Down Expand Up @@ -349,7 +349,7 @@ def test_simple(self, mock_record):
@responses.activate
@patch("sentry.integrations.github.webhook.metrics")
def test_creates_missing_repo(self, mock_metrics):
self._setup_repo_test(self.project)
self._create_integration_and_send_push_event()

repos = Repository.objects.all()
assert len(repos) == 1
Expand All @@ -369,7 +369,7 @@ def test_ignores_hidden_repo(self):
repo.external_id = "35129377"
repo.save()

self._setup_repo_test(self.project)
self._create_integration_and_send_push_event()

repos = Repository.objects.all()
assert len(repos) == 1
Expand All @@ -391,7 +391,7 @@ def test_anonymous_lookup(self):
name="bàxterthehacker",
)

self._setup_repo_test(self.project)
self._create_integration_and_add_send_push_event()

commit_list = list(
Commit.objects.filter(organization_id=self.project.organization.id)
Expand Down Expand Up @@ -576,7 +576,7 @@ def setUp(self):
self.secret = "b3002c3e321d4b7880360d397db2ccfd"
options.set("github-app.webhook-secret", self.secret)

def _setup_repo_test(self, project):
def _create_integration_and_send_pull_request_opened_event(self):
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 makes it clearer what this helper function does.

future_expires = datetime.now().replace(microsecond=0) + timedelta(minutes=5)
with assume_test_silo_mode(SiloMode.CONTROL):
integration = self.create_integration(
Expand Down Expand Up @@ -643,7 +643,7 @@ def test_opened(self, mock_record, mock_metrics, mock_open_pr_comment_workflow_d
name="baxterthehacker/public-repo",
)

self._setup_repo_test(self.project)
self._create_integration_and_send_pull_request_opened_event()

prs = PullRequest.objects.filter(
repository_id=repo.id, organization_id=self.project.organization.id
Expand Down Expand Up @@ -685,14 +685,14 @@ def test_opened_missing_option(self, mock_metrics):
organization=self.organization, key="sentry:github_open_pr_bot", value=False
)

self._setup_repo_test(self.project)
self._create_integration_and_send_pull_request_opened_event()

assert mock_metrics.incr.call_count == 0

@patch("sentry.integrations.github.webhook.metrics")
def test_creates_missing_repo(self, mock_metrics):

self._setup_repo_test(self.project)
self._create_integration_and_send_pull_request_opened_event()

repos = Repository.objects.all()
assert len(repos) == 1
Expand All @@ -713,7 +713,7 @@ def test_ignores_hidden_repo(self):
repo.external_id = "35129377"
repo.save()

self._setup_repo_test(self.project)
self._create_integration_and_send_pull_request_opened_event()

repos = Repository.objects.all()
assert len(repos) == 1
Expand Down
29 changes: 26 additions & 3 deletions tests/sentry/models/test_pullrequest.py
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 whole test we may want to delete, as I believe it may be converted by tests for event_manager or webhooks.

Original file line number Diff line number Diff line change
Expand Up @@ -2,35 +2,51 @@
from uuid import uuid4

from sentry.models.commit import Commit
from sentry.models.group import GroupStatus
from sentry.models.grouphistory import GroupHistory, GroupHistoryStatus
from sentry.models.grouplink import GroupLink
from sentry.models.pullrequest import PullRequest
from sentry.models.repository import Repository
from sentry.testutils.cases import TestCase


class FindReferencedGroupsTest(TestCase):
def test_multiple_matches_basic(self):
def test_multiple_matches_basic(self) -> None:
group = self.create_group()
group2 = self.create_group()

repo = Repository.objects.create(name="example", organization_id=self.group.organization.id)
repo = Repository.objects.create(name="example", organization_id=group.organization.id)

commit = Commit.objects.create(
key=sha1(uuid4().hex.encode("utf-8")).hexdigest(),
repository_id=repo.id,
organization_id=group.organization.id,
# It makes reference to the first group
message=f"Foo Biz\n\nFixes {group.qualified_short_id}",
)

groups = commit.find_referenced_groups()
assert len(groups) == 1
assert group in groups
assert GroupHistory.objects.filter(
group=group,
status=GroupHistoryStatus.SET_RESOLVED_IN_COMMIT,
).exists()
# This gets created in resolved_in_commit
assert GroupLink.objects.filter(
group=group,
linked_type=GroupLink.LinkedType.commit,
linked_id=commit.id,
).exists()
group.refresh_from_db()
assert group.status == GroupStatus.RESOLVED

pr = PullRequest.objects.create(
key="1",
repository_id=repo.id,
organization_id=group.organization.id,
title="very cool PR to fix the thing",
# It makes reference to the second group
message=f"Foo Biz\n\nFixes {group2.qualified_short_id}",
)

Expand All @@ -40,4 +56,11 @@ def test_multiple_matches_basic(self):
assert GroupHistory.objects.filter(
group=group2,
status=GroupHistoryStatus.SET_RESOLVED_IN_PULL_REQUEST,
)
).exists()
assert GroupLink.objects.filter(
group=group2,
linked_type=GroupLink.LinkedType.pull_request,
linked_id=pr.id,
).exists()
group.refresh_from_db()
assert group.status == GroupStatus.RESOLVED
Loading