[PATCH] D76288: [lit] Add builtin support for flaky tests in lit

[Reid Kleckner via Phabricator via llvm-commits]

rnk added a comment.

In D76288#1926688 <https://reviews.llvm.org/D76288#1926688>, @ldionne wrote:

> Note that an alternative design would be to drop the integer from `ALLOW_FAILURES:` and make it a tag keyword only. Then, `config.test_retry_attempts` could be applied only on the tests that are marked with that keyword. I actually prefer this approach, however it is a behavior change for existing users of `config.test_retry_attempts`.


I guess I prefer this approach for now. So far, the big users of lit (clang and llvm) have few flaky tests, so lit has had no support for retries or deflaking. With your keyword approach, hopefully flaky tests will not proliferate too quickly.

> Also, I'm open to changing `ALLOW_FAILURES` to something else, like `ALLOW_RETRIES`. Suggestions welcome.

I guess I like `ALLOW_RETRIES: N` the best. My other idea was `RETRY_LIMIT`.

> Finally, note that this doesn't handle `XFAIL` tests that would succeed repeatedly. This is actually an existing condition of `config.test_retry_attempts`, and the root cause is that we translate test results higher up in the call chain, near `_execute` in `worker.py`. Unfortunately, handling `ALLOW_FAILURES` at that point involves parsing the test several times, which messes up things like `test.xfails`. Since this is a pre-existing condition, I did not try to fix it cause it would have required a probably-breaking refactoring.

I don't think we need to put any effort into making this work. Flaky failing tests should be disabled instead.

Basically, looks good, but I'd suggest a different tag. Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76288/new/

https://reviews.llvm.org/D76288