Bug 204368 - EWS should use results database instead of doing clean-tree runs
Summary: EWS should use results database instead of doing clean-tree runs
Status: ASSIGNED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Aakash Jain
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-11-19 12:09 PST by Aakash Jain
Modified: 2021-09-10 09:27 PDT (History)
5 users (show)

See Also:


Attachments
Patch (4.86 KB, patch)
2019-11-20 11:08 PST, Jonathan Bedard
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Aakash Jain 2019-11-19 12:09:11 PST
EWS bots do clean-tree run when the test fails, in order to confirm if the test-failure is pre-existing or introduced by the patch. A lot of time, test-failure information is already available in results database (through build.webkit.org bots). We should try to get the clean-tree failure info from results database using its API, and avoid doing clean-tree runs. This would save a lot of time on EWS bots.
Comment 1 Alexey Proskuryakov 2019-11-19 17:43:25 PST
How can we confirm that this will be sufficient? It feels like EWS-only failures are somewhat common.

I think that doing this in addition to retries is better than "instead" as the bug suggests. We can still retry with a clean tree if the failures are unknown.
Comment 2 Jonathan Bedard 2019-11-20 09:45:44 PST
(In reply to Alexey Proskuryakov from comment #1)
> How can we confirm that this will be sufficient? It feels like EWS-only
> failures are somewhat common.
> 
> I think that doing this in addition to retries is better than "instead" as
> the bug suggests. We can still retry with a clean tree if the failures are
> unknown.

We could do the retry if the failures are unknown, yes. And we might start that way.

I think the retry is generally a bad idea, though. It means that in the case where an engineer did introduce a regression, we are going to be taking significantly longer to actually report that regression than in the successful case. Given the longer turn around for layout tests, I think this pretty significantly reduces the usefulness of EWS. We're essentially delaying results in the exact circumstances those results would be most impactful.

I suppose we make the bubble orange on retry, but I feel like we might be training folks to ignore orange bubbles given how flakey things are at the moment.
Comment 3 Alexey Proskuryakov 2019-11-20 10:31:45 PST
If we don't retry, then we will have a lot of false positives, training engineers to ignore EWS altogether, not just in orange state.
Comment 4 Jonathan Bedard 2019-11-20 11:08:15 PST Comment hidden (obsolete)
Comment 5 Jonathan Bedard 2019-11-20 11:14:39 PST
Comment on attachment 383973 [details]
Patch

This patch should have been uploaded to https://bugs.webkit.org/show_bug.cgi?id=204409.
Comment 6 Jonathan Bedard 2019-11-20 11:25:44 PST
(In reply to Alexey Proskuryakov from comment #3)
> If we don't retry, then we will have a lot of false positives, training
> engineers to ignore EWS altogether, not just in orange state.

I'm not entirely convinced that we need retries to get rid of false positives. There isn't a good reason that clean tree runs in EWS should differ from trunk results at all, and if we're cross-referencing trunk results, we could even expand the set of commits we're looking at to find flakey tests.

This approach hinges on us quickly fixing things like https://bugs.webkit.org/show_bug.cgi?id=204409, but I really do think that things like that are bugs, not expected behavior.
Comment 7 Alexey Proskuryakov 2019-11-20 15:23:45 PST
One big reason is that EWS is different hardware. We definitely have quite a few tests that are only flaky or failing on EWS.
Comment 8 Aakash Jain 2019-12-05 08:24:24 PST
Seems like one thing we agree on is that, if the test(s) which fails in EWS is also marked as failing in results database, then we can skip the retry/clean-tree-run.

If the test failure is new (not visible in results database), then we can continue with existing retry/clean-tree-run logic. 

This would still be a significant efficiency improvement, given that most of the times test failures seen in EWS are also seen in build.webkit.org
Comment 9 Alexey Proskuryakov 2019-12-05 08:58:33 PST
Correct. I think that we should use a fairly strict definition of "failing in results database', something like the test having failed within 24 hours, and maybe even require the same OS/flavor.
Comment 10 Radar WebKit Bug Importer 2021-09-10 09:27:43 PDT
<rdar://problem/82975679>