[PATCH] [LIT] Change the semantics of the LIT options '-s' and '-v' and modify when information is output.

[Daniel Dunbar]

Hi Eric,

I like the feature change, but I find the implementation makes it harder to
see what the conditions are. I would like:

0. Use the label TEST ... OUTPUT when showing the output with no failures.
1. Keep the metrics output in their own section, and with the current
conditions. We can change that separately if necessary.

This should leave two boolean conditions to check, both of which can be
computed prior to "if shouldShow:..." and then we continue if both are
False.

Also, I would like to see a test that sanity checks for the expected
output, and an update to the docs describing the option.

 - Daniel

On Thu, Mar 19, 2015 at 1:32 PM, Eric Fiselier <eric at efcs.ca> wrote:

> Hi danalbert, jroelofs, ddunbar,
>
> Currently I have benchmark tests that I run with LIT that have meaningful
> output even when the test passes. The aim of this patch is to modify the
> existing LIT output options in such a way  that they can be used to always
> print the output of a test with minimal changes to how output is currently
> handled.
>
> The main change in this patch is that `-v` can be specified multiple times
> to increase the amount of information that is output. Using `lit -vv` will
> cause LIT to report a passing tests output (if any is present).
>
> This patch has a secondary drive-by change that changes when metrics are
> output. Currently metrics are reported when:
> 1. The test failed.
> 2. The test passed and neither '-s' nor '-q' are specified.
>
> With this patch metrics are reported ONLY when '-s' is not specified and
> one of the following is true:
> 1. The test failed.
> 2. The test passed and '-vv' is specified.
>
> I'm happy to remove this part of the change but I think it is beneficial
> to the general readability of the output. I don't think we should consider
> metrics to be part of the "succinct" output.
>
> http://reviews.llvm.org/D8460
>
> Files:
>   utils/lit/lit/main.py
>
> Index: utils/lit/lit/main.py
> ===================================================================
> --- utils/lit/lit/main.py
> +++ utils/lit/lit/main.py
> @@ -42,7 +42,7 @@
>              self.progressBar.update(float(self.completed)/self.numTests,
>                                      test.getFullName())
>
> -        shouldShow = test.result.code.isFailure or \
> +        shouldShow = test.result.code.isFailure or self.opts.showOutput >
> 1 or \
>              (not self.opts.quiet and not self.opts.succinct)
>          if not shouldShow:
>              return
> @@ -55,21 +55,28 @@
>          print('%s: %s (%d of %d)' % (test.result.code.name, test_name,
>                                       self.completed, self.numTests))
>
> -        # Show the test failure output, if requested.
> -        if test.result.code.isFailure and self.opts.showOutput:
> -            print("%s TEST '%s' FAILED %s" % ('*'*20, test.getFullName(),
> -                                              '*'*20))
> -            print(test.result.output)
> -            print("*" * 20)
> -
> -        # Report test metrics, if present.
> -        if test.result.metrics:
> -            print("%s TEST '%s' RESULTS %s" % ('*'*10, test.getFullName(),
> -                                               '*'*10))
> -            items = sorted(test.result.metrics.items())
> -            for metric_name, value in items:
> -                print('%s: %s ' % (metric_name, value.format()))
> -            print("*" * 10)
> +        showMetrics = test.result.metrics and not self.opts.succinct
> +        showPassingTest = (self.opts.showOutput > 1
> +            and (test.result.output or showMetrics))
> +
> +        # Show the test output, if requested.
> +        if ((test.result.code.isFailure and self.opts.showOutput)
> +            or showPassingTest):
> +            result_str = 'FAILED' if test.result.code.isFailure else
> 'RESULT'
> +            banner = '*'*20 if test.result.code.isFailure else '*'*10
> +            print("%s TEST '%s' %s %s" % (banner, test.getFullName(),
> +                                          result_str, banner))
> +            if test.result.output:
> +                print(test.result.output)
> +
> +            # Report test metrics, if present.
> +            if showMetrics:
> +                if test.result.output:
> +                    print(banner)
> +                items = sorted(test.result.metrics.items())
> +                for metric_name, value in items:
> +                    print('%s: %s ' % (metric_name, value.format()))
> +            print(banner)
>
>          # Ensure the output is flushed.
>          sys.stdout.flush()
> @@ -162,7 +169,7 @@
>                       action="store_true", default=False)
>      group.add_option("-v", "--verbose", dest="showOutput",
>                       help="Show all test output",
> -                     action="store_true", default=False)
> +                     action="count", default=0)
>      group.add_option("-o", "--output", dest="output_path",
>                       help="Write test results to the provided path",
>                       action="store", type=str, metavar="PATH")
>
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150405/98602fea/attachment.html>