[PATCH] D77819: [lit] Add SKIPPED test result category

Fri Apr 10 12:54:00 PDT 2020

yln marked 3 inline comments as done.
yln added inline comments.

================
Comment at: llvm/utils/lit/lit/main.py:284
-       (lit.Test.UNSUPPORTED == code and not opts.show_unsupported) or \
-       (lit.Test.UNRESOLVED == code and (opts.max_failures is not None)):
         return
----------------
jdenny wrote:
> There's a change in the effect of `opts.max_failures` on both skipped tests and the remaining unresolved tests.
> 
> It seems like that should be mentioned in the patch summary.
> 
> Can at least the latter change be covered in lit's test suite?
UNRESOLVED tests are failures where the test failed due to an "infrastructure" issue, e.g., failure to parse a `REQUIRES:` line.

Previously all unexecuted tests (unexecuted for whatever reason: infrastructure failures, overall lit timeout, user interrupt, --max-tests) were all marked UNRESOLVED. So both logical groups for unexecuted tests: "skipped" and "unresolved" were given the same label and treated as failures.  However, a "skipped" shouldn't imply failure.

I think the check for `opts.max_failures` here was just an ad-hoc way to dry to deal with mixing both skipped and failed tests into the UNRESOLVED category: when `--max-failures` was specified and we stop executing because of it we mark all remaining tests as UNRESOLVED. However we shouldn't print those as failures here.

Anyways, things are more consistent now.  We have a proper category for "skipped" (not a failure) and the interaction between max_failures and UNRESOLVED has been removed.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77819