[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