This supports a lot more subject formats and better recognizes the patch series.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=48353 Signed-off-by: Francois Gouget fgouget@codeweavers.com --- testbot/lib/WineTestBot/Patches.pm | 78 ++++++- testbot/lib/WineTestBot/PendingPatchSets.pm | 10 +- testbot/tests/TestPatches | 220 ++++++++++++++++++++ 3 files changed, 293 insertions(+), 15 deletions(-) create mode 100755 testbot/tests/TestPatches
diff --git a/testbot/lib/WineTestBot/Patches.pm b/testbot/lib/WineTestBot/Patches.pm index 2b52f4baff..a76c35fd48 100644 --- a/testbot/lib/WineTestBot/Patches.pm +++ b/testbot/lib/WineTestBot/Patches.pm @@ -113,23 +113,76 @@ sub FromSubmission($$) sub ParseSubject($) { my ($self) = @_; + my $SubjectInfo = { Domain => "", Version => 0 };
+ # Extract the reply prefixes: 'Re:', 'Fw:', etc. They are similar to a patch + # version except they are incompatible with patch series. my $Title = $self->Subject; - $Title =~ s/32/64//; - $Title =~ s/64/32//; + if ($Title =~ s~^\s*(\w\w?:\s*)+~~) + { + $SubjectInfo->{Re} = $1; + $SubjectInfo->{Re} =~ s~\s+~ ~g; + $SubjectInfo->{Re} =~ s~\s$~~; + } + + # Get rid of 'resend' wherever it is + $Title =~ s~[[(]resend\d*[])]~~i or + $Title =~ s~\bresend\d*\b~~i; + + # Try to extract the informal 'try N' version + if ($Title =~ s~[[(]try\s*(\d+)[])]~~i or + $Title =~ s~\btry\s*(\d+)\b~~i or + $Title =~ s~[[(]v(\d+)[])]~~i) + { + $SubjectInfo->{Version} = int($1); + } + + my ($Series, $IsFormal); + # If the subject is in the new [PATCH...] format, + # focus on that part to avoid false positives + if ($Title =~ s~^[([^]]*)]~~) + { + $IsFormal = 1; + $Series = $1; + $Series =~ s~PATCH~~i; + } + else + { + # Otherwise remove parts that may be confusing + $Series = $Title; + $Series =~ s~32/64~~g; + $Series =~ s~64/32~~g; + } + + # Extract the formal 'vN' version + if (!$SubjectInfo->{Version}) + { + if ($Series =~ s~[[(]v(\d+)[])]~~i or + $Series =~ s~\bv(\d+)\b~~i) + { + $SubjectInfo->{Version} = int($1); + } + }
- my $SubjectInfo; - if ($Title =~ s~[[(](\d+)/(\d+)[])]~~ or - $Title =~ s~\b(\d+)/(\d+)\b~~) + # Extract the part number for patch series + if ($Series =~ s~[[(](\d+)/(\d+)[])]~~ or + $Series =~ s~\b(\d+)/(\d+)\b~~) { $SubjectInfo->{PartNo} = int($1); $SubjectInfo->{MaxPartNo} = int($2); + if (!$IsFormal) + { + $Title =~ s~[[(]$1/$2[])]~~ or + $Title =~ s~$1/$2~~; + } }
- $Title =~ s/[PATCH[^]]*]//i; - $Title =~ s/\s+/ /g; - $Title =~ s/^\s//; - $Title =~ s/\s$//; + # Extract the 'domain' which usually identifies the patch repository + $SubjectInfo->{Domain} = $1 if ($IsFormal and $Series =~ /^\s*(\w+)\s*$/); + + $Title =~ s~\s+~ ~g; + $Title =~ s~^\s~~; + $Title =~ s~\s$~~; $SubjectInfo->{Title} = $Title;
return $SubjectInfo; @@ -181,7 +234,12 @@ sub Submit($$;$) my $Priority = 5; $NewJob->Priority($Priority); my $PropertyDescriptor = $Jobs->GetPropertyDescriptorByName("Remarks"); - my $Remarks = "[$PatchesMailingList] $SubjectInfo->{Title}"; + my $Remarks = "[$PatchesMailingList] "; + # Keep Re because it's similar to a version + $Remarks = "$SubjectInfo->{Re} $Remarks" if ($SubjectInfo->{Re}); + $Remarks .= "$SubjectInfo->{Domain} " if ($SubjectInfo->{Domain}); + $Remarks .= "v$SubjectInfo->{Version} " if ($SubjectInfo->{Version}); + $Remarks .= $SubjectInfo->{Title}; $NewJob->Remarks(substr($Remarks, 0, $PropertyDescriptor->GetMaxLength())); $NewJob->Patch($self);
diff --git a/testbot/lib/WineTestBot/PendingPatchSets.pm b/testbot/lib/WineTestBot/PendingPatchSets.pm index cfa6dc0e59..8f5b470542 100644 --- a/testbot/lib/WineTestBot/PendingPatchSets.pm +++ b/testbot/lib/WineTestBot/PendingPatchSets.pm @@ -28,11 +28,11 @@ WineTestBot::PendingPatchSet - An object tracking a pending patchset
A patchset is a set of patches that depend on each other. They are numbered so that one knows in which order to apply them. This is typically indicated by a -subject of the form '[3/5] Subject'. This means one must track which patchset -a patch belongs to so it is tested (and applied) together with the earlier -parts rather than in isolation. Furthermore the parts of the set may arrive in -the wrong order so processing of later parts must be deferred until the earlier -ones have been received. +subject of the form '[3/5] Subject' (see TestPatches for more details). +This means one must track which patchset a patch belongs to so it is tested +(and applied) together with the earlier parts rather than in isolation. +Furthermore the parts of the set may arrive in the wrong order so processing of +later parts must be deferred until the earlier ones have been received.
The WineTestBot::PendingPatchSet class is where this tracking is implemented.
diff --git a/testbot/tests/TestPatches b/testbot/tests/TestPatches new file mode 100755 index 0000000000..bd367071cd --- /dev/null +++ b/testbot/tests/TestPatches @@ -0,0 +1,220 @@ +#!/usr/bin/perl +# -*- Mode: Perl; perl-indent-level: 2; indent-tabs-mode: nil -*- +# +# Tests the Patches subject parser. +# +# Copyright 2020 Francois Gouget +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2.1 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this library; if not, write to the Free Software +# Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + +use strict; +use warnings; + +sub BEGIN +{ + if ($0 !~ m=^/=) + { + # Turn $0 into an absolute path so it can safely be used in @INC + require Cwd; + $0 = Cwd::cwd() . "/$0"; + } + if ($0 =~ m=^(/.*)/[^/]+/[^/]+$=) + { + $::RootDir = $1; + unshift @INC, "$::RootDir/lib"; + } +} + +my $name0 = $0; +$name0 =~ s+^.*/++; + +use Test::More; + +use WineTestBot::Log; +use WineTestBot::Patches; + + +sub error(@) +{ + print STDERR "$name0:error: ", @_; +} + + +# +# Process the command line +# + +my $Usage; +while (@ARGV) +{ + my $arg = shift @ARGV; + if ($arg eq "--help") + { + $Usage = 0; + } + else + { + error("unexpected argument '$arg'\n"); + $Usage = 2; + } +} + +if (defined $Usage) +{ + if ($Usage) + { + error("try '$name0 --help' for more information\n"); + exit $Usage; + } + print "Usage: $name0 [--help]\n"; + print "\n"; + print "Tests the Patches subject parser.\n"; + print "\n"; + print "Where:\n"; + print " --help Shows this usage message.\n"; + exit 0; +} + + +# +# The Patches tests +# + +my $TestCount; + +sub TestParseSubject() +{ + my @Tests = ( + {Subject => "Simple subject", + Title => "Simple subject"}, + {Subject => "Re: Simple reply", + Re => "Re:", + Title => "Simple reply"}, + {Subject => "Fw: Re:Re: Complex reply", + Re => "Fw: Re:Re:", + Title => "Complex reply"}, + {Subject => "First one fell into a black hole (resend)", + Title => "First one fell into a black hole"}, + {Subject => "Second try try 2", + Version => 2, + Title => "Second try"}, + {Subject => "Third time's the charm (try3)", + Version => 3, + Title => "Third time's the charm"}, + {Subject => "Still trying (v4)", + Version => 4, + Title => "Still trying"}, + + {Subject => "[PATCH] Simple patch", + Title => "Simple patch"}, + {Subject => "Re:[PATCH] Patch reply", + Title => "Patch reply"}, + {Subject => "[tools] Also a patch", + Domain => "tools", + Title => "Also a patch"}, + {Subject => "[tools resend] Resent patch", + Domain => "tools", + Title => "Resent patch"}, + + {Subject => "[PATCH v2] New patch version", + Version => 2, + Title => "New patch version"}, + {Subject => "Re: [PATCH (v2)] New patch reply", + Version => 2, + Title => "New patch reply"}, + {Subject => "Fw: [v2] New patch version", + Version => 2, + Title => "New patch version"}, + {Subject => "[wtbs v12] New patch version", + Domain => "wtbs", Version => 12, + Title => "New patch version"}, + {Subject => "[wtbs resend v12] Resent new patch version", + Domain => "wtbs", Version => 12, + Title => "Resent new patch version"}, + {Subject => "[wtbs resend (try 3)] Resent new patch version", + Domain => "wtbs", Version => 3, + Title => "Resent new patch version"}, + + {Subject => "1/2 Informal series", + PartNo => 1, MaxPartNo => 2, + Title => "Informal series"}, + {Subject => "(1/2) Parentheses series", + PartNo => 1, MaxPartNo => 2, + Title => "Parentheses series"}, + {Subject => "Re: Tail informal series 2/2", + PartNo => 2, MaxPartNo => 2, + Title => "Tail informal series"}, + {Subject => "Re: Tail bracket series [2/2]", + PartNo => 2, MaxPartNo => 2, + Title => "Tail bracket series"}, + + {Subject => "[PATCH 1/2] Formal series", + PartNo => 1, MaxPartNo => 2, + Title => "Formal series"}, + {Subject => "[1/2 PATCH] Reverse series", + PartNo => 1, MaxPartNo => 2, + Title => "Reverse series"}, + {Subject => " Fw: [ wtbs 1/2 ] Also a patch series", + Domain => "wtbs", + PartNo => 1, MaxPartNo => 2, + Title => "Also a patch series"}, + + {Subject => "[wtbs v1 1/2] First series version", + Domain => "wtbs", Version => 1, + PartNo => 1, MaxPartNo => 2, + Title => "First series version"}, + {Subject => "Re: [wtbs 10/12 v2] Second series version", + Domain => "wtbs", Version => 2, + PartNo => 10, MaxPartNo => 12, + Title => "Second series version"}, + {Subject => "Fw: [PATCH v2 wtbs 1/2] Second series version", + Domain => "wtbs", Version => 2, + PartNo => 1, MaxPartNo => 2, + Title => "Second series version"}, + {Subject => "Fw: [V2 wtbs (resend) 1/2] Second series version", + Domain => "wtbs", Version => 2, + PartNo => 1, MaxPartNo => 2, + Title => "Second series version"}, + {Subject => "Fw: [v2 not a domain 1/2] Second series version", + Version => 2, + PartNo => 1, MaxPartNo => 2, + Title => "Second series version"}, + ); + + my $Patch = CreatePatches()->Add(); + foreach my $Test (@Tests) + { + # Provide defaults for the expected results + $Test->{Domain} ||= ""; + $Test->{Version} ||= 0; + + $Patch->Subject($Test->{Subject}); + my $SubjectInfo = $Patch->ParseSubject(); + foreach my $Field ("Domain", "Version", "PartNo", "MaxPartNo", "Title") + { + is($SubjectInfo->{$Field}, $Test->{$Field}, + "Check $Field for $Test->{Subject}"); + $TestCount++; + } + } +} + + +# +# Run the test +# + +TestParseSubject(); +done_testing($TestCount);