Skip to content

Commit

Permalink
parsemail: Ensure that the patch order cannot be None
Browse files Browse the repository at this point in the history
find_patch_order() was starting with order = None and trying to retrieve
the order from the previous revision. That doesn't work when there's no
previous revision, because patchwork didn't know about series when the
previous patch was sent.

The test case triggering the failure is actually a bit involved. It
needs series as reply to a couple of revision of a single patch, with
the part of thread before series support. The test case has the full
story.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
  • Loading branch information
Damien Lespiau committed Nov 10, 2015
1 parent 1eaca20 commit 4da2a45
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 3 deletions.
5 changes: 2 additions & 3 deletions patchwork/bin/parsemail.py
Expand Up @@ -439,10 +439,9 @@ def find_previous_patch(revision, order, refs):

return None

def find_patch_order(revisions, previous_patch):
def find_patch_order(revisions, previous_patch, order):
# cycle through revisions starting by the more recent one and find
# the revision where previous_patch is
order = None
for revision in revisions:
try:
order = SeriesRevisionPatch.objects.get(revision=revision,
Expand Down Expand Up @@ -477,7 +476,7 @@ def find_series_for_mail(project, name, msgid, is_patch, order, refs):
if is_patch:
previous_patch = find_previous_patch(revision, order, refs)
if previous_patch:
order = find_patch_order(revisions, previous_patch)
order = find_patch_order(revisions, previous_patch, order)
revision = revision.duplicate(exclude_patches=(order,))
# series has been updated, grab the new instance
series = revision.series
Expand Down
63 changes: 63 additions & 0 deletions patchwork/tests/test_series.py
Expand Up @@ -400,6 +400,19 @@ def testNoCoverLetterUpdateLen3Patch3(self):
class SinglePatchUpdatesVariousCornerCasesTest(TestCase):
fixtures = ['default_states', 'default_events']

def setMessageId(self, mail, msgid):
del mail['Message-Id']
mail['Message-Id'] = msgid

def setParentMail(self, mail, parent_mail, references=[]):
del mail['References']
del mail['In-Reply-To']
mail['In-Reply-To'] = parent_mail.get('Message-Id')
if not references:
mail['References'] = parent_mail.get('Message-Id')
return
mail['References'] = ' '.join([m.get('Message-Id') for m in references])

def testSinglePatchUpdatesNotSerialized(self):
""" + patch v1
+--> patch v2
Expand Down Expand Up @@ -473,6 +486,56 @@ def testSinglePatchUpdateNoCoverLetterNoSeriesMarker(self):
self.assertEqual(patches[0].name, '[1/2] ' + defaults.patch_name)
self.assertEqual(patches[1].name, '[v2] ' + defaults.patch_name)

def testSeriesAsReplyofSinglePatchCrossSeriesAwarePatchwork(self):
""" + initia_patch \
+--+ reply_1 | Before patchwork knew
+--+ patch_v2 | about series
+--+ reply_2 /
+--+ cover letter (0/3) \
+--> patch 1/3 | After patchwork knew
+--> patch 2/3 | about series
+--> patch 3/3 /
"""
initial_series = TestSeries(1, has_cover_letter=False)
mails = initial_series.create_mails()
initial_patch = mails[0]
self.setMessageId(initial_patch, 'initial_patch')

reply_1 = initial_series.create_reply(initial_patch)
mails.append(reply_1)
self.setMessageId(reply_1, 'reply_1')

patch_v2 = initial_series.create_patch(in_reply_to=reply_1,
subject_prefix="PATCH v2")
mails.append(patch_v2)

reply_2 = initial_series.create_reply(patch_v2)
mails.append(reply_2)
self.setMessageId(reply_2, 'reply_2')

initial_series.insert(mails)

# deleting the Series objects so far simulates the transition to
# a series-aware patchwork from a previous version not creating
# Series objects.
Series.objects.all().delete()

series = TestSeries(3)
series_mails = series.create_mails()
self.setMessageId(series_mails[0], 'cover_letter')
self.setParentMail(series_mails[0], reply_2)
for mail in series_mails[1:]:
self.setParentMail(mail, series_mails[0],
references=(reply_2, series_mails[0]))
series.insert(series_mails)

series = Series.objects.all()
self.assertEqual(len(series), 1)
revisions = series[0].revisions()
# FIXME: We don't treat a series sent as a reply as a single
# entity. Something that will need fixing.
self.assertEqual(len(revisions), 3)

#
# New version of a full series (separate mail thread)
#
Expand Down

0 comments on commit 4da2a45

Please sign in to comment.