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 <reportname> could contain underscores too such as in 'win32_fr_FR.report'. Dashes are a better choice since they are not allowed in either the VM name nor in the report name. So the new refeence filename format is '<vmname>-job<jobid>-<reportname>'.
Note: This requires running UpdateTaskLogs to move and rename the existing reference reports. --- testbot/bin/Janitor.pl | 2 +- testbot/bin/UpdateTaskLogs | 71 ++++++++++++++++++++++++--- 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, 73 insertions(+), 15 deletions(-)
diff --git a/testbot/bin/Janitor.pl b/testbot/bin/Janitor.pl index 579a43275..ecae5be5a 100755 --- a/testbot/bin/Janitor.pl +++ b/testbot/bin/Janitor.pl @@ -314,7 +314,7 @@ 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)?$/) + 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 2ce192f0b..70a3dd2c1 100755 --- a/testbot/bin/UpdateTaskLogs +++ b/testbot/bin/UpdateTaskLogs @@ -242,6 +242,46 @@ sub DoUpdateLatestReport($$$) return $Rc; }
+sub MoveRefReport($$;$) +{ + my ($RefDir, $RefName, $NewDir) = @_; + + my $RefPath = "$RefDir/$RefName"; + + my $Rc = 0; + 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"; + if (-f $NewPath) + { + Error "Could not move '$RefName' because the '$NewPath' target already exists\n"; + $Rc = 1; + } + elsif ($RefPath ne $NewPath) + { + my $TaskKey = Path2TaskKey($NewDir); + my $RelRefDir = ($RefDir eq $NewDir) ? "" : "../"; + Debug("$TaskKey: $RelRefDir$RefName -> $NewName\n"); + if (!rename($RefPath, $NewPath)) + { + Error "Could not move '$RefPath' to '$NewName': $!\n"; + $Rc = 1; + } + } + } + + return $Rc; +} + sub ProcessTaskLogs($$$) { my ($Step, $Task, $CollectOnly) = @_; @@ -250,6 +290,20 @@ sub ProcessTaskLogs($$$) my $StepDir = $Step->GetDir(); my $TaskDir = $Task->GetDir();
+ if (!$CollectOnly) + { + # Upgrade the naming scheme of the task's old reference reports + foreach my $LogName (@{GetLogFileNames($TaskDir)}) + { + next if ($LogName !~ /.report$/); + my $StepReportName = $Task->VM->Name ."_$LogName"; + foreach my $Suffix ("", ".err") + { + $Rc += MoveRefReport($StepDir, "$StepReportName$Suffix", $TaskDir); + } + } + } + if (($OptDelete or $OptRebuild) and !$CollectOnly) { # Save / delete the task's reference reports... all of them, @@ -263,7 +317,7 @@ sub ProcessTaskLogs($$$) foreach my $ReportName (@$ReportNames) { my $RefReportName = $Task->GetRefReportName($ReportName); - my $RefReportPath = "$StepDir/$RefReportName"; + my $RefReportPath = "$TaskDir/$RefReportName";
if (-f $RefReportPath or -f "$RefReportPath.err") { @@ -283,7 +337,7 @@ sub ProcessTaskLogs($$$) # (this would be the case for the oldest tasks with --rebuild) $Rc += DoUpdateLatestReport($Task, $ReportName, $RefReportPath);
- Debug(TaskKeyStr($Task) .": Deleting ../$RefReportName\n"); + Debug(TaskKeyStr($Task) .": Deleting $RefReportName*\n"); } foreach my $Suffix ("", ".err") { @@ -311,7 +365,7 @@ sub ProcessTaskLogs($$$) { next if ($LogName !~ /.report$/); my $RefReportName = $Task->GetRefReportName($LogName); - next if (-f "$StepDir/$RefReportName"); + next if (-f "$TaskDir/$RefReportName");
my $LatestReportPath = $LatestReports{$RefReportName}; if (!defined $LatestReportPath) @@ -324,9 +378,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; @@ -396,7 +450,7 @@ sub ProcessLatestReports()
if ($OptDelete or $OptRebuild) { - # Delete the reports so they are rebuilt from scratch if appropriate + # Delete the reports that should be deleted / rebuilt foreach my $Suffix ("", ".err") { next if (!-f "$LatestReportPath$Suffix"); @@ -425,6 +479,11 @@ sub ProcessLatestReports() $Rc = 1; } } + else + { + # Upgrade the naming scheme of the task's old reference reports + $Rc += MoveRefReport("$DataDir/latest", $RefReportName); + } }
return $Rc; diff --git a/testbot/bin/WineSendLog.pl b/testbot/bin/WineSendLog.pl index b31d1f5d3..393c76a0e 100755 --- a/testbot/bin/WineSendLog.pl +++ b/testbot/bin/WineSendLog.pl @@ -292,7 +292,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 764330281..08c9911d9 100644 --- a/testbot/lib/WineTestBot/LogUtils.pm +++ b/testbot/lib/WineTestBot/LogUtils.pm @@ -1064,11 +1064,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 bad0e2437..f65e6a632 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 1a46f0d57..7b9ac9b81 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 fc18c866f..a4179c705 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); }