This is an archive of the discontinued LLVM Phabricator instance.

[lit] Print substitutions with --show-suites
ClosedPublic

Authored by ldionne on Apr 9 2020, 11:42 AM.

Details

Summary

We already print available features, and it can be useful to print
substitutions as well since those are a pretty fundamental part of
a test suite. We could also consider printing other things like the
test environment, however the need doesn't appear to be as strong.

Before:

$ lit -sv libcxx/test --show-suites
-- Test Suites --
  libc++ - 6350 tests
    Source Root: [...]
    Exec Root  : [...]
    Available Features : -faligned-allocation -fsized-deallocation [...]

After:

$ lit -sv libcxx/test --show-suites
-- Test Suites --
  libc++ - 6350 tests
    Source Root: [...]
    Exec Root  : [...]
    Available Features : -faligned-allocation -fsized-deallocation [...]
    Available Substitutions : %{build_module} => [...]
                              %{build} => %{cxx} -o [...]

Diff Detail

Event Timeline

ldionne created this revision.Apr 9 2020, 11:42 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 9 2020, 11:42 AM

I thought this was useful, but I do admit it makes the output of --show-suites a lot more verbose because substitutions are larger than available features, and they are one per line. I still think this is useful, but I'm open to discussions.

yln accepted this revision.Apr 9 2020, 12:15 PM

I think --show-suites is already a debugging feature (to figure out what has been discovered) so a bit more verbosity shouldn't hurt.

LGTM, with nits.

llvm/utils/lit/lit/main.py
139–140

Let's change this to always print available_features and substitutions. Not having any is rare and that is information as well.

140–145

Let's remove the space before the colon here. For Source/Exec Root it aligns the output, but for features and substitutions I don't think that it improves legibility.

142

I think tuples already have a good natural sort.

145

Maybe use ljust here?

llvm/utils/lit/tests/Inputs/discovery/lit.cfg
17

Thanks for adding a test for an existing feature! :)

20

Maybe provide unsorted entries here (also for features) so that the test proves that we sort it.

This revision is now accepted and ready to land.Apr 9 2020, 12:15 PM
ldionne updated this revision to Diff 256357.Apr 9 2020, 12:48 PM
ldionne marked 6 inline comments as done.

Address review comments.

jdenny accepted this revision.Apr 10 2020, 8:28 AM

Other than the comment I added, LGTM. Thanks.

llvm/utils/lit/lit/main.py
139–140

Please mention the change for available_features in the patch summary.

ldionne marked an inline comment as done.Apr 13 2020, 9:01 AM
This revision was automatically updated to reflect the committed changes.