[PATCH] D82233: [lit] Add --show command line option

Julian Lettner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 8 11:35:47 PDT 2020


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


================
Comment at: llvm/docs/CommandGuide/lit.rst:120-121
+
+ Show the names of the specified tests. Choose from:
+ all, excluded, skipped, unsupported, pass, flakypass, xfail.
+
----------------
jdenny wrote:
> yln wrote:
> > varungandhi-apple wrote:
> > > 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.
> > 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).
> 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.
> 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?


================
Comment at: llvm/utils/lit/lit/cl_arguments.py:204
+        else:
+           opts.shown_codes.add(lit.Test.ResultCode._instances[code.upper()])
 
----------------
jdenny wrote:
> What happens if there are user-defined result codes that are spelled the same except for case?
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82233





More information about the llvm-commits mailing list