[PATCH] D96662: [lit] Add --skip (inverse of --filter) and `--xfail`

Fri Feb 19 01:57:41 PST 2021

thopre added a comment.

In D96662#2572321 <https://reviews.llvm.org/D96662#2572321>, @yln wrote:

> `--skip`: is it okay if we rename this to `--filter-not`? It's an uglier name but
>
> - more "self explanatory", i..e, "opposite of `--filter`". Anyone who knows `--filter` will know what it does just by looking the arg name.
> - Discoverable: in the usage and help output, it will appear next to `--filter`.
> - In the summary, we have a category of `Skipped` tests: tests that were not run because the user hit `[Ctrl+C]` or because of overall timeout.  `--skip`ped tests would appear in the `Excluded` category (e.g., `--filter` and sharding feature).  Not using the word skip may help prevent some confusion.
>
> Can we also add the summary counts in the test, so that the test shows that `--filter-not` excluded tests are counted in the `Excluded` category? Thanks!
> Otherwise, LGTM, will approve today if you split this part out.
>
> `--xfail`: Thanks everyone for explaining. As someone who also has to deal with downstream test failures, you convinced me that this is useful to have!
> Summarizing my understanding: this is about the desire to (temporarily) mark tests as `XFAIL` in a centralized location (instead of editing individual test files), especially useful for a downstream integration workflows.
> @davezarzycki @jhenderson @thopre Is this accurate?

Yes. Besides the centralisation, it also avoids changing upstream source (i.e. it allows to separate modifications to test as opposed to disabled test).

> How about `--xfail-file` which names a file for which each line means one XFAILed test. I imagine that:
>
> - Working with a file seems better (`echo "test/path" >> xfail.txt`, `sed '/test/d' ./file`), than fiddling with the lit invocation itself.  Count how many XFAILed tests we have in upstream without even opening the file, etc.
> - The file would "make sense" in version control: line-wise addition/removals.  "Didn't this test fail before?  What was the issue previously? `git log -- xfail.txt` and see commit for fix and line removal.
>
> What do you think?

I think a file can be more convenient when lots of tests need to be disabled. I don't think that's ever been our case and I would personally be wary of disabling too many cases but it could be useful. By the way, would the syntax allow to skip entire directories?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96662