From dad54bd53a41b8055f535dcd16fd2eb897e79f09 Mon Sep 17 00:00:00 2001 From: Klaas van Schelven Date: Thu, 14 Dec 2023 22:21:43 +0100 Subject: [PATCH] Tests (and testability) of is_issue_regression --- ingest/views.py | 25 ++++---------- issues/models.py | 24 ++++++++++++++ issues/regressions.py | 17 +++++----- issues/tests.py | 76 +++++++++++++++++++++++++++++++++++++++++-- issues/views.py | 17 +++------- releases/models.py | 20 ++++++++++++ 6 files changed, 138 insertions(+), 41 deletions(-) diff --git a/ingest/views.py b/ingest/views.py index 6d526b1..25223e0 100644 --- a/ingest/views.py +++ b/ingest/views.py @@ -11,12 +11,12 @@ from rest_framework import exceptions from compat.auth import parse_auth_header_value from projects.models import Project -from issues.models import Issue +from issues.models import Issue, IssueResolver from issues.utils import get_hash_for_data -from issues.regressions import event_is_regression +from issues.regressions import issue_is_regression from events.models import Event -from releases.models import Release +from releases.models import create_release_if_needed from .negotiation import IgnoreClientContentNegotiation from .parsers import EnvelopeParser @@ -66,19 +66,7 @@ class BaseIngestAPIView(APIView): if not event_created: return - # NOTE: we even create a Release for the empty release here; we need the associated info (date_released) if a - # real release is ever created later. - release, release_created = Release.objects.get_or_create(project=project, version=event.release) - if release_created and event.release != "": - if not project.has_releases: - project.has_releases = True - project.save() - - if release == project.get_latest_release(): - for bnr_issue in Issue.objects.filter(project=project, is_resolved_by_next_release=True): - bnr_issue.add_fixed_at(release) - bnr_issue.is_resolved_by_next_release = False - bnr_issue.save() + create_release_if_needed(project, event.release) hash_ = get_hash_for_data(event_data) @@ -91,11 +79,12 @@ class BaseIngestAPIView(APIView): if issue_created: pass # alerting code goes here - elif event_is_regression(event): # new issues cannot be regressions by definition, hence the 'else' + elif issue_is_regression(issue, event.release): # new issues cannot be regressions by definition, hence 'else' pass # alerting code goes here - issue.is_resolved = False + IssueResolver.reopen(issue) # TODO bookkeeping of events_at goes here. + issue.save() class IngestEventAPIView(BaseIngestAPIView): diff --git a/issues/models.py b/issues/models.py index 2120b95..c360d21 100644 --- a/issues/models.py +++ b/issues/models.py @@ -69,3 +69,27 @@ class Issue(models.Model): def occurs_in_last_release(self): return False # TODO actually implement (and then: implement in a performant manner) + + +class IssueResolver(object): + """basically: a namespace""" + + @staticmethod + def resolve(issue): + issue.is_resolved = True + + @staticmethod + def resolve_by_latest(issue): + issue.is_resolved = True + issue.add_fixed_at(issue.project.get_latest_release()) + + @staticmethod + def resolve_by_next(issue): + issue.is_resolved = True + issue.is_resolved_by_next_release = True + + @staticmethod + def reopen(issue): + issue.is_resolved = False + issue.is_resolved_by_next_release = False # ?? echt? + # TODO and what about fixed_at ? diff --git a/issues/regressions.py b/issues/regressions.py index 070118e..67cac16 100644 --- a/issues/regressions.py +++ b/issues/regressions.py @@ -25,22 +25,21 @@ def is_regression(sorted_releases, fixed_at, events_at, current_event_at): raise Exception("Can't find release '%s'" % current_event_at) -def event_is_regression(event): - if not event.is_resolved: +def issue_is_regression(issue, current_event_at): + if not issue.is_resolved: return False - if event.is_resolved_by_next_release: + if issue.is_resolved_by_next_release: # i.e. this is solved, but only "in the future". The assumption (which is true in our code) here is: once this # point is reached, all "actually seen releases" will have already been accounted for. return False - if not event.project.has_releases: - return True # i.e. `return event.is_resolved`, which is True if this point is reached. + if not issue.project.has_releases: + return True # i.e. `return issue.is_resolved`, which is True if this point is reached. - sorted_releases = [r.version for r in ordered_releases(project=event.project)] - fixed_at = event.get_fixed_at() - events_at = event.get_events_at() - current_event_at = event.release + sorted_releases = [r.version for r in ordered_releases(project=issue.project)] + fixed_at = issue.get_fixed_at() + events_at = issue.get_events_at() return is_regression(sorted_releases, fixed_at, events_at, current_event_at) diff --git a/issues/tests.py b/issues/tests.py index 68759b4..119b756 100644 --- a/issues/tests.py +++ b/issues/tests.py @@ -1,10 +1,16 @@ from unittest import TestCase +from django.test import TestCase as DjangoTestCase -from .regressions import is_regression, is_regression_2 +from projects.models import Project +from releases.models import create_release_if_needed + +from .models import Issue, IssueResolver +from .regressions import is_regression, is_regression_2, issue_is_regression -class RegressionTestCase(TestCase): +class RegressionUtilTestCase(TestCase): # This tests the concept of "what is a regression?", it _does not_ test for regressions in our code :-) + # this particular testcase tests straight on the utility `is_regression` (i.e. not all issue-handling code) def setUp(self): self.releases = ["a", "b", "c", "d", "e", "f", "g", "h"] @@ -141,3 +147,69 @@ class RegressionTestCase(TestCase): # most recent major branch. (in the below, there is no fix on the 4.x branch reported, but a regression is # detected when 4.0.2 has the same problem it had in 4.0.1), i.e. the below should say 'assertFalse' self.assertTrue(is_regression(releases, ["3.1.2"], events_at, current_event_at="4.0.2")) + + +class RegressionIssueTestCase(DjangoTestCase): + # this particular testcase is more of an integration test: it tests the handling of issue objects. + + def test_issue_is_regression_no_releases(self): + project = Project.objects.create() + + # new issue is not a regression + issue = Issue.objects.create(project=project) + self.assertFalse(issue_is_regression(issue, "anything")) + + # resolve the issue, a reoccurrence is a regression + IssueResolver.resolve(issue) + issue.save() + self.assertTrue(issue_is_regression(issue, "anything")) + + # reopen the issue (as is done when a real regression is seen; or as would be done manually); nothing is a + # regression once the issue is open + IssueResolver.reopen(issue) + issue.save() + self.assertFalse(issue_is_regression(issue, "anything")) + + def test_issue_is_regression_with_releases_resolve_by_latest(self): + project = Project.objects.create() + + create_release_if_needed(project, "1.0.0") + create_release_if_needed(project, "2.0.0") + + # new issue is not a regression + issue = Issue.objects.create(project=project) + self.assertFalse(issue_is_regression(issue, "anything")) + + # resolve the by latest, reoccurrences of older releases are not regressions but occurrences by latest are + IssueResolver.resolve_by_latest(issue) + issue.save() + self.assertFalse(issue_is_regression(issue, "1.0.0")) + self.assertTrue(issue_is_regression(issue, "2.0.0")) + + # reopen the issue (as is done when a real regression is seen; or as would be done manually); nothing is a + # regression once the issue is open + IssueResolver.reopen(issue) + issue.save() + self.assertFalse(issue_is_regression(issue, "1.0.0")) + self.assertFalse(issue_is_regression(issue, "2.0.0")) + + def test_issue_is_regression_with_releases_resolve_by_next(self): + project = Project.objects.create() + + create_release_if_needed(project, "1.0.0") + create_release_if_needed(project, "2.0.0") + + # new issue is not a regression + issue = Issue.objects.create(project=project) + self.assertFalse(issue_is_regression(issue, "anything")) + + # resolve the by next, reoccurrences of any existing releases are not regressions + IssueResolver.resolve_by_next(issue) + issue.save() + self.assertFalse(issue_is_regression(issue, "1.0.0")) + self.assertFalse(issue_is_regression(issue, "2.0.0")) + + # a new release appears (as part of a new event); this is a regression + create_release_if_needed(project, "3.0.0") + issue_fresh = Issue.objects.get(pk=issue.pk) + self.assertTrue(issue_is_regression(issue_fresh, "3.0.0")) diff --git a/issues/views.py b/issues/views.py index 291849e..d42c32b 100644 --- a/issues/views.py +++ b/issues/views.py @@ -6,7 +6,7 @@ from events.models import Event from projects.models import Project from .utils import get_issue_grouper_for_data -from .models import Issue +from .models import Issue, IssueResolver def issue_list(request, project_id): @@ -31,20 +31,13 @@ def issue_event_detail(request, issue_pk, event_pk): if request.method == "POST": if request.POST["action"] == "resolved": - issue.is_resolved = True - + IssueResolver.resolve(issue) elif request.POST["action"] == "resolved_latest": - issue.is_resolved = True - issue.add_fixed_at(issue.project.get_latest_release()) - + IssueResolver.resolve_by_latest(issue) elif request.POST["action"] == "resolved_next": - issue.is_resolved = True - issue.is_resolved_by_next_release = True - + IssueResolver.resolve_by_next(issue) elif request.POST["action"] == "reopen": - issue.is_resolved = False - issue.is_resolved_by_next_release = False # ?? echt? - # TODO and what about fixed_at ? + IssueResolver.reopen(issue) elif request.POST["action"] == "mute": ... diff --git a/releases/models.py b/releases/models.py index 54e9895..4d978c8 100644 --- a/releases/models.py +++ b/releases/models.py @@ -6,6 +6,8 @@ from semver.version import Version from django.db import models from django.utils import timezone +from issues.models import Issue + RE_PACKAGE_VERSION = re.compile('((?P.*)[@])?(?P.*)') @@ -82,6 +84,24 @@ class Release(models.Model): return self.version[:12] +def create_release_if_needed(project, version): + # NOTE: we even create a Release for the empty release here; we need the associated info (date_released) if a + # real release is ever created later. + release, release_created = Release.objects.get_or_create(project=project, version=version) + if release_created and version != "": + if not project.has_releases: + project.has_releases = True + project.save() + + if release == project.get_latest_release(): + for bnr_issue in Issue.objects.filter(project=project, is_resolved_by_next_release=True): + bnr_issue.add_fixed_at(release) + bnr_issue.is_resolved_by_next_release = False + bnr_issue.save() + + return release + + # Some thoughts that should go into a proper doc-like location later: # # 1. The folllowing restrictions are not (yet?) replicated from Sentry: