[webkit-reviews] review denied: [Bug 219500] EWS for layout tests: On the retry steps only run the tests that failed on the first step. : [Attachment 415319] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Dec 3 13:26:18 PST 2020


Alexey Proskuryakov <ap at webkit.org> has denied Carlos Alberto Lopez Perez
<clopez at igalia.com>'s request for review:
Bug 219500: EWS for layout tests: On the retry steps only run the tests that
failed on the first step.
https://bugs.webkit.org/show_bug.cgi?id=219500

Attachment 415319: Patch

https://bugs.webkit.org/attachment.cgi?id=415319&action=review




--- Comment #3 from Alexey Proskuryakov <ap at webkit.org> ---
Comment on attachment 415319
  --> https://bugs.webkit.org/attachment.cgi?id=415319
Patch

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

> Tools/ChangeLog:14
> +	   This helps to speed up the test times on the EWS when the patch
> +	   doesn't pass on the first try. This is specially useful when the
> +	   clean-tree-without-patch is not 100% green or has some flakies.

Great idea!

I can't think of any major problem with running a subset of tests in
clean-tree-without-patch, given that retries are enabled on EWS (so tests that
only fail because of dependencies aren't detected anyway).

However, re-run-layout-tests needs to run the whole test suite, so that we
could detect failures by count. E.g. if a patch introduces failures on a random
subset totaling 1% of tests, we would detect 30 failures on the first run, and
it's absolutely not sufficient to only re-run those 30 (chances are that we'll
hit zero failures, and green-light the patch).

There is a small problem with run-layout-tests-without-patch too. What about
tests that were just added with the patch, just marked in TestExpectations, or
just unmarked? Forcing these tests via command line arguments will produce
incorrect results in some of these cases.

> Tools/CISupport/ews-build/steps.py:1949
> +	       self.setCommand(self.command +
list(first_results_failing_tests))

Are we guaranteed to not run out of command line length limit?


More information about the webkit-reviews mailing list