[tools resubmit] testbot/PatchUtils: Rework GetPatchImpacts() to fix patchset handling.
Rename the Tests set to Modules since it contains information about each module impacted by the patch, including a full list of the still existing and patched test units. Better separate the build impacts from the test impacts: the former impact the build of all following parts, while only the last part impacts the tests that need to be run. This stops the TestBot from running test units which were not impacted by the _last part_ of the patchset. Document the returned $Impacts structure. Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- With the Signed-off-by line this time! For the following wtbsuite tests: WTBS S8.2 - Check the make_* scripts are still run (advpack:advpack). WTBS S5.6 - Rerun a single test despite previous patches (serialui:confdlg). WTBS S5.5 - A non-test patch to another dll (serialui). WTBS S5.4 - Rerun a single test despite previous patches (msi:source). WTBS S4.3 - Trigger an autoconf run. WTBS S4.2 - Another non-test Wine patch (api-ms-win-core-timezone-l1-1-0). --- testbot/bin/WineRunBuild.pl | 6 +- testbot/bin/build/WineTest.pl | 30 +-- testbot/lib/WineTestBot/PatchUtils.pm | 370 ++++++++++++++++---------- testbot/lib/WineTestBot/Patches.pm | 15 +- testbot/web/Submit.pl | 36 ++- 5 files changed, 266 insertions(+), 191 deletions(-) diff --git a/testbot/bin/WineRunBuild.pl b/testbot/bin/WineRunBuild.pl index adb1959f0..8a92f88d1 100755 --- a/testbot/bin/WineRunBuild.pl +++ b/testbot/bin/WineRunBuild.pl @@ -461,16 +461,16 @@ foreach my $TestStep (@{$Job->Steps->GetItems()}) my $Impacts = GetPatchImpacts($FileName); my $StepDir = $Step->CreateDir(); -foreach my $TestInfo (values %{$Impacts->{Tests}}) +foreach my $Module (values %{$Impacts->{Modules}}) { foreach my $Bits ("", "64") { - my $Local = "$TestInfo->{ExeBase}$Bits.exe"; + my $Local = "$Module->{ExeBase}$Bits.exe"; next if (!$TestExes{$Local}); Debug(Elapsed($Start), " Retrieving '$Local'\n"); my $BuildDir = "wine-$TestExes{$Local}"; - if ($TA->GetFile("$BuildDir/$TestInfo->{Path}/$TestInfo->{ExeBase}.exe", + if ($TA->GetFile("$BuildDir/$Module->{Path}/$Module->{ExeBase}.exe", "$StepDir/$Local")) { chmod 0664, "$StepDir/$Local"; diff --git a/testbot/bin/build/WineTest.pl b/testbot/bin/build/WineTest.pl index 3e13637d0..ffd665faa 100755 --- a/testbot/bin/build/WineTest.pl +++ b/testbot/bin/build/WineTest.pl @@ -118,21 +118,21 @@ sub TestPatch($$) } else { - foreach my $Module (sort keys %{$Impacts->{Tests}}) + foreach my $ModuleName (sort keys %{$Impacts->{Modules}}) { - my $TestInfo = $Impacts->{Tests}->{$Module}; - if ($TestInfo->{All} or - ($Mission->{test} eq "module" and $TestInfo->{PatchedModule})) + my $Module = $Impacts->{Modules}->{$ModuleName}; + if ($Module->{All} or + ($Mission->{test} eq "module" and $Module->{Patched})) { # When given a module name WineTest runs all its tests. # But make sure the module actually has tests first! - push @TestList, $Module if (%{$TestInfo->{Files}}); + push @TestList, $ModuleName if (@{$Module->{TestUnits}}); } else { - foreach my $PatchedUnit (sort keys %{$TestInfo->{PatchedUnits}}) + foreach my $TestUnit (@{$Module->{RunTestUnits}}) { - push @TestList, "$Module:$PatchedUnit"; + push @TestList, "$ModuleName:$TestUnit"; } } } @@ -157,15 +157,15 @@ sub TestPatch($$) sub GetExecutable($$$) { - my ($Mission, $Impacts, $Module) = @_; + my ($Mission, $Impacts, $ModuleName) = @_; - my $TestInfo = $Impacts->{Tests}->{$Module}; - my $TestExecutable = "$TestInfo->{ExeBase}.exe"; + my $Module = $Impacts->{Modules}->{$ModuleName}; + my $TestExecutable = "$Module->{ExeBase}.exe"; my $Dst = "$DataDir/staging/$TestExecutable"; unlink $Dst; my $WineDir = GetWineDir($Mission); - if (!symlink "$WineDir/$TestInfo->{Path}/$TestExecutable", $Dst) + if (!symlink "$WineDir/$Module->{Path}/$TestExecutable", $Dst) { LogMsg "Could not find $TestExecutable: $!\n"; return undef; @@ -207,7 +207,7 @@ sub TestExe($$$) # my $Action = ""; -my ($Usage, $OptNoSubmit, $MissionStatement, $FileName, $Module, $BaseTag); +my ($Usage, $OptNoSubmit, $MissionStatement, $FileName, $ModuleName, $BaseTag); while (@ARGV) { my $Arg = shift @ARGV; @@ -268,9 +268,9 @@ while (@ARGV) last; # The remainder are the executable's arguments } } - elsif ($Action eq "testpatch" and !defined $Module) + elsif ($Action eq "testpatch" and !defined $ModuleName) { - $Module = $Arg; + $ModuleName = $Arg; last; # The remainder are the executable's arguments } else @@ -431,7 +431,7 @@ foreach my $Mission (@{$TaskMissions->{Missions}}) } elsif (@ARGV) { - my $FileName = GetExecutable($Mission, $Impacts, $Module); + my $FileName = GetExecutable($Mission, $Impacts, $ModuleName); exit(1) if (!$FileName or !TestExe($Mission, $FileName, \@ARGV)); } else diff --git a/testbot/lib/WineTestBot/PatchUtils.pm b/testbot/lib/WineTestBot/PatchUtils.pm index 87e4508ba..0d60fdb29 100644 --- a/testbot/lib/WineTestBot/PatchUtils.pm +++ b/testbot/lib/WineTestBot/PatchUtils.pm @@ -187,25 +187,24 @@ sub LastPartSeparator() return "===== TestBot: Last patchset part =====\n"; } -sub _CreateTestInfo($$$) +sub _CreateModuleInfo($$$) { my ($Impacts, $Root, $Dir) = @_; - my $Module = _Dir2ModuleName($Root, $Dir); - $Impacts->{BuildModules}->{$Module} = 1; + my $ModuleName = _Dir2ModuleName($Root, $Dir); $Impacts->{IsWinePatch} = 1; - my $Tests = $Impacts->{Tests}; - if (!$Tests->{$Module}) + my $Module = $Impacts->{Modules}->{$ModuleName}; + if (!$Module) { - $Tests->{$Module} = { - "Module" => $Module, + $Impacts->{Modules}->{$ModuleName} = $Module = { + "Name" => $ModuleName, "Path" => "$Root/$Dir/tests", - "ExeBase" => "${Module}_test", + "ExeBase" => "${ModuleName}_test", }; - foreach my $File (keys %{$_TestList->{$Module}}) + foreach my $File (keys %{$_TestList->{$ModuleName}}) { - $Tests->{$Module}->{Files}->{$File} = 0; # not modified + $Module->{TestFiles}->{$File} = 0; # not modified } } @@ -231,9 +230,9 @@ sub _HandleFile($$$) { my ($Root, $Dir, $File) = ($1, $2, $3); - my $Module = _CreateTestInfo($Impacts, $Root, $Dir); - $Impacts->{PatchedTests} = 1; - $Impacts->{Tests}->{$Module}->{Files}->{$File} = $Change; + my $Module = _CreateModuleInfo($Impacts, $Root, $Dir); + $Module->{TestFiles}->{$File} = $Change; + $Module->{PatchedTests} = 1; if ($File eq "Makefile.in" and $Change ne "modify") { @@ -247,10 +246,9 @@ sub _HandleFile($$$) foreach my $Dir ($PatchedDir, keys %{$_WineParentDirs->{$PatchedDir} || {}}) { - my $Module = _CreateTestInfo($Impacts, $Root, $Dir); - $Impacts->{Tests}->{$Module}->{PatchedModule} = 1; + my $Module = _CreateModuleInfo($Impacts, $Root, $Dir); + $Module->{Patched} = 1; } - $Impacts->{PatchedModules} = 1; if ($File eq "Makefile.in" and $Change ne "modify") { @@ -258,35 +256,31 @@ sub _HandleFile($$$) $Impacts->{MakeMakefiles} = 1; } } - else + elsif ($Impacts->{WineFiles}->{$FilePath}) { - my $WineFiles = $Impacts->{WineFiles} || $_WineFiles; - if ($WineFiles->{$FilePath}) + if ($FilePath !~ /^(?:$AmbiguousPathsRe)/) { - if ($FilePath !~ /^(?:$AmbiguousPathsRe)/) - { - $Impacts->{IsWinePatch} = 1; - } - # Else this file exists in Wine but has a very common name so it may just - # as well belong to another repository. + $Impacts->{IsWinePatch} = 1; + } + # Else this file exists in Wine but has a very common name so it may just + # as well belong to another repository. - if ($FilePath !~ /^(?:$IgnoredPathsRe)/) + if ($FilePath !~ /^(?:$IgnoredPathsRe)/) + { + $Impacts->{BuildRoot} = $Impacts->{PatchedRoot} = 1; + if ($FilePath =~ m~/Makefile\.in$~ and $Change ne "modify") { - $Impacts->{PatchedRoot} = 1; - if ($FilePath =~ m~/Makefile\.in$~ and $Change ne "modify") - { - # This adds / removes a directory - $Impacts->{MakeMakefiles} = 1; - } + # This adds / removes a directory + $Impacts->{MakeMakefiles} = 1; } - # Else patches to this file don't impact the Wine build. - } - elsif ($FilePath =~ m~/Makefile\.in$~ and $Change eq "new") - { - # This may or may not be a Wine patch but the new Makefile.in will be - # added to the build by make_makefiles. - $Impacts->{PatchedRoot} = $Impacts->{MakeMakefiles} = 1; } + # Else patches to this file don't impact the Wine build. + } + elsif ($FilePath =~ m~/Makefile\.in$~ and $Change eq "new") + { + # This may or may not be a Wine patch but the new Makefile.in will be + # added to the build by make_makefiles. + $Impacts->{BuildRoot} = $Impacts->{PatchedRoot} = $Impacts->{MakeMakefiles} = 1; } } @@ -299,6 +293,121 @@ Analyzes a patch and returns a hashtable describing the impact it has on the Wine build: whether it requires updating the makefiles, re-running autoconf or configure, whether it impacts the tests, etc. +Notes: A module is a dlls/ or programs/ directory. All references to patchsets +mean the patchset as being tested. So if a patch series has 5 parts and the +TestBot is testing part 3, the LAST PART is part 3, and ANY PART means part 1, +2 or 3. + +=over +=item IsWinePatch + +This flag is set if the LAST PART of the patchset has been identified as a +Wine patch. + +=item BuildModules + +This is a count of the number of modules where either the source or the tests +have been modified by ANY PART of the patchset. + +=item BuildRoot + +This flag is set if ANY PART of the patchset modified the Wine source +outside of a specific module in a way that impacts the build. + +=item Autoconf +=item MakeErrors +=item MakeFir +=item MakeMakefiles +=item MakeOpenGL +=item MakeRequests +=item MakeUnicode +=item MakeVulkan + +These flags are set if ANY PART of the patchset requires running the +corresponding script before building Wine. + +=item TestBuild + +This flag is set if the LAST PART of the patchset modified the Wine source +such that the build needs to be tested. + +=item RunTestUnits +=item PatchedModules +=item PatchedModuleOrTests + +These are counts based on the LAST PART of the patchset, so excluding any +impact from PREVIOUS PARTS. They count, respectively, the test units that need +to be rerun (so excluding deleted ones), the modules whose source has been +modified (including file deletions), and the modules where either the source +or the tests have been modified (again including deletions). + +=item PatchedRoot + +This flag is set if the LAST PART of the patchset modified the Wine source +outside of a specific module. + +=item Modules + +A set of structures containing information about each module impacted by +ANY PART of the patchset, indexed by module name. Each module has the +following information: + +=over + +=item Patched + +This flag is true if the LAST PART of the patchset has modified the module +itself, so excluding its test units. + +=item PatchedTests + +This flag is true if the LAST PART of the patchset has modified the tests of +the module. + +=item TestUnits + +This is an alphabetical list of the module's test units, after applying the +patchset (so excluding any test unit deleted by the patchset). + +=item All + +This flag is true if all the module's test units are impacted by the LAST PART +of the patchset. This is typically true if a resource file or helper dll is +modified. + +=item RunTestUnits + +This is an alphabetical list of the module's test units that are impacted by +the LAST PART of the patchset such that they should be rerun. In particular if +All is true this is identical to TestUnits. + +=item TestFiles + +This hashtable is indexed by the filenames of the files in the module's tests +directory. Note that this is not limited to the test unit C files but also +includes the resource files, makefile and helper dlls (but not testlist.c). +For each file the value describes the modification performed by the +LAST PART of the patchset: 0 (false) if there is no modification, "modify" for +modified files, "new" for added files and "rm" for deletions. + +=back + +=item WineFiles + +The keys of this hashtable enumerate all the Wine source files, including all +files added and removed by PREVIOUS PARTS of the patchset. +This is mostly for the internal use of the impact analyzer and should be +treated as read-only by outside code (and copy-on-write by the analyzer). + +=item NewFiles +=item DeletedFiles + +The keys of these hashtables enumerate, respectively, the files added and +removed by the LAST PART of the patchset. +This is mostly for the internal use of the impact analyzer. + +=back + =back =cut @@ -309,39 +418,31 @@ sub GetPatchImpacts($) my $fh; return undef if (!open($fh, "<", $PatchFileName)); - my $Impacts = { - # Number of test units impacted either directly, or indirectly by a module - # patch. - ModuleUnitCount => 0, - # Number of patched test units. - TestUnitCount => 0, - # The modules that need a rebuild, even if only for the tests. - BuildModules => {}, - # Information about 'tests' directories. - Tests => {}, - }; _LoadWineFiles(); - my $PastImpacts; + my $Impacts = { Modules => {}, WineFiles => $_WineFiles }; my ($Path, $Change); while (my $Line = <$fh>) { + # All the files on '---' lines will be passed to _HandleFile() when the + # corresponding '+++' line comes. if ($Line =~ m=^--- \w+/(aclocal\.m4|configure\.ac)$=) { $Path = $1; - $Impacts->{PatchedRoot} = $Impacts->{Autoconf} = 1; + $Impacts->{BuildRoot} = $Impacts->{Autoconf} = 1; + $Impacts->{PatchedRoot} = 1; } elsif ($Line =~ m=^--- \w+/(tools/make_makefiles)$=) { $Path = $1; - $Impacts->{PatchedRoot} = $Impacts->{MakeMakefiles} = 1; - $Impacts->{IsWinePatch} = 1; + $Impacts->{BuildRoot} = $Impacts->{MakeMakefiles} = 1; + $Impacts->{PatchedRoot} = $Impacts->{IsWinePatch} = 1; } elsif ($Line =~ m=^--- \w+/(tools/make_requests|server/protocol\.def)$=) { $Path = $1; - $Impacts->{PatchedRoot} = $Impacts->{MakeRequests} = 1; - $Impacts->{IsWinePatch} = 1; + $Impacts->{BuildRoot} = $Impacts->{MakeRequests} = 1; + $Impacts->{PatchedRoot} = $Impacts->{IsWinePatch} = 1; } elsif ($Line =~ m=^--- \w+/(dlls/dsound/make_fir)$=) { @@ -358,20 +459,20 @@ sub GetPatchImpacts($) elsif ($Line =~ m=^--- \w+/(dlls/opengl32/make_opengl|dlls/opengl32/winegl\.xml|include/wine/wgl_driver\.h)$=) { $Path = $1; - $Impacts->{PatchedRoot} = $Impacts->{MakeOpenGL} = 1; - $Impacts->{IsWinePatch} = 1; + $Impacts->{BuildRoot} = $Impacts->{MakeOpenGL} = 1; + $Impacts->{PatchedRoot} = $Impacts->{IsWinePatch} = 1; } elsif ($Line =~ m=^--- \w+/(dlls/winevulkan/make_vulkan)$=) { $Path = $1; - $Impacts->{PatchedRoot} = $Impacts->{MakeVulkan} = 1; - $Impacts->{IsWinePatch} = 1; + $Impacts->{BuildRoot} = $Impacts->{MakeVulkan} = 1; + $Impacts->{PatchedRoot} = $Impacts->{IsWinePatch} = 1; } elsif ($Line =~ m=^--- \w+/(tools/make_unicode)$=) { $Path = $1; - $Impacts->{PatchedRoot} = $Impacts->{MakeUnicode} = 1; - $Impacts->{IsWinePatch} = 1; + $Impacts->{BuildRoot} = $Impacts->{MakeUnicode} = 1; + $Impacts->{PatchedRoot} = $Impacts->{IsWinePatch} = 1; } elsif ($Line =~ m=^--- /dev/null$=) { @@ -398,16 +499,18 @@ sub GetPatchImpacts($) elsif ($Line eq LastPartSeparator()) { # All the diffs so far belong to previous parts of this patchset. - # But: - # - Only the last part must be taken into account to determine if a - # rebuild and testing is needed. - # - Yet if a rebuild is needed the previous parts' patches will impact - # the scope of the rebuild so that information must be preserved. - # So save current impacts in $PastImpacts and reset the current state. - $PastImpacts = {}; - - # Build a copy of the Wine files list reflecting the current situation. - $Impacts->{WineFiles} = { %$_WineFiles } if (!$Impacts->{WineFiles}); + # But only the last part must be taken into account to determine if + # testing is needed. So reset all the Patched* fields. + + delete $Impacts->{PatchedRoot}; + foreach my $Module (values %{$Impacts->{Modules}}) + { + delete $Module->{Patched}; + delete $Module->{PatchedTests}; + } + + # Make a copy of the Wine files list reflecting the current situation. + $Impacts->{WineFiles} = { %{$Impacts->{WineFiles}} }; if ($Impacts->{IsWinePatch} or $Impacts->{MakeMakefiles}) { map { $Impacts->{WineFiles}->{$_} = 1 } keys %{$Impacts->{NewFiles}}; @@ -417,30 +520,23 @@ sub GetPatchImpacts($) delete $Impacts->{IsWinePatch}; } - # The modules impacted by previous parts will still need to be built, - # but only if the last part justifies a build. So make a backup. - $PastImpacts->{BuildModules} = $Impacts->{BuildModules}; - $Impacts->{BuildModules} = {}; - - # Also backup the build-related fields. - foreach my $Field ("PatchedRoot", "PatchedModules", "PatchedTests") + # Reset the status of all modified test unit files to not modified + # (deleted files won't come back). + foreach my $Module (values %{$Impacts->{Modules}}) { - $PastImpacts->{$Field} = $Impacts->{$Field}; - $Impacts->{$Field} = undef; - } - - # Reset the status of all test unit files to not modified. - foreach my $TestInfo (values %{$Impacts->{Tests}}) - { - foreach my $File (keys %{$TestInfo->{Files}}) + my $TestFiles = $Module->{TestFiles}; + foreach my $File (keys %$TestFiles) { - if ($TestInfo->{Files}->{$File} ne "rm") + if ($TestFiles->{$File} eq "rm") + { + delete $TestFiles->{$File}; + } + else { - $TestInfo->{Files}->{$File} = 0; + $TestFiles->{$File} = 0; } } } - $Impacts->{ModuleUnitCount} = $Impacts->{TestUnitCount} = 0; } else { @@ -450,76 +546,58 @@ sub GetPatchImpacts($) } close($fh); - foreach my $TestInfo (values %{$Impacts->{Tests}}) + # Calculate the impact counts and lists of test units. + foreach my $Module (values %{$Impacts->{Modules}}) { - # For each module, identify modifications to non-C files and helper dlls - foreach my $File (keys %{$TestInfo->{Files}}) + $Module->{TestUnits} = []; + $Module->{RunTestUnits} = []; + + # For each module build the lists of test units + my $TestFiles = $Module->{TestFiles}; + foreach my $File (sort keys %$TestFiles) { - # Skip unmodified files - next if (!$TestInfo->{Files}->{$File}); # Assume makefile modifications may break the build but not the tests next if ($File eq "Makefile.in"); my $Base = $File; if ($Base !~ s/(?:\.c|\.spec)$//) { - # Any change to a non-C non-Spec file can potentially impact all tests - $TestInfo->{All} = 1; - last; + # Any change to a non-C non-Spec file (including deletions) can + # potentially impact all tests + $Module->{All} = 1 if ($TestFiles->{$File}); + next; } - if (exists $TestInfo->{Files}->{"$Base.spec"} and - ($TestInfo->{Files}->{"$Base.c"} or - $TestInfo->{Files}->{"$Base.spec"})) - { - # Any change to a helper dll can potentially impact all tests - $TestInfo->{All} = 1; - last; - } - } - $TestInfo->{PatchedUnits} = {}; - foreach my $File (keys %{$TestInfo->{Files}}) - { - my $Base = $File; - # Non-C files are not test units - next if ($Base !~ s/(?:\.c|\.spec)$//); - # Helper dlls are not test units - next if (exists $TestInfo->{Files}->{"$Base.spec"}); - # Don't try running a deleted test unit obviously - next if ($TestInfo->{Files}->{$File} eq "rm"); - $TestInfo->{AllUnits}->{$Base} = 1; - - if ($TestInfo->{All} or $TestInfo->{Files}->{$File}) + if (exists $TestFiles->{"$Base.spec"}) { - $TestInfo->{PatchedUnits}->{$Base} = 1; - $Impacts->{ModuleUnitCount}++; + # This is a helper dll, not a test unit. + # Any change, including deletions, could impact all tests. + $Module->{All} ||= ($TestFiles->{"$Base.c"} or + $TestFiles->{"$Base.spec"}); + next; } - elsif ($TestInfo->{PatchedModule}) + + if ($TestFiles->{$File} ne "rm") { - # The module has been patched so this test unit is impacted indirectly. - $Impacts->{ModuleUnitCount}++; + push @{$Module->{TestUnits}}, $Base; + push @{$Module->{RunTestUnits}}, $Base if ($TestFiles->{$File}); } } + $Module->{All} ||= (@{$Module->{TestUnits}} == @{$Module->{RunTestUnits}}); + $Module->{RunTestUnits} = $Module->{TestUnits} if ($Module->{All}); - $TestInfo->{UnitCount} = scalar(keys %{$TestInfo->{PatchedUnits}}); - $Impacts->{TestUnitCount} += $TestInfo->{UnitCount}; - } - - if ($Impacts->{PatchedRoot} or $Impacts->{PatchedModules} or - $Impacts->{PatchedTests}) - { - # Any patched area will need to be rebuilt... - $Impacts->{RebuildRoot} = $Impacts->{PatchedRoot}; - $Impacts->{RebuildModules} = $Impacts->{PatchedModules}; + my $RunTestUnits = scalar(@{$Module->{RunTestUnits}}); + $Impacts->{RunTestUnits} += $RunTestUnits; + $Impacts->{ModuleTestUnits} += $Module->{Patched} ? scalar(@{$Module->{TestUnits}}) : $RunTestUnits; - # ... even if the patch was in previous parts - if ($PastImpacts) + if ($Module->{Patched} or $Module->{PatchedTests}) { - $Impacts->{RebuildRoot} ||= $PastImpacts->{PatchedRoot}; - $Impacts->{RebuildModules} ||= $PastImpacts->{PatchedModules}; - map { $Impacts->{BuildModules}->{$_} = 1 } keys %{$PastImpacts->{BuildModules}}; + $Impacts->{PatchedModuleOrTests}++; + $Impacts->{PatchedModules}++ if ($Module->{Patched}); } + $Impacts->{BuildModules}++; } + $Impacts->{TestBuild} = $Impacts->{PatchedRoot} || $Impacts->{PatchedModuleOrTests}; return $Impacts; } @@ -534,11 +612,11 @@ sub GetBuildTimeout($$) my ($Impacts, $TaskMissions) = @_; my ($ExeCount, $WineCount); - map {$_ =~ /^exe/ ? $ExeCount++ : $WineCount++ } keys %{$TaskMissions->{Builds}}; + map { $_ =~ /^exe/ ? $ExeCount++ : $WineCount++ } keys %{$TaskMissions->{Builds}}; # Set $ModuleCount to 0 if a full rebuild is needed - my $ModuleCount = (!$Impacts or $Impacts->{RebuildRoot}) ? 0 : - scalar(keys %{$Impacts->{BuildModules}}); + my $ModuleCount = (!$Impacts or $Impacts->{BuildRoot}) ? 0 : + $Impacts->{BuildModules}; my $Reconfig = (!$Impacts or $Impacts->{Autoconf} or $Impacts->{MakeMakefiles}); my ($ExeTimeout, $WineTimeout) = (0, 0); @@ -575,9 +653,9 @@ sub GetTestTimeout($$) elsif ($Mission->{test} ne "build") { # Note: If only test units have been patched then - # ModuleUnitCount == TestUnitCount. - my $UnitCount = $Mission->{test} eq "test" ? $Impacts->{TestUnitCount} : - $Impacts->{ModuleUnitCount}; + # ModuleTestUnits == RunTestUnits. + my $UnitCount = $Mission->{test} eq "test" ? $Impacts->{RunTestUnits} : + $Impacts->{ModuleTestUnits}; my $TestsTimeout = min(2, $UnitCount) * $SingleTimeout + max(0, $UnitCount - 2) * $SingleAvgTime; $Timeout += min($SuiteTimeout, $TestsTimeout); diff --git a/testbot/lib/WineTestBot/Patches.pm b/testbot/lib/WineTestBot/Patches.pm index b6a8af4c4..cc0fbe94a 100644 --- a/testbot/lib/WineTestBot/Patches.pm +++ b/testbot/lib/WineTestBot/Patches.pm @@ -213,8 +213,7 @@ sub Submit($$;$) $SubjectInfo ||= $self->ParseSubject(); my $Impacts = GetPatchImpacts($PatchFileName); - if (!$Impacts->{PatchedRoot} and !$Impacts->{PatchedModules} and - !$Impacts->{PatchedTests}) + if (!$Impacts->{TestBuild}) { if ($Impacts->{IsWinePatch}) { @@ -255,14 +254,14 @@ sub Submit($$;$) my $BuildVMs = CreateVMs(); $BuildVMs->AddFilter("Type", ["build"]); $BuildVMs->AddFilter("Role", ["base"]); - if ($Impacts->{TestUnitCount} and !$BuildVMs->IsEmpty()) + if ($Impacts->{RunTestUnits} and !$BuildVMs->IsEmpty()) { # Create steps for the Windows tests my ($BuildStep, $BuildMissions); - foreach my $Module (sort keys %{$Impacts->{Tests}}) + foreach my $ModuleName (sort keys %{$Impacts->{Modules}}) { - my $TestInfo = $Impacts->{Tests}->{$Module}; - foreach my $PatchedUnit (sort keys %{$TestInfo->{PatchedUnits}}) + my $Module = $Impacts->{Modules}->{$ModuleName}; + foreach my $TestUnit (@{$Module->{RunTestUnits}}) { foreach my $Bits ("32", "64") { @@ -305,7 +304,7 @@ sub Submit($$;$) # Create one Step per (module, unit, bitness) combination my $NewStep = $NewJob->Steps->Add(); $NewStep->PreviousNo($BuildStep->No); - my $FileName = $TestInfo->{ExeBase}; + my $FileName = $Module->{ExeBase}; $FileName .= "64" if ($Bits eq "64"); $NewStep->FileName("$FileName.exe"); $NewStep->FileType("exe$Bits"); @@ -317,7 +316,7 @@ sub Submit($$;$) $Task->VM($VM); $Task->Timeout($SingleTimeout); $Task->Missions($TaskMissions->{Statement}); - $Task->CmdLineArg($PatchedUnit); + $Task->CmdLineArg($TestUnit); } } } diff --git a/testbot/web/Submit.pl b/testbot/web/Submit.pl index 68978f089..76069bbc3 100644 --- a/testbot/web/Submit.pl +++ b/testbot/web/Submit.pl @@ -168,15 +168,13 @@ sub _AnalyzePatch($) $self->{Impacts} ||= GetPatchImpacts($self->_GetStagingFilePath()); - if (!$self->{Impacts}->{PatchedRoot} and - !$self->{Impacts}->{PatchedModules} and - !$self->{Impacts}->{PatchedTests}) + if (!$self->{Impacts}->{TestBuild}) { $self->{ErrField} = "FileName"; $self->{ErrMessage} = "'$self->{FileName}' is not a valid patch"; return undef; } - if ($self->{Impacts}->{ModuleUnitCount} == 0) + if ($self->{Impacts}->{PatchedModuleOrTests} == 0) { $self->{ErrField} = "FileName"; $self->{ErrMessage} = "The patch does not directly impact the Wine dlls and programs"; @@ -307,11 +305,10 @@ sub _GetTestExecutables($) { if ($self->{FileType} eq "patch") { - foreach my $Module (sort keys %{$self->{Impacts}->{Tests}}) + foreach my $Module (values %{$self->{Impacts}->{Modules}}) { - my $TestInfo = $self->{Impacts}->{Tests}->{$Module}; - next if (!%{$TestInfo->{Files}}); - $self->{TestExecutables}->{"$TestInfo->{ExeBase}.exe"} = $Module; + next if (!@{$Module->{TestUnits}}); + $self->{TestExecutables}->{"$Module->{ExeBase}.exe"} = $Module->{Name}; } } else @@ -332,10 +329,10 @@ sub _ValidateTestExecutable($;$) $self->{TestExecutable} = (sort keys %{$self->{TestExecutables}})[0]; if (!defined $self->{CmdLineArg} and $self->{Impacts}) { - my $Module = $self->{TestExecutables}->{$self->{TestExecutable}}; - my $TestInfo = $self->{Impacts}->{Tests}->{$Module}; - $self->{CmdLineArg} = (sort keys %{$TestInfo->{PatchedUnits}})[0] || - (sort keys %{$TestInfo->{AllUnits}})[0]; + my $ModuleName = $self->{TestExecutables}->{$self->{TestExecutable}}; + my $Module = $self->{Impacts}->{Modules}->{$ModuleName}; + $self->{CmdLineArg} = ($Module->{RunTestUnits}->[0] || + $Module->{TestUnits}->[0]); } } @@ -640,21 +637,22 @@ sub GetHeaderText($) $self->_GetTestExecutables(); foreach my $TestExecutable (sort keys %{$self->{TestExecutables}}) { - my $Module = $self->{TestExecutables}->{$TestExecutable}; - my $TestInfo = $self->{Impacts}->{Tests}->{$Module}; + my $ModuleName = $self->{TestExecutables}->{$TestExecutable}; + my $Module = $self->{Impacts}->{Modules}->{$ModuleName}; my @TestUnits; - foreach my $TestUnit (sort keys %{$TestInfo->{AllUnits}}) + foreach my $TestUnit (@{$Module->{TestUnits}}) { - if ($TestInfo->{PatchedUnits}->{$TestUnit}) + if ($Module->{TestFiles}->{"$TestUnit.c"}) { + # This test unit was patched push @TestUnits, "<i>$TestUnit</i>"; } - elsif ($TestInfo->{All} or $TestInfo->{PatchedModule}) + elsif ($Module->{All} or $Module->{Patched}) { push @TestUnits, $TestUnit; } } - push @TestExecutables, "$Module: ". join(" ", @TestUnits) if (@TestUnits); + push @TestExecutables, "$ModuleName: ". join(" ", @TestUnits) if (@TestUnits); } push @Headers, join("<br>\n", "Here is a list of the test executables impacted by the patch (patched test units, if any, are in italics).", @TestExecutables); } @@ -788,7 +786,7 @@ sub GenerateFields($) if (!$self->{UserVMSelection} and !$VMRow->{Extra} and $VMRow->{VM}->Status !~ /^(?:offline|maintenance)$/ and ($VMRow->{VM}->Type eq "wine" or !$self->{Impacts} or - $self->{Impacts}->{TestUnitCount})) + $self->{Impacts}->{RunTestUnits})) { $VMRow->{Checked} = 1; } -- 2.20.1
participants (1)
-
Francois Gouget