[tools] testbot/web: Use AddError() and ResetErrors() on the submit job page.
Francois Gouget
fgouget at codeweavers.com
Thu Jul 7 08:13:22 CDT 2022
This replaces a lot of direct field accesses.
Also clearly identify unused variables.
Signed-off-by: Francois Gouget <fgouget at codeweavers.com>
---
testbot/web/Submit.pl | 100 ++++++++++++++++--------------------------
1 file changed, 37 insertions(+), 63 deletions(-)
diff --git a/testbot/web/Submit.pl b/testbot/web/Submit.pl
index c5423c7951..4368aeeb30 100644
--- a/testbot/web/Submit.pl
+++ b/testbot/web/Submit.pl
@@ -77,16 +77,14 @@ sub _Upload($)
{
umask($OldUMask);
delete $self->{FileName};
- $self->{ErrField} = "File";
- $self->{ErrMessage} = "Unable to save the uploaded file";
+ $self->AddError("Unable to save the uploaded file", "File");
return undef;
}
}
else
{
delete $self->{FileName};
- $self->{ErrField} = "File";
- $self->{ErrMessage} = "Unable to upload the file";
+ $self->AddError("Unable to upload the file", "File");
return undef;
}
return 1;
@@ -102,8 +100,7 @@ sub _GetFileType($)
my $Fh;
if (!sysopen($Fh, $self->_GetStagingFilePath(), O_RDONLY))
{
- $self->{ErrField} = "FileName";
- $self->{ErrMessage} = "Unable to open '$self->{FileName}'";
+ $self->AddError("Unable to open '$self->{FileName}'", "FileName");
return undef;
}
@@ -153,8 +150,7 @@ sub _GetFileType($)
if ($self->{FileType} !~ /^(?:exe32|exe64|patch)$/)
{
- $self->{ErrField} = "FileName";
- $self->{ErrMessage} = "Unsupported file type";
+ $self->SerError("FileName", "Unsupported file type");
return undef;
}
@@ -169,14 +165,12 @@ sub _AnalyzePatch($)
if (!$self->{Impacts}->{TestBuild})
{
- $self->{ErrField} = "FileName";
- $self->{ErrMessage} = "'$self->{FileName}' is not a valid patch";
+ $self->AddError("'$self->{FileName}' is not a valid patch", "FileName");
return undef;
}
if ($self->{Impacts}->{PatchedModuleOrTests} == 0)
{
- $self->{ErrField} = "FileName";
- $self->{ErrMessage} = "The patch does not directly impact the Wine dlls and programs";
+ $self->AddError("The patch does not directly impact the Wine dlls and programs", "FileName");
return undef;
}
return 1;
@@ -194,23 +188,20 @@ sub _ValidateFileName($)
if (!defined $self->{FileName})
{
delete $self->{FileName};
- $self->{ErrField} = "FileName";
- $self->{ErrMessage} = "You must provide a file to test";
+ $self->AddError("You must provide a file to test", "FileName");
return undef;
}
if (!IsValidFileName($self->{FileName}))
{
delete $self->{FileName};
- $self->{ErrField} = "FileName";
- $self->{ErrMessage} = "The filename contains invalid characters";
+ $self->AddError("The filename contains invalid characters", "FileName");
return undef;
}
my $PropertyDescriptor = CreateSteps()->GetPropertyDescriptorByName("FileName");
if ($PropertyDescriptor->GetMaxLength() - 32 - 1 < length($self->{FileName}))
{
delete $self->{FileName};
- $self->{ErrField} = "FileName";
- $self->{ErrMessage} = "The filename is too long";
+ $self->AddError("The filename is too long", "FileName");
return undef;
}
@@ -290,7 +281,7 @@ sub _ValidateVMSelection($;$)
if (@Deselected)
{
- $self->{ErrMessage} = "The following VMs are incompatible and have been deselected: @Deselected";
+ $self->AddError("The following VMs are incompatible and have been deselected: @Deselected");
return undef;
}
return 1;
@@ -337,8 +328,7 @@ sub _ValidateTestExecutable($;$)
if (!defined $self->{TestExecutable})
{
- $self->{ErrField} = "TestExecutable";
- $self->{ErrMessage} = "You must specify the test executable filename";
+ $self->AddError("You must specify the test executable filename", "TestExecutable");
return undef;
}
elsif (!$self->{TestExecutables}->{$self->{TestExecutable}} or
@@ -346,8 +336,7 @@ sub _ValidateTestExecutable($;$)
# to defend against malicious patches.
!IsValidFileName($self->{TestExecutable}))
{
- $self->{ErrField} = "TestExecutable";
- $self->{ErrMessage} = "Invalid test executable filename";
+ $self->AddError("Invalid test executable filename", "TestExecutable");
return undef;
}
@@ -393,14 +382,12 @@ sub Validate($)
{
# Note that Page 1 should have provided a default
# so there is no reason for it to still be undefined.
- $self->{ErrField} = "Branch";
- $self->{ErrMessage} = "You must specify the branch to test";
+ $self->AddError("You must specify the branch to test", "Branch");
return undef;
}
if (!CreateBranches()->GetItem($self->{Branch}))
{
- $self->{ErrField} = "Branch";
- $self->{ErrMessage} = "The '$self->{Branch}' branch does not exist";
+ $self->AddError("The '$self->{Branch}' branch does not exist", "Branch");
return undef;
}
}
@@ -410,7 +397,7 @@ sub Validate($)
return undef if (!$self->_ValidateVMSelection());
if (!$self->{CheckedVMCount})
{
- $self->{ErrMessage} = "Select at least one VM";
+ $self->AddError("Select at least one VM");
return undef;
}
}
@@ -424,18 +411,17 @@ sub Validate($)
!$self->{Run64} and $self->{ShowRun64})
{
# Don't set ErrField since Run32 and Run64 may be hidden
- $self->{ErrMessage} = "You must at least pick one of the 32 or 64 bit tests to run on the selected Windows VMs.";
+ $self->AddError("You must at least pick one of the 32 or 64 bit tests to run on the selected Windows VMs.");
return undef;
}
if ($self->{CmdLineArg} eq "" and !$self->{NoCmdLineArgWarn})
{
- $self->{ErrMessage} = "You did not specify a command line argument. ".
- "This is most likely not correct, so please ".
- "fix this. If you are sure that you really don't ".
- 'want a command line argument, press "Submit" '.
- "again.";
- $self->{ErrField} = "CmdLineArg";
+ $self->AddError("You did not specify a command line argument. ".
+ "This is most likely not correct, so please ".
+ "fix this. If you are sure that you really don't ".
+ 'want a command line argument, press "Submit" again.',
+ "CmdLineArg");
$self->{NoCmdLineArgWarn} = 1;
return undef;
}
@@ -969,8 +955,7 @@ sub OnUnset($)
if (!$self->_ValidateFileName())
{
# Ignore the error. What counts is not using a suspicious FileName.
- delete $self->{ErrField};
- delete $self->{ErrMessage};
+ $self->ResetErrors();
}
elsif (defined $self->{FileName})
{
@@ -1012,12 +997,8 @@ 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};
- }
+ # But ignore all errors. The user can change everything anyway.
+ $self->ResetErrors() if (!$self->OnPage1Next());
$self->_SetPage(4);
return 1;
@@ -1151,7 +1132,7 @@ sub _SubmitJob($$)
}
if (defined $ErrMessage)
{
- $self->{ErrMessage} = $ErrMessage;
+ $self->AddError($ErrMessage);
return !1;
}
}
@@ -1210,7 +1191,7 @@ sub _SubmitJob($$)
my ($ErrMessage, $Missions) = ParseMissionStatement($WineMissions{$VM});
if (defined $ErrMessage)
{
- $self->{ErrMessage} = $ErrMessage;
+ $self->AddError($ErrMessage);
return !1;
}
my $Missions = $Missions->[0];
@@ -1229,34 +1210,32 @@ sub _SubmitJob($$)
}
# Now save it all (or whatever's left to save)
- my ($ErrKey, $ErrProperty, $ErrMessage) = $Jobs->Save();
+ my ($_ErrKey, $_ErrProperty, $ErrMessage) = $Jobs->Save();
if (defined $ErrMessage)
{
- $self->{ErrMessage} = $ErrMessage;
+ $self->AddError($ErrMessage);
return undef;
}
# Stage the test patch/executable so the job can pick it up
if (!rename($Staging, "$DataDir/staging/job". $NewJob->Id ."_$self->{FileName}"))
{
- $self->{ErrMessage} = "Could not stage '$self->{FileName}': $!\n";
+ $self->AddError("Could not stage '$self->{FileName}': $!");
return undef;
}
# Switch Status to staging to indicate we are done setting up the job
$NewJob->Status("staging");
- ($ErrKey, $ErrProperty, $ErrMessage) = $Jobs->Save();
+ ($_ErrKey, $_ErrProperty, $ErrMessage) = $Jobs->Save();
if (defined $ErrMessage)
{
- $self->{ErrMessage} = $ErrMessage;
+ $self->AddError($ErrMessage);
return undef;
}
# Notify engine
- my $ErrMessage = RescheduleJobs();
- if (defined $ErrMessage)
+ if ($self->AddError(RescheduleJobs()))
{
- $self->{ErrMessage} = $ErrMessage;
$self->_SetPage(0);
$self->{JobKey} = $NewJob->GetKey();
return undef;
@@ -1277,15 +1256,15 @@ sub OnSubmit($)
my $Staging = CreateNewLink($OldStaging, "$DataDir/staging", "-websubmit2_$self->{FileName}");
if (!defined $Staging)
{
- $self->{ErrMessage} = "Could not rename '$self->{FileName}': $!";
+ $self->AddError("Could not rename '$self->{FileName}': $!");
return undef;
}
if (!unlink $OldStaging)
{
unlink $Staging;
- $self->{ErrMessage} = $!{ENOENT} ?
+ $self->AddError($!{ENOENT} ?
"$self->{FileName} has already been submitted or has expired" :
- "Could not remove the staging '$self->{FileName}' file: $!";
+ "Could not remove the staging '$self->{FileName}' file: $!");
return undef;
}
@@ -1303,13 +1282,8 @@ sub OnSetShowAllVMs($$)
my ($self, $Value) = @_;
# Call _ValidateVMSelection() to identify incompatible VMs so they are not
- # marked as Checked
- if (!$self->_ValidateVMSelection("deselect"))
- {
- # Ignore errors
- delete $self->{ErrField};
- delete $self->{ErrMessage};
- }
+ # marked as Checked but ignore errors.
+ $self->ResetErrors() if (!$self->_ValidateVMSelection("deselect"));
$self->{ShowAll} = $Value;
return undef;
--
2.30.2
More information about the wine-devel
mailing list