[PATCH 1/2] testbot/Submit: Allow submitting patches that touch zero or multiple test units.
Patches submitted through the submit page no longer need to impact one and only one test unit. This means it's no longer necessary to split longer patches in order to test them. With the Wine VMs it also no longer makes sense to ban patches that don't touch the tests as one may need to make sure they compile and don't cause regressions in existing tests. However it is still necessary to specify command line arguments for the test executable which means picking a single test unit to run.
Signed-off-by: Francois Gouget fgouget@codeweavers.com --- testbot/bin/WineRunWineTest.pl | 1 + testbot/bin/build/Build.pl | 6 + testbot/bin/build/WineTest.pl | 79 +++++++++---- testbot/lib/Build/Utils.pm | 43 +++++--- testbot/lib/WineTestBot/PatchUtils.pm | 1 + testbot/web/Submit.pl | 153 ++++++++++++++++++-------- 6 files changed, 199 insertions(+), 84 deletions(-)
diff --git a/testbot/bin/WineRunWineTest.pl b/testbot/bin/WineRunWineTest.pl index beb050276..ff5317c8b 100755 --- a/testbot/bin/WineRunWineTest.pl +++ b/testbot/bin/WineRunWineTest.pl @@ -484,6 +484,7 @@ if ($Step->Type eq "suite") elsif ($Step->FileType eq "patch") { $Script .= "--testpatch ". ShQuote($Task->Missions) ." patch.diff"; + $Script .= " ". $Task->CmdLineArg if (defined $Task->CmdLineArg); } else { diff --git a/testbot/bin/build/Build.pl b/testbot/bin/build/Build.pl index f23b733d6..4c342a5ac 100755 --- a/testbot/bin/build/Build.pl +++ b/testbot/bin/build/Build.pl @@ -87,6 +87,12 @@ sub BuildTestExecutables($$$) }
InfoMsg "\nBuilding the $Build Wine test executable(s)\n"; + if (!@BuildDirs) + { + InfoMsg "Nothing to do\n"; + return 1; + } + my $CPUCount = GetCPUCount(); system("cd '$DataDir/wine-$Build' && set -x && ". "time make -j$CPUCount ". join(" ", sort @BuildDirs)); diff --git a/testbot/bin/build/WineTest.pl b/testbot/bin/build/WineTest.pl index 37c20d1b4..9948e0929 100755 --- a/testbot/bin/build/WineTest.pl +++ b/testbot/bin/build/WineTest.pl @@ -121,9 +121,7 @@ sub DailyWineTest($$$$)
sub TestPatch($$) { - my ($Mission, $Impacts) = @_; - - return 1 if ($Mission->{test} eq "build"); + my ($Mission, $Impacts, $Args) = @_;
my @TestList; if ($Mission->{test} eq "all" and @@ -180,6 +178,24 @@ sub TestPatch($$) return 1; }
+sub GetExecutable($$$) +{ + my ($Mission, $Impacts, $Module) = @_; + + my $TestInfo = $Impacts->{Tests}->{$Module}; + my $TestExecutable = "$TestInfo->{ExeBase}.exe"; + my $Dst = "$DataDir/staging/$TestExecutable"; + unlink $Dst; + + my $WineDir = GetWineDir($Mission); + if (!symlink "$WineDir/$TestInfo->{Path}/$TestExecutable", $Dst) + { + LogMsg "Could not find $TestExecutable: $!\n"; + return undef; + } + return $TestExecutable; +} + sub TestExe($$$) { my ($Mission, $FileName, $Args) = @_; @@ -196,6 +212,21 @@ sub TestExe($$$) } }
+ my $TestDir = GetTestDir($Mission); + if ($TestDir eq ".") + { + $FileName = "$DataDir/staging/$FileName"; + } + else + { + unlink "$TestDir/$FileName"; + if (!symlink "$DataDir/staging/$FileName", "$TestDir/$FileName") + { + LogMsg "Could not create '$FileName' in the test directory: $!\n"; + return 0; + } + } + # Run the test executable RunWine($Mission, GetTestLauncher($Mission), "-t 120 ". ShArgv2Cmd($FileName, @$Args) @@ -210,7 +241,7 @@ sub TestExe($$$) #
my $Action = ""; -my ($Usage, $OptNoSubmit, $MissionStatement, $FileName, $BaseTag); +my ($Usage, $OptNoSubmit, $MissionStatement, $FileName, $Module, $BaseTag); while (@ARGV) { my $Arg = shift @ARGV; @@ -248,17 +279,16 @@ while (@ARGV) elsif ($Action eq "winetest") { $BaseTag = $Arg; - # The remaining arguments are meant for WineTest - last; + last; # The remaining arguments are meant for WineTest } elsif (!defined $FileName) { if (IsValidFileName($Arg)) { - $FileName = "$DataDir/staging/$Arg"; - if (!-r $FileName) + $FileName = $Arg; + if (!-r "$DataDir/staging/$FileName") { - Error "'$Arg' is not readable\n"; + Error "'$FileName' is not readable\n"; $Usage = 2; } } @@ -266,14 +296,17 @@ while (@ARGV) { Error "the '$Arg' filename contains invalid characters\n"; $Usage = 2; - last; } if ($Action eq "testexe") { - # The remainder are the executable's arguments - last; + last; # The remainder are the executable's arguments } } + elsif ($Action eq "testpatch" and !defined $Module) + { + $Module = $Arg; + last; # The remainder are the executable's arguments + } else { Error "unexpected argument '$Arg'\n"; @@ -372,7 +405,7 @@ if (defined $Usage) exit $Usage; } print "Usage: $Name0 [--help] --testexe MISSIONS EXE ARGS...\n"; - print "or $Name0 [--help] --testpatch MISSIONS PATCH\n"; + print "or $Name0 [--help] --testpatch MISSIONS PATCH [MODULE ARGS...]\n"; print "or $Name0 [--help] --winetest [--no-submit] MISSIONS BASETAG ARGS\n"; print "\n"; print "Tests the specified patch or runs WineTest in Wine.\n"; @@ -387,11 +420,10 @@ if (defined $Usage) print " - wow32: The 32 bit WoW Wine build.\n"; print " - wow64: The 64 bit WoW Wine build.\n"; print " EXE Is the staging file containing the executable to run.\n"; - print " ARGS Are the arguments for the test executable.\n"; print " PATCH Is the staging file containing the patch to test.\n"; print " BASETAG Is the tag for this WineTest run. Note that the build type is\n"; print " automatically added to this tag.\n"; - print " ARGS The WineTest arguments.\n"; + print " ARGS Arguments for the test.\n"; print " --help Shows this usage message.\n"; exit $Usage; } @@ -413,7 +445,7 @@ unlink map { GetMissionBaseName($_) .".report" } @{$TaskMissions->{Missions}}; my $Impacts; if ($Action eq "testpatch") { - $Impacts = ApplyPatch("wine", $FileName); + $Impacts = ApplyPatch("wine", "$DataDir/staging/$FileName"); exit(1) if (!$Impacts or !BuildWine($TaskMissions, "win32") or !BuildWine($TaskMissions, "wow64") or @@ -421,18 +453,25 @@ if ($Action eq "testpatch") } foreach my $Mission (@{$TaskMissions->{Missions}}) { + return 1 if ($Mission->{test} eq "build"); + if ($Action eq "testexe") { exit(1) if (!TestExe($Mission, $FileName, @ARGV)); } - elsif ($Action eq "testpatch") - { - exit(1) if (!TestPatch($Mission, $Impacts)); - } elsif ($Action eq "winetest") { exit(1) if (!DailyWineTest($Mission, $OptNoSubmit, $BaseTag, @ARGV)); } + elsif (@ARGV) + { + my $FileName = GetExecutable($Mission, $Impacts, $Module); + exit(1) if (!$FileName or !TestExe($Mission, $FileName, @ARGV)); + } + else + { + exit(1) if (!TestPatch($Mission, $Impacts)); + } }
LogMsg "ok\n"; diff --git a/testbot/lib/Build/Utils.pm b/testbot/lib/Build/Utils.pm index 50146301a..15acf6783 100644 --- a/testbot/lib/Build/Utils.pm +++ b/testbot/lib/Build/Utils.pm @@ -30,7 +30,8 @@ our @EXPORT = qw(GetToolName InfoMsg LogMsg Error GitPull ApplyPatch GetCPUCount BuildNativeTestAgentd BuildWindowsTestAgentd GetTestLauncher BuildTestLauncher UpdateAddOns - SetupWineEnvironment RunWine CreateWinePrefix); + SetupWineEnvironment GetWineDir GetTestDir RunWine + CreateWinePrefix);
use Digest::SHA; use File::Basename; @@ -397,29 +398,37 @@ sub SetupWineEnvironment($) return $BaseName; }
+sub GetWineDir($) +{ + my ($Mission) = @_; + return "$DataDir/wine-$Mission->{Build}"; +} + +sub GetTestDir($) +{ + my ($Mission) = @_; + + my $DevDir = "$ENV{WINEPREFIX}/dosdevices"; + return "." if (!-d $DevDir); + + my $Dir = $Mission->{dir} || ""; + # We cannot put colons in missions so restore the drive letter + $Dir = "$DevDir/c:/$Dir" if ($Dir !~ s~^([a-z])/~$DevDir/$1:/~); + return -d $Dir ? $Dir : "$DevDir/c:"; +} + sub RunWine($$$) { my ($Mission, $Cmd, $CmdArgs) = @_;
- my $WineDir = "$DataDir/wine-$Mission->{Build}"; - $Cmd = "$WineDir/$Cmd" if ($Cmd =~ /.exe.so$/); - - my $CurDir = "$ENV{WINEPREFIX}/dosdevices"; - if (!-d $CurDir) - { - $CurDir = "."; - } - else - { - my $Dir = $Mission->{dir} || ""; - # We cannot put colons in missions so restore the drive letter - $Dir = "$CurDir/c:/$Dir" if ($Dir !~ s~^([a-z])/~$CurDir/$1:/~); - $CurDir = -d $Dir ? $Dir : "$ENV{WINEPREFIX}/dosdevices/c:"; - } + my $TestDir = GetTestDir($Mission);
+ my $WineDir = GetWineDir($Mission); + $Cmd = "$WineDir/$Cmd" if ($Cmd =~ /.exe.so$/); my $Magic = `file '$Cmd'`; my $Wine = ($Magic =~ /ELF 64/ ? "$WineDir/wine64" : "$WineDir/wine"); - return system("set -x && cd '$CurDir' && ". + + return system("set -x && cd '$TestDir' && ". "time '$Wine' '$Cmd' $CmdArgs"); }
diff --git a/testbot/lib/WineTestBot/PatchUtils.pm b/testbot/lib/WineTestBot/PatchUtils.pm index 71be9cf00..3c2f67011 100644 --- a/testbot/lib/WineTestBot/PatchUtils.pm +++ b/testbot/lib/WineTestBot/PatchUtils.pm @@ -441,6 +441,7 @@ sub GetPatchImpacts($) 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}) { diff --git a/testbot/web/Submit.pl b/testbot/web/Submit.pl index a51ace650..b67237528 100644 --- a/testbot/web/Submit.pl +++ b/testbot/web/Submit.pl @@ -32,6 +32,7 @@ use POSIX qw(:fcntl_h); # For SEEK_XXX use File::Basename;
use ObjectModel::BasicPropertyDescriptor; +use ObjectModel::EnumPropertyDescriptor; use WineTestBot::Branches; use WineTestBot::Config; use WineTestBot::Engine::Notify; @@ -174,16 +175,10 @@ sub _AnalyzePatch($) $self->{ErrMessage} = "'$self->{FileName}' is not a valid patch"; return undef; } - if ($self->{Impacts}->{TestUnitCount} == 0) + if ($self->{Impacts}->{ModuleUnitCount} == 0) { $self->{ErrField} = "FileName"; - $self->{ErrMessage} = "The patch does not impact the tests"; - return undef; - } - if ($self->{Impacts}->{TestUnitCount} > 1) - { - $self->{ErrField} = "FileName"; - $self->{ErrMessage} = "The patch contains changes to multiple tests"; + $self->{ErrMessage} = "The patch does not directly impact the Wine dlls and programs"; return undef; } return 1; @@ -271,6 +266,65 @@ sub _ValidateVMSelection($;$) return 1; }
+sub _GetTestExecutables($) +{ + my ($self, $ResetInvalid) = @_; + + if (!$self->{TestExecutables}) + { + if ($self->{FileType} eq "patch") + { + foreach my $Module (sort keys %{$self->{Impacts}->{Tests}}) + { + my $TestInfo = $self->{Impacts}->{Tests}->{$Module}; + next if (!%{$TestInfo->{Files}}); + $self->{TestExecutables}->{"$TestInfo->{ExeBase}.exe"} = $Module; + } + } + else + { + $self->{TestExecutables}->{$self->{FileName}} = 1; + } + } +} + +sub _ValidateTestExecutable($;$) +{ + my ($self, $ResetInvalid) = @_; + + $self->_GetTestExecutables(); + if ($ResetInvalid and (!defined $self->{TestExecutable} or + !$self->{TestExecutables}->{$self->{TestExecutable}})) + { + $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]; + } + } + + if (!defined $self->{TestExecutable}) + { + $self->{ErrField} = "TestExecutable"; + $self->{ErrMessage} = "You must specify the test executable filename"; + return undef; + } + elsif (!$self->{TestExecutables}->{$self->{TestExecutable}} or + # Make sure the test executable filename is valid + # to defend against malicious patches. + !IsValidFileName($self->{TestExecutable})) + { + $self->{ErrField} = "TestExecutable"; + $self->{ErrMessage} = "Invalid test executable filename"; + return undef; + } + + return 1; +} + sub Validate($) { my ($self) = @_; @@ -318,14 +372,7 @@ sub Validate($)
if ($self->{Page} >= 3) { - if (($self->{FileType} eq "patch" and - $self->{TestExecutable} !~ m/^[\w_.]+_test.exe$/) or - !IsValidFileName($self->{TestExecutable})) - { - $self->{ErrField} = "TestExecutable"; - $self->{ErrMessage} = "Invalid test executable filename"; - return undef; - } + return undef if (!$self->_ValidateTestExecutable());
if ($self->{CmdLineArg} eq "" and !$self->{NoCmdLineArgWarn}) { @@ -493,6 +540,30 @@ sub GetHeaderText($) } return $HeaderText; } + elsif ($self->{Page} == 3 and $self->{Impacts}) + { + my @TestExecutables; + $self->_GetTestExecutables(); + foreach my $TestExecutable (sort keys %{$self->{TestExecutables}}) + { + my $Module = $self->{TestExecutables}->{$TestExecutable}; + my $TestInfo = $self->{Impacts}->{Tests}->{$Module}; + my @TestUnits; + foreach my $TestUnit (sort keys %{$TestInfo->{AllUnits}}) + { + if ($TestInfo->{PatchedUnits}->{$TestUnit}) + { + push @TestUnits, "<i>$TestUnit</i>"; + } + elsif ($TestInfo->{All} or $TestInfo->{PatchedModule}) + { + push @TestUnits, $TestUnit; + } + } + push @TestExecutables, "$Module: ". join(" ", @TestUnits) if (@TestUnits); + } + return join("<br>\n", "Here is a list of the test executables impacted by the patch (patched test units, if any, are in italics).", @TestExecutables); + }
return ""; } @@ -512,15 +583,18 @@ sub GetPropertyDescriptors($) } elsif ($self->{Page} == 3) { - $self->_GetFileType(); - my $IsPatch = ($self->{FileType} eq "patch"); - $self->{PropertyDescriptors} ||= [ - CreateBasicPropertyDescriptor("TestExecutable", "Test executable", !1, $IsPatch, "A", 50), - CreateBasicPropertyDescriptor("CmdLineArg", "Command line arguments", !1, !1, "A", 50), - CreateBasicPropertyDescriptor("Run64", "Run 64-bit tests in addition to 32-bit tests", !1, 1, "B", 1), - CreateBasicPropertyDescriptor("DebugLevel", "Debug level (WINETEST_DEBUG)", !1, 1, "N", 2), - CreateBasicPropertyDescriptor("ReportSuccessfulTests", "Report successful tests (WINETEST_REPORT_SUCCESS)", !1, 1, "B", 1), - ]; + if (!$self->{PropertyDescriptors}) + { + $self->_GetTestExecutables(); + my @TestExecutables = sort keys %{$self->{TestExecutables}}; + $self->{PropertyDescriptors} = [ + CreateEnumPropertyDescriptor("TestExecutable", "Test executable", !1, 1, @TestExecutables), + CreateBasicPropertyDescriptor("CmdLineArg", "Command line arguments", !1, !1, "A", 50), + CreateBasicPropertyDescriptor("Run64", "Run 64-bit tests in addition to 32-bit tests", !1, 1, "B", 1), + CreateBasicPropertyDescriptor("DebugLevel", "Debug level (WINETEST_DEBUG)", !1, 1, "N", 2), + CreateBasicPropertyDescriptor("ReportSuccessfulTests", "Report successful tests (WINETEST_REPORT_SUCCESS)", !1, 1, "B", 1), + ]; + } } return $self->SUPER::GetPropertyDescriptors(); } @@ -601,7 +675,9 @@ sub GenerateFields($)
# By default select the base VMs that are ready to run tasks if (!$self->{UserVMSelection} and !$VMRow->{Extra} and - $VMRow->{VM}->Status !~ /^(?:offline|maintenance)$/) + $VMRow->{VM}->Status !~ /^(?:offline|maintenance)$/ and + ($VMRow->{VM}->Type eq "wine" or !$self->{Impacts} or + $self->{Impacts}->{TestUnitCount})) { $VMRow->{Checked} = 1; } @@ -764,8 +840,6 @@ sub OnUnset($) unlink($StagingFilePath) if ($StagingFilePath); delete $self->{FileName}; } - delete $self->{TestExecutable}; - delete $self->{CmdLineArg};
return 1; } @@ -783,29 +857,13 @@ sub OnPage1Next($) $self->{FileName} = $self->GetParam("Upload"); return undef if (!$self->_ValidateFileName() or !$self->_Upload()); } - if (!$self->Validate() or !$self->_ValidateVMSelection("deselect")) + if (!$self->Validate() or !$self->_ValidateVMSelection("deselect") or + !$self->_ValidateTestExecutable("reset")) { return undef; }
# Set defaults - if ((!defined $self->{TestExecutable} or !defined $self->{CmdLineArg}) and - $self->{Impacts}) - { - foreach my $TestInfo (values %{$self->{Impacts}->{Tests}}) - { - next if (!$TestInfo->{UnitCount}); - if (!defined $self->{TestExecutable}) - { - $self->{TestExecutable} = "$TestInfo->{ExeBase}.exe"; - } - if (!defined $self->{CmdLineArg}) - { - $self->{CmdLineArg} = (keys %{$TestInfo->{PatchedUnits}})[0]; - } - last; - } - } if (!defined $self->{Run64}) { # Whether we show it or not the default is true @@ -997,7 +1055,8 @@ sub _SubmitJob($$) $Task->VM($VM); $Task->Timeout($Timeout); $Task->Missions($MissionStatement); - $Task->CmdLineArg($self->{CmdLineArg}) if ($self->{FileType} ne "patch"); + my $Module = $self->{TestExecutables}->{$self->{TestExecutable}}; + $Task->CmdLineArg(($Module eq 1 ? "" : "$Module ") .$self->{CmdLineArg}); }
# Now save it all (or whatever's left to save)
Signed-off-by: Francois Gouget fgouget@codeweavers.com --- testbot/web/Submit.pl | 131 ++++++++++++++++++++++++++++++------------ 1 file changed, 95 insertions(+), 36 deletions(-)
diff --git a/testbot/web/Submit.pl b/testbot/web/Submit.pl index b67237528..dc3fdca3d 100644 --- a/testbot/web/Submit.pl +++ b/testbot/web/Submit.pl @@ -325,6 +325,22 @@ sub _ValidateTestExecutable($;$) return 1; }
+sub _ValidateUpload($) +{ + my ($self) = @_; + + if (defined $self->{FileName}) + { + return undef if (!$self->_ValidateFileName()); + } + else + { + $self->{FileName} = $self->GetParam("Upload"); + return undef if (!$self->_ValidateFileName() or !$self->_Upload()); + } + return 1; +} + sub Validate($) { my ($self) = @_; @@ -410,7 +426,7 @@ sub _initialize($$$)
$self->{Page} = $self->GetParam("Page"); # Page is a hidden parameter so fix it instead of issuing an error - $self->{Page} = 1 if (!defined $self->{Page} or $self->{Page} !~ /^[0-3]$/); + $self->{Page} = 1 if (!defined $self->{Page} or $self->{Page} !~ /^[0-4]$/); $self->{LastPage} = $self->{Page};
# Load the Page 1 parameters @@ -468,6 +484,9 @@ sub _GenerateStateFields($)
$self->_GenerateStateField("Page");
+ # There are no parameters to preserve when there is a single page + return if ($self->{Page} == 4); + if ($self->{Page} != 1) { $self->_GenerateStateField("Branch"); @@ -514,22 +533,24 @@ sub GetHeaderText($) { my ($self) = @_;
+ my @Headers; if ($self->{Page} == 0) { - return "Your job was successfully queued, but the job engine that takes " . - "care of actually running it seems to be unavailable (perhaps it " . - "crashed). Your job will remain queued until the engine is " . - "restarted."; + push @Headers, + "Your job was successfully queued, but the job engine that takes " . + "care of actually running it seems to be unavailable (perhaps it " . + "crashed). Your job will remain queued until the engine is " . + "restarted."; } - elsif ($self->{Page} == 1) + if ($self->{Page} == 1 or $self->{Page} == 4) { - return defined $self->{FileName} ? "" : + push @Headers, defined $self->{FileName} ? "" : "Specify the patch file that you want to upload and submit " . "for testing.<br>\n" . "You can also specify a Windows .exe file, this would normally be " . "a Wine test executable that you cross-compiled." } - elsif ($self->{Page} == 2) + if ($self->{Page} == 2 or $self->{Page} == 4) { my $HeaderText = "Select the VMs on which you want to run your test."; my $VMs = CreateVMs(); @@ -538,9 +559,9 @@ sub GetHeaderText($) { $HeaderText .= "<br>NOTE: Offline VMs and those undergoing maintenance will not be able to run your tests right away."; } - return $HeaderText; + push @Headers, $HeaderText; } - elsif ($self->{Page} == 3 and $self->{Impacts}) + if (($self->{Page} == 3 or $self->{Page} == 0) and $self->{Impacts}) { my @TestExecutables; $self->_GetTestExecutables(); @@ -562,10 +583,10 @@ sub GetHeaderText($) } push @TestExecutables, "$Module: ". join(" ", @TestUnits) if (@TestUnits); } - return join("<br>\n", "Here is a list of the test executables impacted by the patch (patched test units, if any, are in italics).", @TestExecutables); + 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); }
- return ""; + return join("<br>\n", @Headers); }
sub GetPropertyDescriptors($) @@ -581,21 +602,33 @@ sub GetPropertyDescriptors($) CreateBasicPropertyDescriptor("Remarks", "Remarks", !1, !1, "A", 128), ]; } - elsif ($self->{Page} == 3) + elsif (($self->{Page} == 3 or $self->{Page} == 4) and + !$self->{PropertyDescriptors}) { - if (!$self->{PropertyDescriptors}) + if ($self->{Page} == 4) + { + $self->{PropertyDescriptors} = [ + CreateBasicPropertyDescriptor("Remarks", "Remarks", !1, !1, "A", 128), + # Let the user type in the TestExecutable since we won't be able to + # figure it out in advance + CreateBasicPropertyDescriptor("TestExecutable", "Test executable", !1, 1, "A", 50), + ]; + } + else { $self->_GetTestExecutables(); my @TestExecutables = sort keys %{$self->{TestExecutables}}; $self->{PropertyDescriptors} = [ CreateEnumPropertyDescriptor("TestExecutable", "Test executable", !1, 1, @TestExecutables), - CreateBasicPropertyDescriptor("CmdLineArg", "Command line arguments", !1, !1, "A", 50), - CreateBasicPropertyDescriptor("Run64", "Run 64-bit tests in addition to 32-bit tests", !1, 1, "B", 1), - CreateBasicPropertyDescriptor("DebugLevel", "Debug level (WINETEST_DEBUG)", !1, 1, "N", 2), - CreateBasicPropertyDescriptor("ReportSuccessfulTests", "Report successful tests (WINETEST_REPORT_SUCCESS)", !1, 1, "B", 1), ]; } + push @{$self->{PropertyDescriptors}}, + CreateBasicPropertyDescriptor("CmdLineArg", "Command line arguments", !1, !1, "A", 50), + CreateBasicPropertyDescriptor("Run64", "Run 64-bit tests in addition to 32-bit tests", !1, 1, "B", 1), + CreateBasicPropertyDescriptor("DebugLevel", "Debug level (WINETEST_DEBUG)", !1, 1, "N", 2), + CreateBasicPropertyDescriptor("ReportSuccessfulTests", "Report successful tests (WINETEST_REPORT_SUCCESS)", !1, 1, "B", 1); } + return $self->SUPER::GetPropertyDescriptors(); }
@@ -611,7 +644,7 @@ sub GenerateFields($) $self->_GenerateStateField("JobKey"); }
- elsif ($self->{Page} == 1) + if ($self->{Page} == 1 or $self->{Page} == 4) { print "<div class='ItemProperty'><label>File</label>", "<div class='ItemValue'>"; @@ -657,9 +690,11 @@ sub GenerateFields($) } # The other fields are taken care of by FreeFormPage. $self->{HasRequired} = 1; + + print "<br>\n" if ($self->{Page} == 4); }
- elsif ($self->{Page} == 2) + if ($self->{Page} == 2 or $self->{Page} == 4) { $self->_GenerateStateField("ShowAll"); print "<div class='CollectionBlock'><table>\n"; @@ -742,15 +777,23 @@ EOF print "<input type='submit' name='Action' value='", $self->{ShowAll} ? "Show base VMs" : "Show all VMs", "'/>\n"; print "</div>\n"; + + print "<br>\n" if ($self->{Page} == 4); }
- elsif ($self->{Page} == 3) + if ($self->{Page} == 3 or $self->{Page} == 4) { $self->_GenerateStateField("NoCmdLineArgWarn");
# Preserve these fields if they are not shown - $self->_GenerateStateField("Run64") if (!$self->{ShowRun64} or $self->{FileType} ne "patch"); - $self->_GenerateStateField("TestExecutable") if ($self->{FileType} ne "patch"); + if (!$self->{ShowRun64} or $self->{FileType} ne "patch") + { + $self->_GenerateStateField("Run64"); + } + if ($self->{FileType} ne "patch" and $self->{Page} != 4) + { + $self->_GenerateStateField("TestExecutable"); + }
# The other fields are taken care of by FreeFormPage. } @@ -769,7 +812,7 @@ sub GetActions($) } elsif ($self->{Page} == 1) { - push @$Actions, "Next >"; + push @$Actions, "Single Page", "Next >"; } elsif ($self->{Page} == 2) { @@ -779,6 +822,10 @@ sub GetActions($) { push @$Actions, "< Prev", "Submit"; } + elsif ($self->{Page} == 4) + { + push @$Actions, "Submit"; + }
return $Actions; } @@ -848,16 +895,8 @@ sub OnPage1Next($) { my ($self) = @_;
- if (defined $self->{FileName}) - { - return undef if (!$self->_ValidateFileName()); - } - else - { - $self->{FileName} = $self->GetParam("Upload"); - return undef if (!$self->_ValidateFileName() or !$self->_Upload()); - } - if (!$self->Validate() or !$self->_ValidateVMSelection("deselect") or + if (!$self->_ValidateUpload() or !$self->Validate() or + !$self->_ValidateVMSelection("deselect") or !$self->_ValidateTestExecutable("reset")) { return undef; @@ -874,6 +913,22 @@ sub OnPage1Next($) return 1; }
+sub OnSinglePage($) +{ + my ($self) = @_; + + # The checks and defaults are the same as for Page1Next() + if (!$self->OnPage1Next()) + { + # But ignore all errors. The user can change everything anyway. + delete $self->{ErrField}; + delete $self->{ErrMessage}; + } + + $self->_SetPage(4); + return 1; +} + sub OnPage2Next($) { my ($self) = @_; @@ -1101,7 +1156,7 @@ sub OnSubmit($) { my ($self) = @_;
- return undef if (!$self->Validate()); + return undef if (!$self->_ValidateUpload() or !$self->Validate());
# Rename the staging file to avoid race conditions if the user clicks on # Submit multiple times @@ -1165,7 +1220,11 @@ sub OnAction($$) { my ($self, $Action) = @_;
- if ($Action eq "Next >") + if ($Action eq "Single Page") + { + return $self->OnSinglePage(); + } + elsif ($Action eq "Next >") { return $self->{Page} == 2 ? $self->OnPage2Next() : $self->OnPage1Next(); }
-
Francois Gouget