tests: Remove 'create_series_patch'

The 'SeriesPatch' object was recently removed, but the
'create_series_patch' was retained in order to minimize the changes
necessary. This can now be removed and the logic moved to the
'create_patch' and 'create_cover' functions instead.

Signed-off-by: Stephen Finucane <stephen@that.guru>

diff --git a/patchwork/tests/api/test_series.py b/patchwork/tests/api/test_series.py
index 4d576fc0154b0db3bdf2fd1aa922ee3cce495bbc..21ce2547906f5fbdb453c7749e3e6fa70ffe46d3 100644 (file)
--- a/patchwork/tests/api/test_series.py
+++ b/patchwork/tests/api/test_series.py
@@ -10,10 +10,10 @@ from django.urls import reverse
 
 from patchwork.tests.utils import create_cover
 from patchwork.tests.utils import create_maintainer
-from patchwork.tests.utils import create_project
+from patchwork.tests.utils import create_patch
 from patchwork.tests.utils import create_person
+from patchwork.tests.utils import create_project
 from patchwork.tests.utils import create_series
-from patchwork.tests.utils import create_series_patch
 from patchwork.tests.utils import create_user
 
 if settings.ENABLE_REST_API:
@@ -69,10 +69,9 @@ class TestSeriesAPI(APITestCase):
 
         project_obj = create_project(linkname='myproject')
         person_obj = create_person(email='test@example.com')
-        cover_obj = create_cover()
         series_obj = create_series(project=project_obj, submitter=person_obj)
-        series_obj.add_cover_letter(cover_obj)
-        create_series_patch(series=series_obj)
+        create_cover(series=series_obj)
+        create_patch(series=series_obj)
 
         # anonymous users
         resp = self.client.get(self.api_url())
@@ -118,9 +117,8 @@ class TestSeriesAPI(APITestCase):
 
     def test_detail(self):
         """Validate we can get a specific series."""
-        cover = create_cover()
         series = create_series()
-        series.add_cover_letter(cover)
+        create_cover(series=series)
 
         resp = self.client.get(self.api_url(series.id))
         self.assertEqual(status.HTTP_200_OK, resp.status_code)

diff --git a/patchwork/tests/test_events.py b/patchwork/tests/test_events.py
index bd4f9be1d388d2844e40c8f638e564bed0382183..7d03d65db31537803f67e8e32369451bccd40c76 100644 (file)
--- a/patchwork/tests/test_events.py
+++ b/patchwork/tests/test_events.py
@@ -44,45 +44,46 @@ class PatchCreateTest(_BaseTestCase):
 
     def test_patch_dependencies_present_series(self):
         """Patch dependencies already exist."""
-        series_patch = utils.create_series_patch()
+        series = utils.create_series()
+        patch = utils.create_patch(series=series)
 
         # This should raise both the CATEGORY_PATCH_CREATED and
         # CATEGORY_PATCH_COMPLETED events
-        events = _get_events(patch=series_patch.patch)
+        events = _get_events(patch=patch)
         self.assertEqual(events.count(), 2)
         self.assertEqual(events[0].category, Event.CATEGORY_PATCH_CREATED)
-        self.assertEqual(events[0].project, series_patch.patch.project)
+        self.assertEqual(events[0].project, patch.project)
         self.assertEqual(events[1].category, Event.CATEGORY_PATCH_COMPLETED)
-        self.assertEqual(events[1].project, series_patch.patch.project)
+        self.assertEqual(events[1].project, patch.project)
         self.assertEventFields(events[0])
         self.assertEventFields(events[1])
 
         # This shouldn't be affected by another update to the patch
-        series_patch.patch.commit_ref = 'aac76f0b0f8dd657ff07bb'
-        series_patch.patch.save()
+        patch.commit_ref = 'aac76f0b0f8dd657ff07bb32df369705696d4831'
+        patch.save()
 
-        events = _get_events(patch=series_patch.patch)
+        events = _get_events(patch=patch)
         self.assertEqual(events.count(), 2)
 
     def test_patch_dependencies_out_of_order(self):
         series = utils.create_series()
-        series_patch_3 = utils.create_series_patch(series=series, number=3)
-        series_patch_2 = utils.create_series_patch(series=series, number=2)
+        patch_3 = utils.create_patch(series=series, number=3)
+        patch_2 = utils.create_patch(series=series, number=2)
 
         # This should only raise the CATEGORY_PATCH_CREATED event for
         # both patches as they are both missing dependencies
-        for series_patch in [series_patch_2, series_patch_3]:
-            events = _get_events(patch=series_patch.patch)
+        for patch in [patch_2, patch_3]:
+            events = _get_events(patch=patch)
             self.assertEqual(events.count(), 1)
             self.assertEqual(events[0].category, Event.CATEGORY_PATCH_CREATED)
             self.assertEventFields(events[0])
 
-        series_patch_1 = utils.create_series_patch(series=series, number=1)
+        patch_1 = utils.create_patch(series=series, number=1)
 
         # We should now see the CATEGORY_PATCH_COMPLETED event for all patches
         # as the dependencies for all have been met
-        for series_patch in [series_patch_1, series_patch_2, series_patch_3]:
-            events = _get_events(patch=series_patch.patch)
+        for patch in [patch_1, patch_2, patch_3]:
+            events = _get_events(patch=patch)
             self.assertEqual(events.count(), 2)
             self.assertEqual(events[0].category, Event.CATEGORY_PATCH_CREATED)
             self.assertEqual(events[1].category,
@@ -91,11 +92,12 @@ class PatchCreateTest(_BaseTestCase):
             self.assertEventFields(events[1])
 
     def test_patch_dependencies_missing(self):
-        series_patch = utils.create_series_patch(number=2)
+        series = utils.create_series()
+        patch = utils.create_patch(series=series, number=2)
 
         # This should only raise the CATEGORY_PATCH_CREATED event as
         # there is a missing dependency (patch 1)
-        events = _get_events(patch=series_patch.patch)
+        events = _get_events(patch=patch)
         self.assertEqual(events.count(), 1)
         self.assertEqual(events[0].category, Event.CATEGORY_PATCH_CREATED)
         self.assertEventFields(events[0])

diff --git a/patchwork/tests/test_mboxviews.py b/patchwork/tests/test_mboxviews.py
index 63191a17822e5bced18094e8712a31438cdb5af1..50444d65453f3c99950d89300fe903f0353c43e4 100644 (file)
--- a/patchwork/tests/test_mboxviews.py
+++ b/patchwork/tests/test_mboxviews.py
@@ -18,7 +18,6 @@ from patchwork.tests.utils import create_patch
 from patchwork.tests.utils import create_project
 from patchwork.tests.utils import create_person
 from patchwork.tests.utils import create_series
-from patchwork.tests.utils import create_series_patch
 from patchwork.tests.utils import create_user
 
 
@@ -200,28 +199,29 @@ class MboxSeriesDependencies(TestCase):
 
     @staticmethod
     def _create_patches():
-        patch_a = create_series_patch()
-        patch_b = create_series_patch(series=patch_a.series)
+        series = create_series()
+        patch_a = create_patch(series=series)
+        patch_b = create_patch(series=series)
 
-        return patch_a.series, patch_a, patch_b
+        return series, patch_a, patch_b
 
     def test_patch_with_wildcard_series(self):
         _, patch_a, patch_b = self._create_patches()
 
         response = self.client.get('%s?series=*' % reverse(
-            'patch-mbox', args=[patch_b.patch.id]))
+            'patch-mbox', args=[patch_b.id]))
 
-        self.assertContains(response, patch_a.patch.content)
-        self.assertContains(response, patch_b.patch.content)
+        self.assertContains(response, patch_a.content)
+        self.assertContains(response, patch_b.content)
 
     def test_patch_with_numeric_series(self):
         series, patch_a, patch_b = self._create_patches()
 
         response = self.client.get('%s?series=%d' % (
-            reverse('patch-mbox', args=[patch_b.patch.id]), series.id))
+            reverse('patch-mbox', args=[patch_b.id]), series.id))
 
-        self.assertContains(response, patch_a.patch.content)
-        self.assertContains(response, patch_b.patch.content)
+        self.assertContains(response, patch_a.content)
+        self.assertContains(response, patch_b.content)
 
     def test_patch_with_invalid_series(self):
         series, patch_a, patch_b = self._create_patches()
@@ -247,10 +247,10 @@ class MboxSeries(TestCase):
 
     def test_series(self):
         series = create_series()
-        patch_a = create_series_patch(series=series)
-        patch_b = create_series_patch(series=series)
+        patch_a = create_patch(series=series)
+        patch_b = create_patch(series=series)
 
         response = self.client.get(reverse('series-mbox', args=[series.id]))
 
-        self.assertContains(response, patch_a.patch.content)
-        self.assertContains(response, patch_b.patch.content)
+        self.assertContains(response, patch_a.content)
+        self.assertContains(response, patch_b.content)

diff --git a/patchwork/tests/utils.py b/patchwork/tests/utils.py
index 0c3bbff2ab5797e729bb9e5017255ebb38b74f52..c8acda405cca0db1983e70bd629a6f9d8bddb055 100644 (file)
--- a/patchwork/tests/utils.py
+++ b/patchwork/tests/utils.py
@@ -149,6 +149,12 @@ def create_patch(**kwargs):
     """Create 'Patch' object."""
     num = Patch.objects.count()
 
+    # NOTE(stephenfin): Even though we could simply pass 'series' into the
+    # constructor, we don't as that's not what we do in the parser and not what
+    # our signal handlers (for events) expect
+    series = kwargs.pop('series', None)
+    number = kwargs.pop('number', None)
+
     values = {
         'submitter': create_person() if 'submitter' not in kwargs else None,
         'delegate': None,
@@ -161,16 +167,31 @@ def create_patch(**kwargs):
         'diff': SAMPLE_DIFF,
     }
     values.update(kwargs)
+
     if 'patch_project' not in values:
         values['patch_project'] = values['project']
 
-    return Patch.objects.create(**values)
+    patch = Patch.objects.create(**values)
+
+    if series:
+        number = number or series.patches.count() + 1
+        series.add_patch(patch, number)
+
+    return patch
 
 
 def create_cover(**kwargs):
     """Create 'CoverLetter' object."""
     num = CoverLetter.objects.count()
 
+    # NOTE(stephenfin): Despite first appearances, passing 'series' to the
+    # 'create' function doesn't actually cause the relationship to be created.
+    # This is probably a bug in Django. However, it's convenient to do so we
+    # emulate that here. For more info, see [1].
+    #
+    # [1] https://stackoverflow.com/q/43119575/
+    series = kwargs.pop('series', None)
+
     values = {
         'submitter': create_person() if 'person' not in kwargs else None,
         'project': create_project() if 'project' not in kwargs else None,
@@ -181,7 +202,12 @@ def create_cover(**kwargs):
     }
     values.update(kwargs)
 
-    return CoverLetter.objects.create(**values)
+    cover = CoverLetter.objects.create(**values)
+
+    if series:
+        series.add_cover_letter(cover)
+
+    return cover
 
 
 def create_comment(**kwargs):
@@ -226,27 +252,6 @@ def create_series(**kwargs):
     return Series.objects.create(**values)
 
 
-def create_series_patch(**kwargs):
-    """Create 'Patch' object and associate with a series."""
-    # TODO(stephenfin): Remove this and all callers
-    num = 1 if 'series' not in kwargs else kwargs['series'].patches.count() + 1
-    if 'number' in kwargs:
-        num = kwargs['number']
-
-    series = create_series() if 'series' not in kwargs else kwargs['series']
-    patch = create_patch() if 'patch' not in kwargs else kwargs['patch']
-
-    series.add_patch(patch, num)
-
-    class SeriesPatch(object):
-        """Simple wrapper to avoid needing to update all tests at once."""
-        def __init__(self, series, patch):
-            self.series = series
-            self.patch = patch
-
-    return SeriesPatch(series=series, patch=patch)
-
-
 def create_series_reference(**kwargs):
     """Create 'SeriesReference' object."""
     values = {