Let test runner rerun failures to test for flakes.

Let test runner rerun failures to test for flakes.

When enabled, this dynamically adds jobs that had failures back to the pool. Special json output for flakes will be handled in a separate CL.

BUG=374134
LOG=n
R=jkummerow@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=22143

[Michael Achenbach, 2014-07-01 15:21:11]

PTAL

https://codereview.chromium.org/360113003/diff/20001/tools/testrunner/local/e...
File tools/testrunner/local/execution.py (right):

https://codereview.chromium.org/360113003/diff/20001/tools/testrunner/local/e...
tools/testrunner/local/execution.py:149: queue.append([self._GetJob(test)])
For simplicity, I put it all within the try block. Shouldn't make a big
difference as there is not much that can fail besides GetCommand.

[Jakob Kummerow, 2014-07-02 07:27:27]

LGTM with comments.

Of course the high-level comment is that handling/hiding flaky failures in the
infrastructure takes away incentive to fix flaky tests for good, which in the
long run might have undesirable consequences...

https://codereview.chromium.org/360113003/diff/20001/tools/run-deopt-fuzzer.py
File tools/run-deopt-fuzzer.py (right):

https://codereview.chromium.org/360113003/diff/20001/tools/run-deopt-fuzzer.p...
tools/run-deopt-fuzzer.py:375: 0,
nit: may want to add a comment what the False/True/0/0 lines are specifying

https://codereview.chromium.org/360113003/diff/20001/tools/run-tests.py
File tools/run-tests.py (right):

https://codereview.chromium.org/360113003/diff/20001/tools/run-tests.py#newco...
tools/run-tests.py:189: help="Maximum number of failures to rerun.",
nit: for better clarity, s/failures/failing test cases/. And maybe accordingly
change the other option's description to "Number of times to rerun each failing
test case".

https://codereview.chromium.org/360113003/diff/20001/tools/testrunner/local/e...
File tools/testrunner/local/execution.py (right):

https://codereview.chromium.org/360113003/diff/20001/tools/testrunner/local/e...
tools/testrunner/local/execution.py:123: test.duration = None
I don't think we need to reset duration and output (but it shouldn't hurt if we
do).

https://codereview.chromium.org/360113003/diff/20001/tools/testrunner/objects...
File tools/testrunner/objects/testcase.py (right):

https://codereview.chromium.org/360113003/diff/20001/tools/testrunner/objects...
tools/testrunner/objects/testcase.py:55: self.dependency, list(self.outcomes or
[]), self.id, self.run]
IIUC, |run| is always 1 when a test is packed for network transmission. So we
could skip the value here...

https://codereview.chromium.org/360113003/diff/20001/tools/testrunner/objects...
tools/testrunner/objects/testcase.py:64: test.run = task[6]
...and  simply fill in 1 here (or not set it at all, as the constructor defaults
to 1 anyway). Sending bytes over the wire is fairly expensive, compared to
almost anything else the test runner does.

[Michael Achenbach, 2014-07-02 08:06:33]

All done.

> Of course the high-level comment is that handling/hiding flaky failures in the
> infrastructure takes away incentive to fix flaky tests for good, which in the
> long run might have undesirable consequences...

Depends on how we handle it. I don't want to hide flakes, rather add the
information that a failure is a flake. The overall build can still be marked
red.

https://codereview.chromium.org/360113003/diff/20001/tools/run-deopt-fuzzer.py
File tools/run-deopt-fuzzer.py (right):

https://codereview.chromium.org/360113003/diff/20001/tools/run-deopt-fuzzer.p...
tools/run-deopt-fuzzer.py:375: 0,
On 2014/07/02 07:27:26, Jakob wrote:
> nit: may want to add a comment what the False/True/0/0 lines are specifying

Done.

https://codereview.chromium.org/360113003/diff/20001/tools/run-tests.py
File tools/run-tests.py (right):

https://codereview.chromium.org/360113003/diff/20001/tools/run-tests.py#newco...
tools/run-tests.py:189: help="Maximum number of failures to rerun.",
On 2014/07/02 07:27:26, Jakob wrote:
> nit: for better clarity, s/failures/failing test cases/. And maybe accordingly
> change the other option's description to "Number of times to rerun each
failing
> test case".

Done.

https://codereview.chromium.org/360113003/diff/20001/tools/testrunner/local/e...
File tools/testrunner/local/execution.py (right):

https://codereview.chromium.org/360113003/diff/20001/tools/testrunner/local/e...
tools/testrunner/local/execution.py:123: test.duration = None
On 2014/07/02 07:27:26, Jakob wrote:
> I don't think we need to reset duration and output (but it shouldn't hurt if
we
> do).

OK - keeping it.

https://codereview.chromium.org/360113003/diff/20001/tools/testrunner/objects...
File tools/testrunner/objects/testcase.py (right):

https://codereview.chromium.org/360113003/diff/20001/tools/testrunner/objects...
tools/testrunner/objects/testcase.py:55: self.dependency, list(self.outcomes or
[]), self.id, self.run]
On 2014/07/02 07:27:26, Jakob wrote:
> IIUC, |run| is always 1 when a test is packed for network transmission. So we
> could skip the value here...

Done.

https://codereview.chromium.org/360113003/diff/20001/tools/testrunner/objects...
tools/testrunner/objects/testcase.py:64: test.run = task[6]
On 2014/07/02 07:27:26, Jakob wrote:
> ...and  simply fill in 1 here (or not set it at all, as the constructor
defaults
> to 1 anyway). Sending bytes over the wire is fairly expensive, compared to
> almost anything else the test runner does.

Done.

[Michael Achenbach, 2014-07-02 08:15:58]

Committed patchset #2 manually as r22143 (presubmit successful).