Bug 265066 – [run-webkit-tests] Tests which fail and then crash on retry aren't flakey

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Jonathan Bedard]

Our run-webkit-tests logic retries failed tests runs in some configurations. It will classify a test as "flakey" if the retry results don't match the original results, and EWS uses this to decide when to mark a change as having failed tests.

This logic isn't quite right, though, as demonstrated by https://github.com/WebKit/WebKit/pull/20624. When we say "flakey", we mean that the second test run matches an expected test result, not that the second test run is a different (but also failing) result.

[Radar WebKit Bug Importer]

<rdar://problem/118578976>

[Aakash Jain]

I'm not sure if I understand this bug report. It would be helpful to have expected and actual behaviours, along-with link to buildbot build demonstrating the issue (the linked PR has many builds).

[Jonathan Bedard]

Relevant bit of code is this in webkitpy, line 311 in Tools/Scripts/webkitpy/layout_tests/models/test_run_results.py:
```
elif test_name in initial_results.unexpected_results_by_name:
    if retry_results and test_name in retry_results.unexpected_results_by_name:
        retry_result_type = retry_results.unexpected_results_by_name[test_name].type
        if result_type != retry_result_type:
            if enabled_pixel_tests_in_retry and result_type == test_expectations.TEXT and (retry_result_type == test_expectations.IMAGE_PLUS_TEXT or retry_result_type == test_expectations.MISSING):
                if retry_result_type == test_expectations.MISSING:
                    num_missing += 1
                num_regressions += 1
                test_dict['report'] = 'REGRESSION'
            else:
                num_flaky += 1
                test_dict['report'] = 'FLAKY'
            actual.append(keywords[retry_result_type])
        else:
            num_regressions += 1
            test_dict['report'] = 'REGRESSION'
```
These 'if' statements are a mess, but basically, instead of checking if the retry result is unexpected, we just check if the retry result matches the original, and if it doesn't, consider the test flaky. That's not really what people mean when they say "flaky". "flaky" means that a test sometimes passes, but sometimes fails. A test which, for example, either times out or crashes, is failing, not flaky.

[Jonathan Bedard]

Pull request: https://github.com/WebKit/WebKit/pull/28856

[EWS]

Committed 279220@main (ca4e82e8dd0a): <https://commits.webkit.org/279220@main>

Reviewed commits have been landed. Closing PR #28856 and removing active labels.