[tools] testbot: Improve the patch subject parsing.
Francois Gouget
fgouget at codeweavers.com
Wed Mar 4 14:21:09 CST 2020
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 at 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);
--
2.20.1
More information about the wine-devel
mailing list