[tools 1/2] testbot/web: Automatically refresh the failure bug information.

[Francois Gouget]

Automatically refresh the failure bug information when the bug id is
modified.
Also trigger a refresh when a failure is undeleted.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=48912
Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com>
---
This ensure failures have an initial bug status and description as soon 
as they are added. Later refreshes happen when UpdateFailures is called 
by cron.
---
 testbot/bin/Engine.pl                    | 39 ++++++++++++++++++++++++
 testbot/lib/WineTestBot/Engine/Notify.pm | 20 +++++++++++-
 testbot/lib/WineTestBot/Failures.pm      | 31 +++++++++++++++++++
 testbot/web/FailureDetails.pl            |  1 -
 testbot/web/FailuresList.pl              |  5 ++-
 5 files changed, 91 insertions(+), 5 deletions(-)

diff --git a/testbot/bin/Engine.pl b/testbot/bin/Engine.pl
index 88852730e..29dce183b 100755
--- a/testbot/bin/Engine.pl
+++ b/testbot/bin/Engine.pl
@@ -49,6 +49,7 @@ use WineTestBot::Config;
 use WineTestBot::Engine::Events;
 use WineTestBot::Engine::Notify;
 use WineTestBot::Engine::Scheduler;
+use WineTestBot::Failures;
 use WineTestBot::Jobs;
 use WineTestBot::Log;
 use WineTestBot::Patches;
@@ -548,6 +549,43 @@ sub HandleGetScreenshot($)
   return "1" . $ImageBytes;
 }
 
+sub HandleUpdateFailure($)
+{
+  my ($FailureId) = @_;
+
+  # Validate FailureId
+  if ($FailureId !~ /^(\d+)$/)
+  {
+    LogMsg "Invalid failure id\n";
+    return "0Invalid Failure Id";
+  }
+  $FailureId = $1;
+
+  if (!CreateFailures()->GetItem($FailureId))
+  {
+    LogMsg "Unknown Failure $FailureId\n";
+    return "0Unknown Failure $FailureId";
+  }
+
+  my $Pid = fork;
+  if (!defined $Pid)
+  {
+    LogMsg "Unable to fork for UpdateFailures.pl: $!\n";
+  }
+  elsif (!$Pid)
+  {
+    # Clean up the child environment
+    CloseAllDBBackEnds();
+    WineTestBot::Log::SetupRedirects();
+
+    exec("$BinDir/UpdateFailures.pl --log-only $FailureId") or
+        LogMsg "Unable to exec ${ProjectName}UpdateFailures.pl: $!\n";
+    exit(1);
+  }
+
+  return "1";
+}
+
 my %Handlers=(
     "getscreenshot"            => \&HandleGetScreenshot,
     "jobcontrol"               => \&HandleJobControl,
@@ -558,6 +596,7 @@ my %Handlers=(
     "vmstatuschange"           => \&HandleVMStatusChange,
     "winepatchmlsubmission"    => \&HandleWinePatchMLSubmission,
     "winepatchwebsubmission"   => \&HandleWinePatchWebSubmission,
+    "updatefailure"            => \&HandleUpdateFailure,
     );
 
 sub HandleClientCmd(@)
diff --git a/testbot/lib/WineTestBot/Engine/Notify.pm b/testbot/lib/WineTestBot/Engine/Notify.pm
index 82221c9ed..9c1802984 100644
--- a/testbot/lib/WineTestBot/Engine/Notify.pm
+++ b/testbot/lib/WineTestBot/Engine/Notify.pm
@@ -31,7 +31,8 @@ use Exporter 'import';
 our $RunningInEngine;
 our @EXPORT = qw(Shutdown PingEngine JobStatusChange JobControl
                  RescheduleJobs VMStatusChange
-                 WinePatchMLSubmission WinePatchWebSubmission GetScreenshot);
+                 WinePatchMLSubmission WinePatchWebSubmission
+                 UpdateFailure GetScreenshot);
 our @EXPORT_OK = qw($RunningInEngine);
 
 use Socket;
@@ -194,6 +195,23 @@ sub WinePatchWebSubmission()
   return substr($Reply, 1);
 }
 
+sub UpdateFailure($)
+{
+  my ($FailureId) = @_;
+
+  my $Reply = SendCmdReceiveReply("updatefailure $FailureId\n");
+  if (length($Reply) < 1)
+  {
+    return "The Engine did not acknowledge the UpdateFailure command";
+  }
+  if (substr($Reply, 0, 1) eq "1")
+  {
+    return (undef, substr($Reply, 1));
+  }
+
+  return (substr($Reply, 1), undef);
+}
+
 sub GetScreenshot($)
 {
   my ($VMName) = @_;
diff --git a/testbot/lib/WineTestBot/Failures.pm b/testbot/lib/WineTestBot/Failures.pm
index b37b9e046..a9118f64b 100644
--- a/testbot/lib/WineTestBot/Failures.pm
+++ b/testbot/lib/WineTestBot/Failures.pm
@@ -40,6 +40,8 @@ TaskFailure objects.
 use WineTestBot::WineTestBotObjects;
 our @ISA = qw(WineTestBot::WineTestBotItem);
 
+use WineTestBot::Engine::Notify;
+
 
 sub Compare($$)
 {
@@ -55,6 +57,35 @@ sub Compare($$)
   return $Cmp
 }
 
+sub BugId($;$)
+{
+  my ($self, $NewBugId) = @_;
+
+  my $CurrentBugId = $self->SUPER::BugId;
+  return $CurrentBugId if (!defined $NewBugId);
+
+  if (!defined $CurrentBugId or $NewBugId ne $CurrentBugId)
+  {
+    $self->{OldBugId} = $CurrentBugId if (!defined $self->{OldBugId});
+    $self->SUPER::BugId($NewBugId);
+  }
+
+  return $NewBugId;
+}
+
+sub OnSaved($)
+{
+  my ($self) = @_;
+
+  $self->SUPER::OnSaved();
+  if ((defined $self->{OldBugId} and $self->{OldBugId} ne $self->BugId) or
+      !$self->BugStatus or !$self->BugDescription)
+  {
+    UpdateFailure($self->Id);
+  }
+  delete $self->{OldBugId};
+}
+
 sub Validate($)
 {
   my ($self) = @_;
diff --git a/testbot/web/FailureDetails.pl b/testbot/web/FailureDetails.pl
index 7cab1d2a6..c05146448 100644
--- a/testbot/web/FailureDetails.pl
+++ b/testbot/web/FailureDetails.pl
@@ -105,7 +105,6 @@ sub OnAction($$)
       $self->{Item}->BugDescription("");
     }
     return undef if (!$self->Save());
-    # FIXME Notify the TestBot Engine so it updates the bug information
     exit($self->RedirectToParent());
   }
   return $self->SUPER::OnAction($Action);
diff --git a/testbot/web/FailuresList.pl b/testbot/web/FailuresList.pl
index f2608dc45..8825fea69 100644
--- a/testbot/web/FailuresList.pl
+++ b/testbot/web/FailuresList.pl
@@ -129,9 +129,9 @@ sub OnItemAction($$$)
     return 1 if ($Action eq "Restore" and $Failure->BugStatus ne "deleted");
 
     my $NewStatus = $Action eq "Delete" ? "deleted" :
-                    $Action eq "Restore" ? "unknown" :
+                    $Action eq "Restore" ? "" :
                     undef;
-    if ($NewStatus)
+    if (defined $NewStatus)
     {
       $Failure->BugStatus($NewStatus);
       my ($_ErrProperty, $ErrMessage) = $Failure->Save();
@@ -141,7 +141,6 @@ sub OnItemAction($$$)
         $self->{EnclosingPage}->SetError(undef, $ErrMessage);
         return 0;
       }
-      # FIXME Notify the TestBot Engine so it updates the bug information
       return 1;
     }
   }
-- 
2.30.2

[tools 2/2] testbot/Engine: Also handle notifications originating from the Engine.

[Francois Gouget]

In particular notifications of job status changes must be processed in
order for the result emails to be sent and for the patch status to be
updated.

Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com>
---
I believe this explains why a number of developer emails were getting 
lost during some tests I ran. I had suspected ISP-level spam filtering 
issues, or too big emails, but I recently noticed cases where 
WineSendLog was not getting called which turned out to be because of 
this issue.
---
 testbot/bin/Engine.pl                    |  2 +-
 testbot/lib/WineTestBot/Engine/Notify.pm | 16 ++++++++++++----
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/testbot/bin/Engine.pl b/testbot/bin/Engine.pl
index 29dce183b..9d836529c 100755
--- a/testbot/bin/Engine.pl
+++ b/testbot/bin/Engine.pl
@@ -751,7 +751,7 @@ sub main()
   $SIG{CHLD} = \&REAPER;
   WineTestBot::Log::SetupRedirects();
 
-  $WineTestBot::Engine::Notify::RunningInEngine = 1;
+  SetNotificationHandler(\&HandleClientCmd);
   LogMsg "Starting the WineTestBot Engine\n";
 
   # Validate and adjust the configuration options
diff --git a/testbot/lib/WineTestBot/Engine/Notify.pm b/testbot/lib/WineTestBot/Engine/Notify.pm
index 9c1802984..c9930e6e6 100644
--- a/testbot/lib/WineTestBot/Engine/Notify.pm
+++ b/testbot/lib/WineTestBot/Engine/Notify.pm
@@ -2,6 +2,7 @@
 # Notification of WineTestBot engine
 #
 # Copyright 2009 Ge van Geldorp
+# Copyright 2012-2022 Francois Gouget
 #
 # This library is free software; you can redistribute it and/or
 # modify it under the terms of the GNU Lesser General Public
@@ -29,8 +30,8 @@ WineTestBot::Engine::Notify - Engine notification
 
 use Exporter 'import';
 our $RunningInEngine;
-our @EXPORT = qw(Shutdown PingEngine JobStatusChange JobControl
-                 RescheduleJobs VMStatusChange
+our @EXPORT = qw(SetNotificationHandlerShutdown PingEngine JobStatusChange
+                 JobControl RescheduleJobs VMStatusChange
                  WinePatchMLSubmission WinePatchWebSubmission
                  UpdateFailure GetScreenshot);
 our @EXPORT_OK = qw($RunningInEngine);
@@ -38,14 +39,21 @@ our @EXPORT_OK = qw($RunningInEngine);
 use Socket;
 use WineTestBot::Config;
 
+my $_CmdHandler;
+
+sub SetNotificationHandler($)
+{
+  my ($Handler) = @_;
+  $_CmdHandler = $Handler;
+}
 
 sub SendCmdReceiveReply($)
 {
   my ($Cmd) = @_;
 
-  if (defined($RunningInEngine))
+  if ($_CmdHandler)
   {
-    return "1";
+    return &$_CmdHandler(split ' ', $Cmd);
   }
 
   if (! socket(SOCK, PF_UNIX, SOCK_STREAM, 0))
-- 
2.30.2

[tools 2/2] testbot/Engine: Also handle notifications originating from the Engine.

[Francois Gouget]

On Thu, 16 Jun 2022, Francois Gouget wrote:
[...]
> -our @EXPORT = qw(Shutdown PingEngine JobStatusChange JobControl
> -                 RescheduleJobs VMStatusChange
> +our @EXPORT = qw(SetNotificationHandlerShutdown PingEngine JobStatusChange

Oups. I introduced a typo here during a rebase. I will resubmit.


-- 
Francois Gouget <fgouget(a)codeweavers.com>