From patchwork Fri Sep 21 17:51:04 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stephen Finucane X-Patchwork-Id: 973379 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [203.11.71.2]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 42H1R04ptnz9sCS for ; Sat, 22 Sep 2018 03:53:00 +1000 (AEST) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=that.guru Authentication-Results: ozlabs.org; dkim=fail reason="key not found in DNS" (0-bit key; unprotected) header.d=that.guru header.i=@that.guru header.b="W+zKc/eR"; dkim-atps=neutral Received: from bilbo.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 42H1R039HLzF3Rv for ; Sat, 22 Sep 2018 03:53:00 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dmarc=none (p=none dis=none) header.from=that.guru Authentication-Results: lists.ozlabs.org; dkim=fail reason="key not found in DNS" (0-bit key; unprotected) header.d=that.guru header.i=@that.guru header.b="W+zKc/eR"; dkim-atps=neutral X-Original-To: patchwork@lists.ozlabs.org Delivered-To: patchwork@lists.ozlabs.org Authentication-Results: lists.ozlabs.org; spf=none (mailfrom) smtp.mailfrom=that.guru (client-ip=185.234.75.7; helo=relay-direct7.mxroute.com; envelope-from=stephen@that.guru; receiver=) Authentication-Results: lists.ozlabs.org; dmarc=none (p=none dis=none) header.from=that.guru Authentication-Results: lists.ozlabs.org; dkim=fail reason="key not found in DNS" (0-bit key; unprotected) header.d=that.guru header.i=@that.guru header.b="W+zKc/eR"; dkim-atps=neutral Received: from relay-direct7.mxroute.com (relay-direct7.mxroute.com [185.234.75.7]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 42H1PZ3r7XzF3R7 for ; Sat, 22 Sep 2018 03:51:45 +1000 (AEST) Received: from filter002.mxroute.com (unknown [185.133.192.179]) by relay-direct7.mxroute.com (Postfix) with ESMTP id 74CDE3F0FD for ; Fri, 21 Sep 2018 17:51:12 +0000 (UTC) Received: from one.mxroute.com (one.mxroute.com [195.201.59.211]) by filter002.mxroute.com (Postfix) with ESMTPS id 59F473F39E for ; Fri, 21 Sep 2018 17:51:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=that.guru; s=default; h=References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From: Sender:Reply-To:MIME-Version:Content-Type:Content-Transfer-Encoding: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=m56TNNvY7asTFq4rsE/Pkbpn/herGo6tcnsGmIKSmwY=; b=W+zKc/eRYiz7CQndE6uQDgYNaB zwKu+N2Dk5TIIjOAd7t5srKIwZ8pNTaujdaDgH9tHjiHBQ4JQgaUD3ucYJKzQD/WAqvbp2NRV8RsW 5v7Y4brN8qtJy2KmAI9Ei5z1fg9xuslm1n4Lw5U7Xk8dWnnGG31Lsj+e286nXkw89zRvnxr0nqn7y EMvgubstB06NVaAndxGHbHv49iYZEv/gwiOED2dL0wDfhDFtTPHyPofjF8B6d9VFdity+em/3i4U6 logUStZS7pePBv+pF2VsKZSGVKupXvhkJj7fQ38yW6STaag3DCUYy+ysYJZRfO9pFkf80ubOpy1hs 9GhZ8g2Q==; From: Stephen Finucane To: patchwork@lists.ozlabs.org Subject: [PATCH v3 4/5] tests: Remove 'create_series_patch' Date: Fri, 21 Sep 2018 18:51:04 +0100 Message-Id: <20180921175105.18000-5-stephen@that.guru> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20180921175105.18000-1-stephen@that.guru> References: <20180921175105.18000-1-stephen@that.guru> X-AuthUser: stephen@that.guru X-BeenThere: patchwork@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Patchwork development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: patchwork-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: "Patchwork" 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 --- patchwork/tests/api/test_series.py | 12 +++---- patchwork/tests/test_events.py | 34 ++++++++++---------- patchwork/tests/test_mboxviews.py | 28 ++++++++-------- patchwork/tests/utils.py | 51 ++++++++++++++++-------------- 4 files changed, 65 insertions(+), 60 deletions(-) diff --git a/patchwork/tests/api/test_series.py b/patchwork/tests/api/test_series.py index 4d576fc0..21ce2547 100644 --- 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 bd4f9be1..7d03d65d 100644 --- 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 9de9c762..5c29226b 100644 --- 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 0c3bbff2..c8acda40 100644 --- 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 = {