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

Fri Feb 19 09:39:03 PST 2021

jdenny added a comment.

In D96662#2574156 <https://reviews.llvm.org/D96662#2574156>, @thopre wrote:

> I like --filter-out. Could we maybe introduce --filter-in and make --filter an alias for --filter-in?

Possibly.  We'd have to decide whether there should be a LIT_FILTER_IN and how it would override or otherwise combine with LIT_FILTER.  The aliasing probably should be a separate patch.

In D96662#2574214 <https://reviews.llvm.org/D96662#2574214>, @davezarzycki wrote:

> I've renamed `--skip` to `--filter-out`. I prefer "out" to "not" because that matches the documentation for the command.

Thanks.

> I agree that `--xfail-file` will be useful but I don't need it yet and I think designing that feature is slightly complicated. Personally, if one is going to have a file, why limit it to just XFAIL? Why not support other `lit` features like `REQUIRES` or `UNSUPPORTED` or `ALLOW_RETRIES`? All of these might be useful to a downstream maintainer that needs to temporarily tweak a test until something is fixed.

I do see some appeal to having a general config file (for test suite users as opposed to test suite authors) instead of an xfail-specific file, but I've formed no strong opinions on the way `--xfail` is specified.

Other than some inline comments I just added, I'm fine with this patch as is, but I'm going to leave acceptance to reviewers who described their use cases.

================
Comment at: llvm/utils/lit/tests/Inputs/xfail-cl/lit.cfg:20
+# Check that substitutions are printed by --show-suites (and in the right order)
+config.substitutions = [('%key2', 'value2'), ('%key1', 'value1')]
----------------
Most of this config appears irrelevant to this test.

================
Comment at: llvm/utils/lit/tests/xfail-cl.py:7
+# CHECK-FILTER-DAG: FAIL: top-level-suite :: false.txt
+# CHECK-FILTER-DAG: FAIL: top-level-suite :: false2.txt
+# CHECK-FILTER-DAG: PASS: top-level-suite :: true.txt
----------------
Shouldn't these say XFAIL?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96662