Bug 31829 – In run-webkit-tests $failureCount shouldn't include new tests

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Csaba Osztrogonác]

If you execute run-webkit-tests with --exitAfterNFailures NUM, 
the script will exit after NUM failed tests. I think, the 
$failureCount shouldn't include new tests.

[Csaba Osztrogonác]

Created attachment 43765 [details]
proposed patch

[Eric Seidel (no email)]

Can you explain the use case?  The bots want this behavior since new tests will still cause run-webkit-tests to exit(1) in the end, even if you change it to not exit early here.

[Csaba Osztrogonác]

(In reply to comment #2)
> Can you explain the use case?  The bots want this behavior since new tests will
> still cause run-webkit-tests to exit(1) in the end, even if you change it to
> not exit early here.

If there aren't failing and crashing tests but some new, the buildbot will be
henceforward green.(run-webkit-tests case - orange) In my opinion, only new 
tests don't explain stop run-webkit-tests on buildbot ahead of time. Because stopping buildbot earlier can hide latter failing or crashing tests.

[Eric Seidel (no email)]

I think this is papering over the symptom instead of solving the problem.  The Qt bot shouldn't have missing test results in the first place. :(

In the case of the commit-queue we would prefer to exit after the first test requiring new results or the first failure.  If that has to be refactored into separate options to work with the Qt buildbot that's OK, but as this is this would cause the commit-queue to fail slower (not a huge issue, but certainly one worth mentioning).

[Csaba Osztrogonác]

(In reply to comment #4)
> I think this is papering over the symptom instead of solving the problem.  The
> Qt bot shouldn't have missing test results in the first place. :(
Yes, you're right, our port shouldn't have so many missing test results.
Unfortunately we can't keep step with new tests. We regularly put these
tests into skiplist when we haven't any time to make expected files. 
 
> In the case of the commit-queue we would prefer to exit after the first test
> requiring new results or the first failure.  If that has to be refactored into
> separate options to work with the Qt buildbot that's OK, but as this is this
> would cause the commit-queue to fail slower (not a huge issue, but certainly
> one worth mentioning).
Refactoring into separate options is a good idea, I will try to do it.

[Eric Seidel (no email)]

Can't we just fix the Qt bot having so many missing tests?  We don't generally have 20 tests added per day.  Chromium has a rebaselining tool, which knows how to suck results off of a buildbot output, which is used in chromium land to suck results off of the try bot results, and thus provide patch-writters with complete results for all platforms.  We could build something similar in WebKit land, if only for Qt. :)

If folks aren't looking at the bots enough now so as to keep them from going red, I'm not sure why preventing them from displaying less data after 20 failures, vs. 40 failures matters.  And I'm not sure why new tests being "false failures" matters either.  I think we're still trying to solve the wrong problems with this bug (and the 20 -> 40 change you filed). :)

[Csaba Osztrogonác]

Created attachment 43864 [details]
updated patch

[Csaba Osztrogonác]

(In reply to comment #7)
> Created an attachment (id=43864) [details]
> updated patch

sorry, wrong bug :)

[Eric Seidel (no email)]

Comment on attachment 43765 [details]
proposed patch

OK.

[Csaba Osztrogonác]

*** Bug 39012 has been marked as a duplicate of this bug. ***

[Csaba Osztrogonác]

(In reply to comment #9)
> (From update of attachment 43765 [details])
> OK.

Landed: http://trac.webkit.org/changeset/59254
and my fault fixed here: http://trac.webkit.org/changeset/59255