[PATCH 1/2] testbot: Move reference logs to the task and prepare for having more than one.
Francois Gouget
fgouget at codeweavers.com
Mon Feb 3 20:50:07 CST 2020
The old reference report filename format was '<vmname>_<reportname>'
which means there could only be one per report. Each job should only
have one report of a given type per VM so add it to the reference
filename.
Also the use of an underscore as the separator made it hard to split
the components apart as both <vmname> and <reportname> can contain
underscores such as 'w1064v1809_fr' and 'win32_fr_FR.report'. Dashes
are a better choice since they are not allowed in either the VM name
or the report name.
So the new refeence filename format is '<vmname>-job<jobid>-<reportname>'.
Note:
* After applying this patch run UpdateTaskLogs to move and rename the
existing reference reports.
* When downgrading to run code before this patch it's best to run
UpdateTaskLogs --delete before downgrading and then UpdateTaskLogs
after the downgrade.
Signed-off-by: Francois Gouget <fgouget at codeweavers.com>
---
To simplify one can just apply both patches in the series and then just
run UpdateTaskLogs once to upgrade the existing reports and logs.
It may be a good idea to run it as follows to help with debugging in
case something goes wrong:
cd testbot/var && ../bin/UpdateTaskLogs --debug 2>&1 | tee UpdateTaskLogs.log
testbot/bin/Janitor.pl | 3 +-
testbot/bin/UpdateTaskLogs | 113 +++++++++++++++++++++++---
testbot/bin/WineSendLog.pl | 2 +-
testbot/lib/WineTestBot/LogUtils.pm | 5 +-
testbot/lib/WineTestBot/StepsTasks.pm | 2 +-
testbot/lib/WineTestBot/Tasks.pm | 2 +-
testbot/web/JobDetails.pl | 4 +-
7 files changed, 110 insertions(+), 21 deletions(-)
diff --git a/testbot/bin/Janitor.pl b/testbot/bin/Janitor.pl
index bf3989a712..dd13966018 100755
--- a/testbot/bin/Janitor.pl
+++ b/testbot/bin/Janitor.pl
@@ -314,7 +314,8 @@ if (opendir(my $dh, "$DataDir/latest"))
my $Age = int((-M $FileName) + 0.5);
my $TTL = $JobPurgeDays ? $JobPurgeDays - $Age : undef;
- if ($Entry =~ /^([a-zA-Z0-9_]+)_(?:exe|win|wow)(?:32|64)[a-zA-Z0-9_]*\.report(?:\.err)?$/)
+ # Keep the regexp in sync with WineTestBot::Task::GetRefReportName()
+ if ($Entry =~ /^([a-zA-Z0-9_]+)-job[0-9]+-[a-zA-Z0-9_]+\.report(?:\.err)?$/)
{
# Keep the reference WineTest reports for all VMs even if they are
# retired or scheduled for deletion.
diff --git a/testbot/bin/UpdateTaskLogs b/testbot/bin/UpdateTaskLogs
index 887061dab2..11dd6d31b9 100755
--- a/testbot/bin/UpdateTaskLogs
+++ b/testbot/bin/UpdateTaskLogs
@@ -261,18 +261,66 @@ sub DoUpdateLatestReport($$$)
return $Rc;
}
+sub MoveRefReport($$;$)
+{
+ my ($RefDir, $RefName, $NewDir) = @_;
+
+ my $RefPath = "$RefDir/$RefName";
+ return Delete("$RefPath.err", "orphaned") if (!-f $RefPath);
+
+ $NewDir ||= $RefDir;
+ my $NewName = $RefName;
+ if ($RefName =~ /^([a-zA-Z0-9_]+)_((?:exe|win|wow)(?:32|64)[a-zA-Z0-9_]*\.report(?:\.err)?)$/)
+ {
+ # We don't know which job generated this reference log.
+ # So use a fixed, arbitrary JobId.
+ $NewName = "$1-job000000-$2";
+ }
+
+ my $NewPath = "$NewDir/$NewName";
+ return 0 if ($RefPath eq $NewPath);
+
+ my $Rc = 0;
+ my $TaskKey = Path2TaskKey($NewDir);
+ if (-f $NewPath and -M $NewPath <= -M $RefPath)
+ {
+ # A WineTest job has probably completed after the upgrade already
+ $Rc += Delete($RefPath);
+ $Rc += Delete("$RefPath.err");
+ return $Rc;
+ }
+
+ if (-f $NewPath)
+ {
+ Error "Could not move '$RefName' because '$NewName' already exists and is older!\n";
+ return 1;
+ }
+
+ my $RelRefDir = ($RefDir eq $NewDir) ? "" : "../";
+ Debug("$TaskKey: $RelRefDir$RefName* -> $NewName\n");
+ foreach my $Suffix ("", ".err")
+ {
+ next if (!-f "$RefPath$Suffix");
+ next if (rename("$RefPath$Suffix", "$NewPath$Suffix"));
+ Error "Could not move '$RefName$Suffix' to '$NewName$Suffix' for $TaskKey: $!\n";
+ $Rc = 1;
+ }
+
+ return $Rc;
+}
+
sub ProcessTaskLogs($$$)
{
my ($Step, $Task, $CollectOnly) = @_;
my $Rc = 0;
- my $StepDir = $Step->GetDir();
+ my $ReportNames;
my $TaskDir = $Task->GetDir();
- if (($OptDelete or $OptRebuild) and !$CollectOnly)
+ if (!$CollectOnly)
{
- # Save / delete the task's reference reports... all of them,
- # even if they were not supposed to exist (e.g. failed builds).
+ # Upgrade the naming scheme of the task's old reference reports,
+ # delete those that are not meant to exist.
my ($ErrMessage, $ReportNames, $_TaskMissions) = $Task->GetReportNames();
if ($ErrMessage)
{
@@ -280,9 +328,30 @@ sub ProcessTaskLogs($$$)
$Rc = 1;
}
foreach my $ReportName (@$ReportNames)
+ {
+ my $StepReportName = $Task->VM->Name ."_$ReportName";
+ my $StepReportPath = $Step->GetFullFileName($StepReportName);
+ if (-f "$TaskDir/$ReportName")
+ {
+ $Rc += MoveRefReport(dirname($StepReportPath), $StepReportName, $TaskDir);
+ # The old WineTest reference reports for Windows tasks may end up being
+ # duplicated: one copy in the build step and another in the step for
+ # the task. MoveRefReport() moved the latter, now ensure Delete() can
+ # remove the former.
+ $StepReportPath = $Step->GetFullFileName($StepReportName);
+ }
+ $Rc += Delete($StepReportPath, "unneeded");
+ }
+ }
+
+ if (($OptDelete or $OptRebuild) and !$CollectOnly)
+ {
+ # Save / delete the task's reference reports... all of them,
+ # even if they were not supposed to exist (e.g. failed builds).
+ foreach my $ReportName (@$ReportNames)
{
my $RefReportName = $Task->GetRefReportName($ReportName);
- my $RefReportPath = "$StepDir/$RefReportName";
+ my $RefReportPath = "$TaskDir/$RefReportName";
if (-f $RefReportPath or -f "$RefReportPath.err")
{
@@ -290,7 +359,7 @@ sub ProcessTaskLogs($$$)
{
# (Re)Build the .err file before adding the reference report to
# latest/.
- my $ErrMessage = BuildErrFile($StepDir, $RefReportName, 1, 0);
+ my $ErrMessage = BuildErrFile($TaskDir, $RefReportName, 1, 0);
if (defined $ErrMessage)
{
Error "$ErrMessage\n";
@@ -315,7 +384,7 @@ sub ProcessTaskLogs($$$)
{
next if ($ReportName !~ /\.report$/);
my $RefReportName = $Task->GetRefReportName($ReportName);
- next if (-f "$StepDir/$RefReportName");
+ next if (-f "$TaskDir/$RefReportName");
my $LatestReportPath = $LatestReports{$RefReportName};
if (!defined $LatestReportPath)
@@ -328,9 +397,9 @@ sub ProcessTaskLogs($$$)
Debug(TaskKeyStr($Task) .": Snapshotting $RefReportName from ". Path2TaskKey($LatestReportPath) ."\n");
foreach my $Suffix ("", ".err")
{
- unlink "$StepDir/$RefReportName$Suffix";
+ unlink "$TaskDir/$RefReportName$Suffix";
if (-f "$LatestReportPath$Suffix" and
- !link("$LatestReportPath$Suffix", "$StepDir/$RefReportName$Suffix"))
+ !link("$LatestReportPath$Suffix", "$TaskDir/$RefReportName$Suffix"))
{
Error "Could not link '$RefReportName$Suffix': $!\n";
$Rc = 1;
@@ -390,18 +459,38 @@ sub ProcessLatestReports()
my $Rc = 0;
my $LatestGlob = "$DataDir/latest/*.report";
- # Perform cleanups and updates
+ # Delete or rename the old reference reports
foreach my $LatestReportPath (glob("$LatestGlob $LatestGlob.err"))
{
my $RefReportName = basename($LatestReportPath);
- # Keep the regexp in sync with WineTestBot::Task::GetRefReportName()
next if ($RefReportName !~ /^([a-zA-Z0-9_]+_[a-zA-Z0-9_]+\.report)(?:\.err)?$/);
$RefReportName = $1; # untaint
$LatestReportPath = "$DataDir/latest/$RefReportName";
if ($OptDelete or $OptRebuild)
{
- # Delete the reports that should be deleted / rebuilt
+ $Rc += Delete($LatestReportPath);
+ $Rc += Delete("$LatestReportPath.err");
+ }
+ else
+ {
+ # MoveRefReport() also renames .err files
+ $Rc += MoveRefReport("$DataDir/latest", $RefReportName);
+ }
+ }
+
+ # Then perform cleanups and rebuild missing files
+ foreach my $LatestReportPath (glob("$LatestGlob $LatestGlob.err"))
+ {
+ my $RefReportName = basename($LatestReportPath);
+ # Keep the regexp in sync with WineTestBot::Task::GetRefReportName()
+ next if ($RefReportName !~ /^([a-zA-Z0-9_]+-job[0-9]+-[a-zA-Z0-9_]+\.report)(?:\.err)?$/);
+ $RefReportName = $1; # untaint
+ $LatestReportPath = "$DataDir/latest/$RefReportName";
+
+ if ($OptDelete or $OptRebuild)
+ {
+ # These can be rebuilt from the task reports
$Rc += Delete($LatestReportPath);
$Rc += Delete("$LatestReportPath.err");
}
diff --git a/testbot/bin/WineSendLog.pl b/testbot/bin/WineSendLog.pl
index 44d7255e71..e0cd585b11 100755
--- a/testbot/bin/WineSendLog.pl
+++ b/testbot/bin/WineSendLog.pl
@@ -293,7 +293,7 @@ EOF
next if (!$LogInfo->{ErrCount});
my $AllNew;
- my $RefReportPath = $StepTask->GetFullFileName($StepTask->GetRefReportName($LogName));
+ my $RefReportPath = "$TaskDir/". $StepTask->GetRefReportName($LogName);
TagNewErrors($RefReportPath, $LogInfo);
if ($LogInfo->{NoRef})
{
diff --git a/testbot/lib/WineTestBot/LogUtils.pm b/testbot/lib/WineTestBot/LogUtils.pm
index 5b567bc23b..36f367497b 100644
--- a/testbot/lib/WineTestBot/LogUtils.pm
+++ b/testbot/lib/WineTestBot/LogUtils.pm
@@ -1070,11 +1070,10 @@ sub SnapshotLatestReport($$)
{
next if (!-f "$DataDir/latest/$RefReportName$Suffix");
- # FIXME: The reference reports are stored at the step level!
if (!link("$DataDir/latest/$RefReportName$Suffix",
- "$TaskDir/../$RefReportName$Suffix"))
+ "$TaskDir/$RefReportName$Suffix"))
{
- push @ErrMessages, "Could not create the '../$RefReportName$Suffix' link: $!";
+ push @ErrMessages, "Could not create the '$RefReportName$Suffix' link: $!";
}
}
diff --git a/testbot/lib/WineTestBot/StepsTasks.pm b/testbot/lib/WineTestBot/StepsTasks.pm
index bad0e24375..f65e6a6327 100644
--- a/testbot/lib/WineTestBot/StepsTasks.pm
+++ b/testbot/lib/WineTestBot/StepsTasks.pm
@@ -71,7 +71,7 @@ sub GetTaskDir($)
sub GetRefReportName($$)
{
my ($self, $ReportName) = @_;
- return $self->VM->Name ."_$ReportName";
+ return $self->VM->Name ."-job000000-$ReportName";
}
sub GetTitle($)
diff --git a/testbot/lib/WineTestBot/Tasks.pm b/testbot/lib/WineTestBot/Tasks.pm
index 1a46f0d57c..7b9ac9b816 100644
--- a/testbot/lib/WineTestBot/Tasks.pm
+++ b/testbot/lib/WineTestBot/Tasks.pm
@@ -141,7 +141,7 @@ sub GetReportNames($)
sub GetRefReportName($$)
{
my ($self, $ReportName) = @_;
- return $self->VM->Name ."_$ReportName";
+ return $self->VM->Name ."-job000000-$ReportName";
}
sub _SetupTask($$)
diff --git a/testbot/web/JobDetails.pl b/testbot/web/JobDetails.pl
index f1ffd498ed..3dce91595d 100644
--- a/testbot/web/JobDetails.pl
+++ b/testbot/web/JobDetails.pl
@@ -494,7 +494,7 @@ EOF
if ($LogInfo->{ErrCount})
{
# Identify new errors in test reports
- my $RefReportPath = $StepTask->GetFullFileName($StepTask->GetRefReportName($MoreInfo->{Full}));
+ my $RefReportPath = "$TaskDir/". $StepTask->GetRefReportName($MoreInfo->{Full});
TagNewErrors($RefReportPath, $LogInfo);
}
}
@@ -563,7 +563,7 @@ EOF
if ($LogName =~ /\.report$/)
{
# For test reports try to identify the new errors
- my $RefReportPath = $StepTask->GetFullFileName($StepTask->GetRefReportName($LogName));
+ my $RefReportPath = "$TaskDir/". $StepTask->GetRefReportName($LogName);
TagNewErrors($RefReportPath, $LogInfo);
}
--
2.20.1
More information about the wine-devel
mailing list