Bug 28192

Summary: run-webkit-tests needs a --exit-after-failures=N option
Product: WebKit Reporter: Eric Seidel (no email) <eric>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
Patch v1
none
Removed extra change
none
now with misspelling fixed mrowe: review+

Description Eric Seidel (no email) 2009-08-11 15:15:45 PDT
I'm not sure --fail-fast is the right name.

But I want a mode where run-webkit-tests exits after the first failure or new result.  This is useful for bugzilla-tool (or possibly some bots) when validating that everything still passes before committing.  In some cases we don't care to know all of the failures, only that something failed (and want to know as fast as possible).
Comment 1 Mark Rowe (bdash) 2009-08-11 15:23:13 PDT
The build bots would be better served by a mode that gave up after a certain number of crashes or failures.  Perhaps a --exit-after-n-failures flag could fill both of these needs?
Comment 2 Eric Seidel (no email) 2009-08-11 15:27:29 PDT
Excellent idea!  (and that's a much better name)
Comment 3 Eric Seidel (no email) 2009-08-12 16:41:42 PDT
Created attachment 34704 [details]
Patch v1
Comment 4 Darin Adler 2009-08-12 16:44:02 PDT
Comment on attachment 34704 [details]
Patch v1

> +    my $testsWhithDidNotMatch = $count - $counts{match};

Which?
Comment 5 Eric Seidel (no email) 2009-08-12 16:44:08 PDT
Created attachment 34705 [details]
Removed extra change
Comment 6 Eric Seidel (no email) 2009-08-12 16:45:19 PDT
Created attachment 34706 [details]
now with misspelling fixed
Comment 7 Mark Rowe (bdash) 2009-08-12 16:54:32 PDT
Comment on attachment 34706 [details]
now with misspelling fixed

> diff --git a/WebKitTools/Scripts/run-webkit-tests b/WebKitTools/Scripts/run-webkit-tests
> index 1f908ec..003ac8d 100755
> --- a/WebKitTools/Scripts/run-webkit-tests
> +++ b/WebKitTools/Scripts/run-webkit-tests
> @@ -109,6 +109,7 @@ sub writeToFile($$);
>  # Argument handling
>  my $addPlatformExceptions = 0;
>  my $complexText = 0;
> +my $exitAfterNFailures = 0;
>  my $guardMalloc = '';
>  my $httpdPort = 8000;
>  my $httpdSSLPort = 8443;
> @@ -193,6 +194,7 @@ Usage: $programName [options] [testdir|testpath ...]
>    --complex-text                  Use the complex text code path for all text (Mac OS X and Windows only)
>    -c|--configuration config       Set DumpRenderTree build configuration
>    -g|--guard-malloc               Enable malloc guard
> +  --exit-after-n-failures         Exit after the first N failures instead of running all the tests

The help for this flag should indicate that it takes an argument.  Is there any reason to put it at this point in the help?  Are they in any specific order?

>      countFinishedTest($test, $base, $result, $isText);
> +
> +    my $testsWhichDidNotMatch = $count - $counts{match};
> +    if ($exitAfterNFailures && $testsWhichDidNotMatch >= $exitAfterNFailures) {
> +        print "\nExiting early after $testsWhichDidNotMatch failures.  $count tests run.";
> +        closeDumpTool();
> +        last;
> +    }

$testsWhichDidNotMatch sounds like an array of tests that didn't match.  Perhaps $failureCount would be a clearer variable name?

r=me
Comment 8 Eric Seidel (no email) 2009-08-12 16:55:42 PDT
(In reply to comment #7)
> (From update of attachment 34706 [details])
> > diff --git a/WebKitTools/Scripts/run-webkit-tests b/WebKitTools/Scripts/run-webkit-tests
> > index 1f908ec..003ac8d 100755
> > --- a/WebKitTools/Scripts/run-webkit-tests
> > +++ b/WebKitTools/Scripts/run-webkit-tests
> > @@ -109,6 +109,7 @@ sub writeToFile($$);
> >  # Argument handling
> >  my $addPlatformExceptions = 0;
> >  my $complexText = 0;
> > +my $exitAfterNFailures = 0;
> >  my $guardMalloc = '';
> >  my $httpdPort = 8000;
> >  my $httpdSSLPort = 8443;
> > @@ -193,6 +194,7 @@ Usage: $programName [options] [testdir|testpath ...]
> >    --complex-text                  Use the complex text code path for all text (Mac OS X and Windows only)
> >    -c|--configuration config       Set DumpRenderTree build configuration
> >    -g|--guard-malloc               Enable malloc guard
> > +  --exit-after-n-failures         Exit after the first N failures instead of running all the tests
> 
> The help for this flag should indicate that it takes an argument.  Is there any
> reason to put it at this point in the help?  Are they in any specific order?

It seemed they were in alphabetical order.  But I'm happy to put it wherever.

> >      countFinishedTest($test, $base, $result, $isText);
> > +
> > +    my $testsWhichDidNotMatch = $count - $counts{match};
> > +    if ($exitAfterNFailures && $testsWhichDidNotMatch >= $exitAfterNFailures) {
> > +        print "\nExiting early after $testsWhichDidNotMatch failures.  $count tests run.";
> > +        closeDumpTool();
> > +        last;
> > +    }
> 
> $testsWhichDidNotMatch sounds like an array of tests that didn't match. 
> Perhaps $failureCount would be a clearer variable name?

Sounds fine to me.  Will change.
Comment 9 Eric Seidel (no email) 2009-08-12 17:03:58 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebKitTools/ChangeLog
	M	WebKitTools/Scripts/bugzilla-tool
	M	WebKitTools/Scripts/run-webkit-tests
Committed r47166
	M	WebKitTools/ChangeLog
	M	WebKitTools/Scripts/bugzilla-tool
	M	WebKitTools/Scripts/run-webkit-tests
r47166 = 581871acba07344f98ea6b67c0be8ed60106d917 (trunk)
No changes between current HEAD and refs/remotes/trunk
Resetting to the latest refs/remotes/trunk
http://trac.webkit.org/changeset/47166