Summary: | Add a unit-test to EWS for scenario when there is flakiness with patch and also on clean tree | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Aakash Jain <aakash_jain> | ||||||
Component: | Tools / Tests | Assignee: | Aakash Jain <aakash_jain> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | aakash_jain, ap, ews-watchlist, glenn, jbedard, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | Other | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Aakash Jain
2019-11-29 10:06:01 PST
Created attachment 384518 [details]
Unit-test for old EWS
Committed r252943: <https://trac.webkit.org/changeset/252943> Created attachment 384519 [details]
Similar unit-test for new EWS
. Committed r252946: <https://trac.webkit.org/changeset/252946> I think that I know why I didn't believe that this was the old EWS behavior. Looking at the "if self._results_failed_different_tests" branch in PatchAnalysisTask._retry_layout_tests, I see that it calls self._should_defer_patch_or_throw, which says that the result should be a PASS. But then we ignore the result, and make it a DEFER anyway! if tests_that_consistently_failed: if self._should_defer_patch_or_throw(tests_that_consistently_failed, first_results_archive, first_script_error, first_failure_status_id): return False # Defer patch # At this point we know that at least one test flaked, but no consistent failures # were introduced. This is a bit of a grey-zone. return False # Defer patch What's the point of the first "return False" here if we are going to defer either way? |