[lit] Add --show-xxx command line options

[yln]

Summary

Provide --show-xxx flags for all non-failure result codes, just as we
already do for --show-xfail and --show-unsupported.

[yln]

Since --show subsumes --show-unsupported and --show-xfail, how do people feel about removing those in a separate follow-up change?

llvm/docs/CommandGuide/lit.rst

166 ↗

(On Diff #272174)

Drive by fix. Lit does not respect the MAX_FAILURES env var.

[varungandhi-apple]

llvm/docs/CommandGuide/lit.rst

120–121 ↗

(On Diff #272174)

Could you add an example here with how multiple items should be selected? For example, one might wonder

1. Can you do something like --show skipped,xfail?
2. What are the semantics of --show skipped --show xfail (does it mean skipped AND xfail or does it only mean xfail)?

You might also want to add a sentence to --help text on how to use multiple options.

[yln]

llvm/docs/CommandGuide/lit.rst

120–121 ↗

(On Diff #272174)

Good point, I will add an example here.

1. See test.
2. Last one wins.
3. The --help part is auto-generated by argparse (because we use the choices parameter).

[ldionne]

In D82233#2104573, @yln wrote:

Since --show subsumes --show-unsupported and --show-xfail, how do people feel about removing those in a separate follow-up change?

I like this a lot. And +1 for removing the redundant options.

[yln]

Update docs to show how to specify multiple result codes.

[jdenny]

Looks like a useful feature. Thanks for working on it.

llvm/docs/CommandGuide/lit.rst

120–121 ↗

(On Diff #272174)

Why not a comma-separated list?

For example, I downloaded your patch and tried this:

$ ./bin/llvm-lit test/Support
-- Testing: 2 tests, 2 workers --
PASS: LLVM :: Support/check-default-options.txt (1 of 2)
PASS: LLVM :: Support/interrupts.test (2 of 2)

Testing Time: 0.11s
  Passed: 2
$ ./bin/llvm-lit --show all test/Support
usage: lit [-h] [--version] [-j N] [--config-prefix NAME] [-D NAME=VAL] [-q]
           [-s] [-v] [-vv] [-a] [-o PATH] [--no-progress-bar]
           [--show-unsupported] [--show-xfail]
           [--show {all,excluded,skipped,unsupported,pass,flakypass,xfail} [{all,excluded,skipped,unsupported,pass,flakypass,xfail} ...]]
           [--path PATH] [--vg] [--vg-leak] [--vg-arg ARG] [--time-tests]
           [--no-execute] [--xunit-xml-output XUNIT_XML_OUTPUT]
           [--timeout MAXINDIVIDUALTESTTIME] [--max-failures MAX_FAILURES]
           [--allow-empty-runs] [--max-tests N] [--max-time N] [--shuffle]
           [-i] [--filter REGEX] [--num-shards M] [--run-shard N] [--debug]
           [--show-suites] [--show-tests] [--show-used-features]
           TEST_PATH [TEST_PATH ...]
lit: error: argument --show: invalid choice: 'test/Support' (choose from 'all', 'excluded', 'skipped', 'unsupported', 'pass', 'flakypass', 'xfail')

The usage message shows that --show can be used before TEST_PATH, but it doesn't work unless I add -- in between, which isn't listed in the options. Alternatively, I can specify --show after TEST_PATH, but that also isn't mentioned in the usage summary above.

If the values were comma-separated, this wouldn't be an issue.

166 ↗

(On Diff #272174)

Thanks for taking care of that.

llvm/utils/lit/lit/cl_arguments.py

199

What happens if there are user-defined result codes that are spelled the same except for case?

[yln]

llvm/docs/CommandGuide/lit.rst

120–121 ↗

(On Diff #272174)

Why not a comma-separated list?

Yes, I think this would also "feel" more natural than the current space-separated list and avoid the oddities about parameter ordering.
Surprisingly, argparse does not support this. I see two options:

• Implement this ourselves (it's actually not too bad): https://stackoverflow.com/a/60205263/271968
• We could generate flags, e.g., --show-xfail, for each result code. Pro: this would be in line with the existing flags for unsupported and fail. Con: Extending this scheme to accept user-defined codes would be harder.

What do you think? Do you have a preference or additional ideas?

llvm/utils/lit/lit/cl_arguments.py

199

Unfortunately, this can't be used (yet) to specify user-defined result codes at all. User codes are usually registered in config files and this code executes before we evaluate configs, i.e., it will print argument --show: invalid choice: 'user-code' (choose from ...)

If we think it's worth it then we could push "choice validation" to a later point after we processed the user configs.

[jdenny]

llvm/docs/CommandGuide/lit.rst

120–121 ↗

(On Diff #272174)

One aspect I like about the second option (--show-*) is that the semantics of multiple occurrences are more intuitive. The first option (comma-separated list) begs the question of accummulate vs. override. But this is a minor point.

If I had to choose right now, I'd go with the second option, primarily because it's consistent with the existing interface. However, I don't have strong feelings about this, so I could be happy with either.

llvm/utils/lit/lit/cl_arguments.py

199

I don't know that --show support for user-defined result codes needs to be implemented in this patch.

In that case, the case-insensitivity issue I raised is not relevant yet, right? That can be addressed later then.

[yln]

Add -show-xxx for all result codes in addtion to unsupported and xfail.

[yln]

@jdenny

I changed our approach to just adding --show-<result-code> flags for all non-failure result codes, just as we already do for --show-xfail and --show-unsupported.

The help entries for the flags look like this:

--show-excluded       Show excluded tests (EXCLUDED)
--show-skipped        Show skipped tests (SKIPPED)
--show-unsupported    Show unsupported tests (UNSUPPORTED)
--show-pass           Show passed tests (PASS)
--show-flakypass      Show passed with retry tests (FLAKYPASS)
--show-xfail          Show expectedly failed tests (XFAIL)

llvm/utils/lit/lit/cl_arguments.py

199

In that case, the case-insensitivity issue I raised is not relevant yet, right?

That's right!

[jdenny]

The last update lost a drive-by fix for MAX_FAILURES. I didn't check to see if that was fixed elsewhere.

Other than the comment suggestion I just added, LGTM.

llvm/utils/lit/lit/cl_arguments.py

71

I'd appreciate a comment here to clarify that user-defined result codes are not supported by --show-*.

[yln]

In D82233#2140554, @jdenny wrote:

The last update lost a drive-by fix for MAX_FAILURES. I didn't check to see if that was fixed elsewhere.

Fixed by author: https://github.com/llvm/llvm-project/commit/8cd117c24f48428e01f88cf18480e5af7eb20c0c

Other than the comment suggestion I just added, LGTM.

I will add a comment before committing. Thanks for the quick reviews!

llvm/utils/lit/lit/cl_arguments.py

def parse_args():
     format_group.add_argument("-o", "--output",
             type=lit.reports.JsonReport,
             help="Write test results to the provided path",
             metavar="PATH")
     format_group.add_argument("--no-progress-bar",
             dest="useProgressBar",
             help="Do not use curses based progress bar",
             action="store_false")
-    format_group.add_argument("--show-unsupported",
-            help="Show unsupported tests",
-            action="store_true")
-    format_group.add_argument("--show-xfail",
-            help="Show tests that were expected to fail",
-            action="store_true")
+
+    success_codes = [c for c in lit.Test.ResultCode.all_codes()
+                     if not c.isFailure]
+    for code in success_codes:
+        format_group.add_argument(
+            "--show-{}".format(code.name.lower()),
+            dest="shown_codes",
+            help="Show {} tests ({})".format(code.label.lower(), code.name),
+            action="append_const",
+            const=code,
+            default=[])
 
     execution_group = parser.add_argument_group("Test Execution")
     execution_group.add_argument("--path",
             help="Additional paths to add to testing environment",
             action="append",
             default=[])
     execution_group.add_argument("--vg",
             dest="useValgrind",
if opts.numShards or opts.runShard:
         if not opts.numShards or not opts.runShard:
             parser.error("--num-shards and --run-shard must be used together")
         if opts.runShard > opts.numShards:
             parser.error("--run-shard must be between 1 and --num-shards (inclusive)")
         opts.shard = (opts.runShard, opts.numShards)
     else:
         opts.shard = None
 
-    opts.show_results = set()
-    if opts.show_unsupported:
-        opts.show_results.add(lit.Test.UNSUPPORTED)
-    if opts.show_xfail:
-        opts.show_results.add(lit.Test.XFAIL)
-
     opts.reports = filter(None, [opts.output, opts.xunit_xml_output])
 
     return opts
 
 
 def _positive_int(arg):
     return _int(arg, 'positive', lambda i: i > 0)
 
 
 def _non_negative_int(arg):
     return _int(arg, 'non-negative', lambda i: i >= 0)
 
 

llvm/utils/lit/lit/main.py

 
 
 def print_results(tests, elapsed, opts):
     tests_by_code = {code: [] for code in lit.Test.ResultCode.all_codes()}
     for test in tests:
         tests_by_code[test.result.code].append(test)
 
     for code in lit.Test.ResultCode.all_codes():
-        print_group(tests_by_code[code], code, opts.show_results)
+        print_group(tests_by_code[code], code, opts.shown_codes)
 
     print_summary(tests_by_code, opts.quiet, elapsed)
 
 
-def print_group(tests, code, show_results):
+def print_group(tests, code, shown_codes):
     if not tests:
         return
-    if not code.isFailure and code not in show_results:
+    if not code.isFailure and code not in shown_codes:
         return
     print('*' * 20)
     print('{} Tests ({}):'.format(code.label, len(tests)))
     for test in tests:
         print('  %s' % test.getFullName())
     sys.stdout.write('\n')
 
 

llvm/utils/lit/tests/Inputs/show-result-codes/fail.txt

+RUN: false

llvm/utils/lit/tests/Inputs/show-result-codes/lit.cfg

+import lit.formats
+config.name = 'show-result-codes'
+config.suffixes = ['.txt']
+config.test_format = lit.formats.ShTest()
+config.test_source_root = None
+config.test_exec_root = None

llvm/utils/lit/tests/Inputs/show-result-codes/pass.txt

+RUN: true

llvm/utils/lit/tests/Inputs/show-result-codes/unsupported.txt

+REQUIRES: missing-feature
+RUN: true

llvm/utils/lit/tests/Inputs/show-result-codes/xfail.txt

+XFAIL: *
+RUN: false

llvm/utils/lit/tests/show-result-codes.py

+# Test the --show-<result-code> {pass,unsupported,xfail,...} options.
+#
+# RUN: not %{lit} %{inputs}/show-result-codes                           |  FileCheck %s --check-prefix=NONE
+# RUN: not %{lit} %{inputs}/show-result-codes --show-pass               |  FileCheck %s --check-prefix=ONE
+# RUN: not %{lit} %{inputs}/show-result-codes --show-pass --show-xfail  |  FileCheck %s --check-prefix=MULTIPLE
+
+# Failing tests are always shown
+# NONE-NOT: Unsupported Tests (1)
+# NONE-NOT: Passed Tests (1)
+# NONE-NOT: Expectedly Failed Tests (1)
+# NONE:     Failed Tests (1)
+
+# ONE-NOT: Unsupported Tests (1)
+# ONE:     Passed Tests (1)
+# ONE-NOT: Expectedly Failed Tests (1)
+# ONE:     Failed Tests (1)
+
+# MULTIPLE-NOT: Unsupported Tests (1)
+# MULTIPLE:     Passed Tests (1)
+# MULTIPLE:     Expectedly Failed Tests (1)
+# MULTIPLE:     Failed Tests (1)