[tools 1/4] testbot/LogUtils: Add support for arrays in .errors files.
Array lines allow providing multiple values for a given 'property' name. Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- testbot/lib/WineTestBot/LogUtils.pm | 30 ++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/testbot/lib/WineTestBot/LogUtils.pm b/testbot/lib/WineTestBot/LogUtils.pm index 3488c7b28..79714358c 100644 --- a/testbot/lib/WineTestBot/LogUtils.pm +++ b/testbot/lib/WineTestBot/LogUtils.pm @@ -848,6 +848,10 @@ Property lines contain (name, value) pairs. Note that properties which can be calculated while reading the errors file are not saved (e.g. ErrCount and NewCount). +=item a <name> <value> +Array lines contain (name, value) pairs and can appear multiple times for a +given name, resulting in a corresponding list of values. + =item g <lineno> <groupname> Group lines contain the group name and the line number of the first line of the group in the log. Note that the first line would typically not be an @@ -897,6 +901,22 @@ sub LoadLogErrorsFromFh($$) return $Line; } } + elsif ($Type eq "a") + { + if (!defined $LogInfo->{$Property}) + { + $LogInfo->{$Property} = [ $Value ]; + } + elsif (ref($LogInfo->{$Property}) eq "ARRAY") + { + push @{$LogInfo->{$Property}}, $Value; + } + else + { + $LogInfo->{BadLog} = "$LogInfo->{LineNo}: $Property is not an array, its value is $LogInfo->{$Property}"; + return $Line; + } + } elsif ($Type eq "g") { $LogInfo->{CurGroup} = _AddLogGroup($LogInfo, $Value, $Property); @@ -1025,7 +1045,15 @@ sub _DumpErrors next; } - print STDERR "+ $Key $LogInfo->{$Key}\n"; + if (ref($LogInfo->{$Key}) eq "ARRAY") + { + print STDERR "+ $Key\n"; + map { print STDERR " | $_\n" } (@{$LogInfo->{$Key}}); + } + else + { + print STDERR "+ $Key $LogInfo->{$Key}\n"; + } } _WriteLogErrorsToFh(*STDERR, $LogInfo); map { _DumpErrors("$Label.$_", $LogInfo->{$_}) } (@SubKeys); -- 2.20.1
Either for their presence or their absence. This is particularly useful to check that specific build actions were performed (or skipped) as expected. For instance: ----- TestWTBS ----- a build.log.Grep ^Applying patch a build.log.GrepV ^Running make_requests a build.log.GrepV ^Running make_opengl ----- TestWTBS ----- Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- testbot/tests/TestWTBS | 84 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 83 insertions(+), 1 deletion(-) diff --git a/testbot/tests/TestWTBS b/testbot/tests/TestWTBS index 01f46bfde..58bf8a756 100755 --- a/testbot/tests/TestWTBS +++ b/testbot/tests/TestWTBS @@ -3,7 +3,7 @@ # # Automate checking the Wine TestBot test Suite results. # -# Copyright 2020 Francois Gouget +# Copyright 2020-2021 Francois Gouget # # This library is free software; you can redistribute it and/or # modify it under the terms of the GNU Lesser General Public @@ -353,6 +353,22 @@ sub LoadTestInfo($) } } + # Validate and fix the Grep* fields + foreach my $GrepType ("Grep", "GrepV") + { + foreach my $TaskType ("tasks", "tests", "build", "win", "win32", "win64", "wine") + { + foreach my $LogType ("report", "log", "testbot") + { + my $Array = $TestInfo->{$TaskType}->{"$LogType.$GrepType"}; + next if (!defined $Array); + next if (ref($Array) eq "ARRAY"); + fail("$TaskType.$LogType.$GrepType should be an array (a lines)"); + $TestInfo->{$TaskType}->{"$LogType.$GrepType"} = [ $Array ]; + } + } + } + return $TestInfo; } @@ -488,6 +504,51 @@ sub CheckLogErrors($$$) } } +sub GrepTaskLog($$$$) +{ + my ($Task, $LogName, $TaskInfo, $LogType) = @_; + + my @Grep = @{$TaskInfo->{"$LogType.Grep"} || []}; + my @GrepV = @{$TaskInfo->{"$LogType.GrepV"} || []}; + return if (!@Grep and !@GrepV); + + if (open(my $LogFile, "<", $Task->GetDir() ."/$LogName")) + { + my $LineNo; + foreach my $Line (<$LogFile>) + { + $LineNo++; + my $i = 0; + while ($i < @GrepV) + { + if ($Line =~ /$GrepV[$i]/) + { + fail(TaskKeyStr($Task) ."/$LogName:$LineNo should not match $GrepV[$i]"); + splice @GrepV, $i, 1; + next; + } + $i++; + } + $i = 0; + while ($i < @Grep) + { + if ($Line =~ /$Grep[$i]/) + { + splice @Grep, $i, 1; + next; + } + $i++; + } + last if (!@Grep and !@GrepV); + } + close($LogFile); + } + foreach my $RegExp (@Grep) + { + fail(TaskKeyStr($Task) ."/$LogName should match $RegExp"); + } +} + =pod =item <job.Status> @@ -503,6 +564,26 @@ By default the Status field should be 'completed'. Checks the value of the task's TestFailures field. By default the TestFailures field is not checked. +=item <tasks.(report|log|testbot).Grep> +=item <tasks.(report|log|testbot).GrepV> + +Verifies the presence or absence of specific regular expressions in the +specified error log or reports of the tasks in the specified category. +Multiple regular expressions can be specified so this uses the array type of +line ('a'). + +For instance: +a build.log.Grep ^Applying patch +a build.log.GrepV ^Running make_requests +a build.log.GrepV ^Running make_opengl + +Checks that the TestBot applied the patch and did not find it necessary to +run make_requests and make_opengl. + +Finally, note that while the Grep* directives are inherited by subcategories, +they are not merged. So if if both win and win32 have Grep directives, win32 +will not inherit anything from the win category. + =cut sub CheckTask($$$$) @@ -548,6 +629,7 @@ sub CheckTask($$$$) CheckLogErrors($LogInfo, $TaskInfo->{"$LogType.errors"}, TaskKeyStr($Task) ."/$LogName"); } + GrepTaskLog($Task, $LogName, $TaskInfo, $LogType); } if ($Task->Status eq "completed" and CheckValue($TaskInfo->{TestFailures})) { -- 2.20.1
This allows verifying that patches that should not result in a TestBot job are indeed ignored for the right reason (aka disposition). For instance: ----- TestWTBS ----- p patch.Disposition Does not impact the Wine build ----- TestWTBS ----- Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- testbot/tests/TestWTBS | 115 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 106 insertions(+), 9 deletions(-) diff --git a/testbot/tests/TestWTBS b/testbot/tests/TestWTBS index 58bf8a756..56864282e 100755 --- a/testbot/tests/TestWTBS +++ b/testbot/tests/TestWTBS @@ -48,6 +48,7 @@ use WineTestBot::Config; # For $PatchesMailingList use WineTestBot::Jobs; use WineTestBot::Log; use WineTestBot::LogUtils; +use WineTestBot::Patches; use WineTestBot::VMs; @@ -161,9 +162,10 @@ if (defined $Usage) print "\n"; print "Where:\n"; print " --jobs RANGES Only check the jobs in one of the specified comma-separated\n"; - print " job id ranges. Each range is either a single job id, or of the\n"; - print " form 'FIRST..LAST' where FIRST and LAST are either the empty\n"; - print " string or a job id.\n"; + print " job id ranges. This also causes patches not related to one of\n"; + print " these jobs to be ignored. Each range is either a single job id,\n"; + print " or of the form 'FIRST..LAST' where FIRST and LAST are either the\n"; + print " empty string or a job id.\n"; print " --help Shows this usage message.\n"; exit 0; } @@ -318,9 +320,12 @@ sub LoadTestInfo($) return undef if (!$HasTestInfo); # Fill in some useful defaults - SetDefault($TestInfo, "job", "Status", "completed"); - SetDefault($TestInfo, "tasks", "Status", "completed"); - SetDefault($TestInfo, "tasks", "HasTask", 1); + if (!defined $TestInfo->{patch}->{Disposition}) + { + SetDefault($TestInfo, "job", "Status", "completed"); + SetDefault($TestInfo, "tasks", "Status", "completed"); + SetDefault($TestInfo, "tasks", "HasTask", 1); + } # Then propagate the defaults foreach my $Pair (["tasks", ["build", "tests"]], @@ -666,6 +671,10 @@ sub CheckJob($$) { is($Job->Status, $JobInfo->{Status}, "Check Status for job ". $Job->Id); } + elsif (CheckValue($TestInfo->{patch}->{Disposition})) + { + fail("The '$TestInfo->{patch}->{Disposition}' patch disposition is incompatible with job ". $Job->Id); + } } sub IsJobInRange($) @@ -681,6 +690,8 @@ sub IsJobInRange($) return 0; } +my %CheckedJobs; + =pod =item <tasks.HasTask> @@ -705,12 +716,15 @@ p win.TestUnits cabinet:extract cabinet:fdi =cut -sub CheckJobTree($) +sub CheckJobTree($;$) { - my ($Job) = @_; + my ($Job, $TestInfo) = @_; return if (!IsJobInRange($Job)); - my ($TestInfo, $HasTask); + return if ($CheckedJobs{$Job->Id}); + $CheckedJobs{$Job->Id} = 1; + + my $HasTask; my $TestUnits = { wine => {} }; my $Steps = $Job->Steps; @@ -783,6 +797,85 @@ sub CheckJobs() } +# +# Verify the Patches and the patches site +# + +=pod + +=item <patch.Disposition> + +Checks the patch's disposition. +By default the disposition is supposed to point to the relevant job, in which +case default values are provided for the job.Status, tasks.Status and +tasks.HasTask properties. If a specific disposition is specified, then these +defaults are not provided since no job should be created in that case. + +For instance: +p patch.Disposition Does not impact the Wine build + +=cut + +sub CheckPatch($$$) +{ + my ($Jobs, $Patch, $TestInfo) = @_; + + my $Job; + if ($Patch->Disposition =~ /^Submitted job (\d+)$/) + { + $Job = $Jobs->GetItem($1); + } + # The job range restriction extends to their associated patches... + return if ($Job and !IsJobInRange($Job)); + # ..and to patches with no job + return if (!$Job and @JobRanges); + + my $PatchInfo = $TestInfo->{patch}; + if (CheckValue($PatchInfo->{Disposition})) + { + is($Patch->Disposition, $PatchInfo->{Disposition}, "Check Disposition for patch ". $Patch->Id .": ". $Patch->Subject); + } + elsif ($Patch->Disposition =~ /^Submitted job (.*)$/) + { + isnt($Job, undef, "Check job $1 for patch ". $Patch->Id .": ". $Patch->Subject); + if (!$Job or $Job->Status eq "canceled") + { + ; # Nothing to check + } + elsif ($Job->Status =~ /^(?:queued|running)$/) + { + print "...skipping job ". $Job->Id ." because it is not complete yet (". $Job->Status .")\n"; + } + else + { + # Provide defaults for the job checks + my $Remarks = $Patch->Subject; + $Remarks =~ s~^\[\d+/\d+\]\s~~; # In case submitted as a patch series + $Remarks =~ s~\s+$~~; + SetDefault($TestInfo, "job", "Remarks", $Remarks); + CheckJobTree($Job, $TestInfo); + } + } + else + { + fail("Patch ". $Patch->Id ."(". $Patch->Subject .") should have an associated job but Disposition is: ". $Patch->Disposition); + } +} + +sub CheckPatches() +{ + my $Jobs = CreateJobs(); + my $Patches = CreatePatches(); + foreach my $Patch (sort { $b->Id <=> $a->Id } @{$Patches->GetItems()}) + { + my $PatchFileName = "$DataDir/patches/". $Patch->Id; + next if (!-f $PatchFileName); + my $TestInfo = LoadTestInfo($PatchFileName); + CheckPatch($Jobs, $Patch, $TestInfo) if ($TestInfo); + } +} + + # # Run the tests # @@ -799,6 +892,10 @@ if (!$HasBaseVM->{build}) delete $HasBaseVM->{win64}; } +print "***** Checking Patches *****\n"; +CheckPatches(); + +print "\n***** Checking Jobs *****\n"; CheckJobs(); done_testing(); -- 2.20.1
Checks the Failed / OK status on Wine's 'Patch status' page. For instance: ----- TestWTBS ----- p webpatch.Status Failed ----- TestWTBS ----- Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- testbot/tests/TestWTBS | 100 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 97 insertions(+), 3 deletions(-) diff --git a/testbot/tests/TestWTBS b/testbot/tests/TestWTBS index 56864282e..098008110 100755 --- a/testbot/tests/TestWTBS +++ b/testbot/tests/TestWTBS @@ -202,7 +202,7 @@ sub DumpTestInfo($) { my ($TestInfo) = @_; - foreach my $Category ("patch", "job", "tasks", "tests", "win", + foreach my $Category ("webpatch", "patch", "job", "tasks", "tests", "win", "build", "win32", "win64", "wine") { WineTestBot::LogUtils::_WriteLogErrorsToFh(*STDERR, $TestInfo->{$Category}); @@ -225,6 +225,12 @@ sub CheckValue($) return (defined $Value and $Value ne "ignore"); } +sub SkipCheck($) +{ + my ($Value) = @_; + return (defined $Value and $Value eq "ignore"); +} + sub LoadTestInfo($) { my ($FileName) = @_; @@ -280,7 +286,7 @@ sub LoadTestInfo($) # Split up the information to jobs, tasks, etc. my $TestInfo = { - patch => {}, job => {}, + webpatch => {}, patch => {}, job => {}, tasks => {}, tests => {}, win => {}, build => {}, win32 => {}, win64 => {}, wine => {}, }; @@ -288,7 +294,7 @@ sub LoadTestInfo($) foreach my $Entry (keys %{$RawInfo}) { my $Field = lcfirst($Entry); - if ($Field =~ s/^(patch|job|tasks|tests|win|build|win32|win64|wine)\.//) + if ($Field =~ s/^(webpatch|patch|job|tasks|tests|win|build|win32|win64|wine)\.//) { my $TaskType = $1; $TestInfo->{$TaskType}->{$Field} = $RawInfo->{$Entry}; @@ -374,6 +380,20 @@ sub LoadTestInfo($) } } + # Automatically check webpatch.Status in simple cases + if (!defined $TestInfo->{patch}->{Disposition} and + CheckValue($TestInfo->{job}->{Status}) and + !SkipCheck($TestInfo->{win32}->{TestFailures}) and + !SkipCheck($TestInfo->{win64}->{TestFailures}) and + !SkipCheck($TestInfo->{wine}->{TestFailures})) + { + my $Status = ($TestInfo->{job}->{Status} ne 'completed' or + $TestInfo->{win32}->{TestFailures} or + $TestInfo->{win64}->{TestFailures} or + $TestInfo->{wine}->{TestFailures}) ? "Failed" : "OK"; + SetDefault($TestInfo, "webpatch", "Status", $Status); + } + return $TestInfo; } @@ -803,6 +823,78 @@ sub CheckJobs() =pod +=item <webpatch.Status> + +Checks the webpatch's status. +This is the 'Failed' or 'OK' status on the Wine 'Patch status' page (outside +the TestBot). +If patch.Disposition is not set the default is to expect the status to be +'Failed' if job.Status is not 'completed' or any of the tests has a non-zero +TestFailures field; and 'OK' otherwise. + +For instance: +p webpatch.Status Failed + +=cut + +sub CheckWebPatch($$$) +{ + my ($Patch, $Job, $TestInfo) = @_; + + # Only patches submitted through WinePatchesWebSubmit.pl get a WebPatchId. + return if (!$Patch->WebPatchId or !-d "$DataDir/webpatches"); + + my $WebFileName = "$DataDir/webpatches/". $Patch->WebPatchId; + ok(-f $WebFileName, "Check presence of the ". $Patch->WebPatchId ." webpatch"); + + my $WebResultFile = "$WebFileName.testbot"; + if (!$Job or $Job->Status !~ /^(completed|badpatch|badbuild|boterror)$/) + { + ok(!-f $WebResultFile, "Check absence of the ". $Patch->WebPatchId ." TestBot result file"); + return; + } + + ok(-f $WebResultFile, "Check presence of the ". $Patch->WebPatchId ." TestBot result file for job ". $Job->Id); + + if (open(my $ResultFh, "<", $WebResultFile)) + { + my ($HasStatus, $HasJobId, $HasURL); + while (my $Line = <$ResultFh>) + { + if ($Line =~ /^Status: (\w+)$/) + { + my $WebStatus = $1; + if (CheckValue($TestInfo->{webpatch}->{Status})) + { + is($WebStatus, $TestInfo->{webpatch}->{Status}, "Check the ". $Patch->WebPatchId ." web patch status (job ". $Job->Id .")"); + } + $HasStatus = 1; + } + elsif ($Line =~ /^Job-ID: (\d+)$/) + { + is($1, $Job->Id, "Check the ". $Patch->WebPatchId ." job id"); + $HasJobId = 1; + } + elsif ($Line =~ /^URL: (.+)$/) + { + my $JobId = $Job->Id; + ok($1 =~ m~\Q/$WebHostName\E/JobDetails\.pl\?Key=$JobId$~, + "Check the ". $Patch->WebPatchId ." URL"); + $HasURL = 1; + } + # Ignore the rest of the file + last if ($HasStatus and $HasJobId and $HasURL); + } + close($ResultFh); + } + else + { + fail("Could not open $WebResultFile for reading: $!"); + } +} + +=pod + =item <patch.Disposition> Checks the patch's disposition. @@ -860,6 +952,8 @@ sub CheckPatch($$$) { fail("Patch ". $Patch->Id ."(". $Patch->Subject .") should have an associated job but Disposition is: ". $Patch->Disposition); } + + CheckWebPatch($Patch, $Job, $TestInfo); } sub CheckPatches() -- 2.20.1
participants (1)
-
Francois Gouget