Bug 31829 - In run-webkit-tests $failureCount shouldn't include new tests
Summary: In run-webkit-tests $failureCount shouldn't include new tests
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
: 39012 (view as bug list)
Depends on:
Blocks:
 
Reported: 2009-11-24 06:02 PST by Csaba Osztrogonác
Modified: 2010-05-12 13:18 PDT (History)
7 users (show)

See Also:


Attachments
proposed patch (1.48 KB, patch)
2009-11-24 06:04 PST, Csaba Osztrogonác
eric: review+
Details | Formatted Diff | Diff
updated patch (3.65 KB, patch)
2009-11-25 12:54 PST, Csaba Osztrogonác
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Csaba Osztrogonác 2009-11-24 06:02:12 PST
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.
Comment 1 Csaba Osztrogonác 2009-11-24 06:04:56 PST
Created attachment 43765 [details]
proposed patch
Comment 2 Eric Seidel (no email) 2009-11-24 08:32:12 PST
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.
Comment 3 Csaba Osztrogonác 2009-11-24 08:48:24 PST
(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.
Comment 4 Eric Seidel (no email) 2009-11-24 09:00:07 PST
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).
Comment 5 Csaba Osztrogonác 2009-11-24 09:16:17 PST
(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.
Comment 6 Eric Seidel (no email) 2009-11-24 09:23:21 PST
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). :)
Comment 7 Csaba Osztrogonác 2009-11-25 12:54:33 PST
Created attachment 43864 [details]
updated patch
Comment 8 Csaba Osztrogonác 2009-11-25 13:15:14 PST
(In reply to comment #7)
> Created an attachment (id=43864) [details]
> updated patch

sorry, wrong bug :)
Comment 9 Eric Seidel (no email) 2010-05-12 12:52:59 PDT
Comment on attachment 43765 [details]
proposed patch

OK.
Comment 10 Csaba Osztrogonác 2010-05-12 13:00:37 PDT
*** Bug 39012 has been marked as a duplicate of this bug. ***
Comment 11 Csaba Osztrogonác 2010-05-12 13:18:28 PDT
(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