Bug 59279

Summary: The bots should learn from expected failures without having to retry
Product: WebKit Reporter: Adam Barth <abarth>
Component: New BugsAssignee: Adam Barth <abarth>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, eric
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Attachments:
Description Flags
Patch
none
now with test case
none
Updated with more tests
none
Patch for landing eric: commit-queue+

Description Adam Barth 2011-04-23 09:31:17 PDT
The bots should learn from expected failures without having to retry
Comment 1 Adam Barth 2011-04-23 09:33:06 PDT
Created attachment 90853 [details]
Patch
Comment 2 Eric Seidel (no email) 2011-04-23 09:35:44 PDT
Comment on attachment 90853 [details]
Patch

Cute.  But we need a test.
Comment 3 Adam Barth 2011-04-23 11:00:07 PDT
> Cute.  But we need a test.

Do you think we should report failure at the end?
Comment 4 Eric Seidel (no email) 2011-04-23 11:35:28 PDT
Comment on attachment 90853 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=90853&action=review

I'm not sure what you mean.

> Tools/Scripts/webkitpy/tool/bot/patchanalysistask.py:217
> +        if self._expected_failures.failures_were_expected(first_results):

This also could be complicated by flaky tests.  We won't get to here in the flaky test case (we would have stopped to report them already).
Comment 5 Adam Barth 2011-04-23 11:44:46 PDT
(In reply to comment #4)
> (From update of attachment 90853 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=90853&action=review
> 
> I'm not sure what you mean.

In the line where I added a FIXME, we could report failure instead of returning False (which causes a retry).

> > Tools/Scripts/webkitpy/tool/bot/patchanalysistask.py:217
> > +        if self._expected_failures.failures_were_expected(first_results):
> 
> This also could be complicated by flaky tests.  We won't get to here in the flaky test case (we would have stopped to report them already).

The only way flaky tests cause problem here is if we think one test failed turing the clean run when it should have passed.  The result of that is only slightly less test coverage.
Comment 6 Ojan Vafai 2011-04-26 17:02:42 PDT
Comment on attachment 90853 [details]
Patch

R- per lack of test
Comment 7 Eric Seidel (no email) 2011-05-01 22:55:18 PDT
Comment on attachment 90853 [details]
Patch

This should be easy to test. :)
Comment 8 Eric Seidel (no email) 2011-05-01 23:28:24 PDT
Created attachment 91886 [details]
now with test case
Comment 9 Eric Seidel (no email) 2011-05-02 00:32:01 PDT
Created attachment 91890 [details]
Updated with more tests
Comment 10 Adam Barth 2011-05-02 00:39:48 PDT
Comment on attachment 91890 [details]
Updated with more tests

View in context: https://bugs.webkit.org/attachment.cgi?id=91890&action=review

This is a patch of amazingness now.  Thanks you.  :)

> Tools/Scripts/webkitpy/tool/bot/commitqueuetask_unittest.py:88
> +        self._test_run_counter = -1

Why does this start at -1?
Comment 11 Eric Seidel (no email) 2011-05-02 00:47:36 PDT
Created attachment 91892 [details]
Patch for landing
Comment 12 Eric Seidel (no email) 2011-05-02 00:47:44 PDT
(In reply to comment #10)
> (From update of attachment 91890 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=91890&action=review
> 
> This is a patch of amazingness now.  Thanks you.  :)
> 
> > Tools/Scripts/webkitpy/tool/bot/commitqueuetask_unittest.py:88
> > +        self._test_run_counter = -1
> 
> Why does this start at -1?

I've updated the comments to explain.
Comment 13 Eric Seidel (no email) 2011-05-02 00:53:17 PDT
Committed r85463: <http://trac.webkit.org/changeset/85463>