[utils] revamp options controlling lit's output

Lit has a number of options controlling the output, but they don't
compose very well. This breaks the existing options down into smaller,
orthogonal options, and makes the existing options aliases of the new
ones.

This introduces the following options:
  --test-output {off,failed,all}
  --print-result-after {off,failed,all}
  --diagnostic-level {error,warning,note}
  --terse-summary
  --no-terse-summary
  --progress-bar (mirroring --no-progress-bar)

--test-output and --print-result-after are not entirely orthogonal, as
'--test-output X' requires that --print-result-after is set to at least
X, and implicitly does so if it isn't already. Conversely,
'--print-result-after Y' requires that --test-output is at most Y, and
implicitly lowers if it is higher. This means that the following
invocations have different end results, as they are applied in order:
  '--test-output all --print-result-after off'
  '--print-result-after off --test-output all'

The following existing options are now aliases as follows:
  -q, --quiet
    '--diagnostic-level=error --test-output=off --terse-summary'
  -s, --succinct
    '--progress-bar --test-progress=failed'
  -v, --verbose
    '--test-output=failed'
  -a, --show-all
    '--test-output=all'

These where all completely separate options and would override each
other in ad-hoc ways, with no regard to the order they were given.

This fixes #106643
This is based on the RFC
https://discourse.llvm.org/t/rfc-new-command-line-options-for-controlling-llvm-lit-output/
with the addition of --terse-summary, which was a behaviour of -q that
was not captured by the original RFC. This also diverges from the RFC in
that --debug is NOT folded into --diagnostic-level, because it can be
useful to debug any configuration, including those specifying
--diagnostic-level.

Example combination that is possible now but wasn't before:
'--diagnostic-level error --test-output all --progress-bar'
Another use case is aliases, where you can alias e.g:
  alias lit=llvm-lit --quiet
but still override the specified default options.

Author: hnrklssn

compiler-rt/test/lit.common.cfg.py

@@ -196,12 +196,12 @@ def push_dynamic_library_lookup_path(config, new_path):
 if test_cc_resource_dir is not None:
     test_cc_resource_dir = os.path.realpath(test_cc_resource_dir)
 if lit_config.debug:
-    lit_config.note(f"Resource dir for {config.clang} is {test_cc_resource_dir}")
+    lit_config.dbg(f"Resource dir for {config.clang} is {test_cc_resource_dir}")
 local_build_resource_dir = os.path.realpath(config.compiler_rt_output_dir)
 if test_cc_resource_dir != local_build_resource_dir and config.test_standalone_build_libs:
     if config.compiler_id == "Clang":
         if lit_config.debug:
-            lit_config.note(
+            lit_config.dbg(
                 f"Overriding test compiler resource dir to use "
                 f'libraries in "{config.compiler_rt_libdir}"'
             )

libcxx/utils/libcxx/test/config.py

@@ -22,6 +22,7 @@ def _appendToSubstitution(substitutions, key, value):
 
 def configure(parameters, features, config, lit_config):
     note = lambda s: lit_config.note("({}) {}".format(config.name, s))
+    debug = lambda s: lit_config.dbg("({}) {}".format(config.name, s))
     config.environment = dict(os.environ)
 
     # Apply the actions supplied by parameters to the configuration first, since
@@ -32,7 +33,7 @@ def configure(parameters, features, config, lit_config):
         for action in actions:
             action.applyTo(config)
             if lit_config.debug:
-                note(
+                debug(
                     "Applied '{}' as a result of parameter '{}'".format(
                         action.pretty(config, lit_config.params),
                         param.pretty(config, lit_config.params),
@@ -45,7 +46,7 @@ def configure(parameters, features, config, lit_config):
         for action in actions:
             action.applyTo(config)
             if lit_config.debug:
-                note(
+                debug(
                     "Applied '{}' as a result of implicitly detected feature '{}'".format(
                         action.pretty(config, lit_config.params), feature.pretty(config)
                     )

llvm/utils/lit/lit/LitConfig.py

@@ -1,6 +1,7 @@
 from __future__ import absolute_import
 import inspect
 import os
+import enum
 import platform
 import sys
 
@@ -25,7 +26,7 @@ def __init__(
         self,
         progname,
         path,
-        quiet,
+        diagnostic_level,
         useValgrind,
         valgrindLeakCheck,
         valgrindArgs,
@@ -46,7 +47,7 @@ def __init__(
         self.progname = progname
         # The items to add to the PATH environment variable.
         self.path = [str(p) for p in path]
-        self.quiet = bool(quiet)
+        self.diagnostic_level = diagnostic_level
         self.useValgrind = bool(useValgrind)
         self.valgrindLeakCheck = bool(valgrindLeakCheck)
         self.valgrindUserArgs = list(valgrindArgs)
@@ -155,8 +156,7 @@ def per_test_coverage(self, value):
     def load_config(self, config, path):
         """load_config(config, path) - Load a config object from an alternate
         path."""
-        if self.debug:
-            self.note("load_config from %r" % path)
+        self.dbg("load_config from %r" % path)
         config.load_from_path(path, self)
         return config
 
@@ -209,6 +209,8 @@ def getToolsPath(self, dir, paths, tools):
         return dir
 
     def _write_message(self, kind, message):
+        if not self.diagnostic_level_enabled(kind):
+            return
         # Get the file/line where this message was generated.
         f = inspect.currentframe()
         # Step out of _write_message, and then out of wrapper.
@@ -234,13 +236,19 @@ def substitute(self, string):
                 "unable to find %r parameter, use '--param=%s=VALUE'" % (key, key)
             )
 
+    def diagnostic_level_enabled(self, kind):
+        if kind == "debug":
+            return self.debug
+        return DiagnosticLevel.create(self.diagnostic_level) >= DiagnosticLevel.create(kind)
+
+    def dbg(self, message):
+        self._write_message("debug", message)
+
     def note(self, message):
-        if not self.quiet:
-            self._write_message("note", message)
+        self._write_message("note", message)
 
     def warning(self, message):
-        if not self.quiet:
-            self._write_message("warning", message)
+        self._write_message("warning", message)
         self.numWarnings += 1
 
     def error(self, message):
@@ -250,3 +258,22 @@ def error(self, message):
     def fatal(self, message):
         self._write_message("fatal", message)
         sys.exit(2)
+
+@enum.unique
+class DiagnosticLevel(enum.IntEnum):
+    FATAL = 0
+    ERROR = 1
+    WARNING = 2
+    NOTE = 3
+
+    @classmethod
+    def create(cls, value):
+        if value == "fatal":
+            return cls.FATAL
+        if value == "error":
+            return cls.ERROR
+        if value == "warning":
+            return cls.WARNING
+        if value == "note":
+            return cls.NOTE
+        raise ValueError(f"invalid diagnostic level {repr(value)} of type {type(value)}")

llvm/utils/lit/lit/LitTestCase.py

@@ -46,7 +46,7 @@ def load_test_suite(inputs):
     lit_config = lit.LitConfig.LitConfig(
         progname="lit",
         path=[],
-        quiet=False,
+        diagnostic_level="note",
         useValgrind=False,
         valgrindLeakCheck=False,
         valgrindArgs=[],

llvm/utils/lit/lit/TestingConfig.py

@@ -144,7 +144,7 @@ def load_from_path(self, path, litConfig):
         try:
             exec(compile(data, path, "exec"), cfg_globals, None)
             if litConfig.debug:
-                litConfig.note("... loaded config %r" % path)
+                litConfig.dbg("... loaded config %r" % path)
         except SystemExit:
             e = sys.exc_info()[1]
             # We allow normal system exit inside a config file to just

llvm/utils/lit/lit/cl_arguments.py

@@ -15,6 +15,58 @@ class TestOrder(enum.Enum):
     SMART = "smart"
 
 
+@enum.unique
+class TestOutputLevel(enum.IntEnum):
+    OFF = 0
+    FAILED = 1
+    ALL = 2
+
+    @classmethod
+    def create(cls, value):
+        if value == "off":
+            return cls.OFF
+        if value == "failed":
+            return cls.FAILED
+        if value == "all":
+            return cls.ALL
+        raise ValueError(f"invalid output level {repr(value)} of type {type(value)}")
+
+
+class TestOutputAction(argparse.Action):
+    def __init__(self, option_strings, dest, **kwargs):
+        super().__init__(option_strings, dest, nargs=None, **kwargs)
+
+    def __call__(self, parser, namespace, value, option_string=None):
+        TestOutputAction.setOutputLevel(namespace, self.dest, value)
+
+    @classmethod
+    def setOutputLevel(cls, namespace, dest, value):
+        setattr(namespace, dest, value)
+        #print(dest, value)
+        if dest == "test_output" and TestOutputLevel.create(namespace.print_result_after) < TestOutputLevel.create(value):
+            #print("print_result_after", value)
+            setattr(namespace, "print_result_after", value)
+        elif dest == "print_result_after" and TestOutputLevel.create(namespace.test_output) > TestOutputLevel.create(value):
+            #print("test_output", value)
+            setattr(namespace, "test_output", value)
+
+
+class AliasAction(argparse.Action):
+    def __init__(self, option_strings, dest, nargs=None, **kwargs):
+        self.expansion = kwargs.pop("alias", None)
+        if not self.expansion:
+            raise ValueError("no aliases expansion provided")
+        super().__init__(option_strings, dest, nargs=0, **kwargs)
+
+    def __call__(self, parser, namespace, value, option_string=None):
+        for e in self.expansion:
+            if callable(e):
+                e(namespace)
+            else:
+                dest, val = e
+                setattr(namespace, dest, val)
+
+
 def parse_args():
     parser = argparse.ArgumentParser(prog="lit", fromfile_prefix_chars="@")
     parser.add_argument(
@@ -55,41 +107,86 @@ def parse_args():
     )
 
     format_group = parser.add_argument_group("Output Format")
-    # FIXME: I find these names very confusing, although I like the
-    # functionality.
     format_group.add_argument(
-        "-q", "--quiet", help="Suppress no error output", action="store_true"
+        "--test-output",
+        help="Control whether the executed commands and their outputs are printed after each test has executed (default off)",
+        choices=["off", "failed", "all"],
+        default="off",
+        action=TestOutputAction,
+    )
+    format_group.add_argument(
+        "--print-result-after",
+        help="Control which the executed test names and results are printed after each test has executed (default all)",
+        choices=["off", "failed", "all"],
+        default="all",
+        action=TestOutputAction,
+    )
+    format_group.add_argument(
+        "--diagnostic-level",
+        help="Control how verbose lit diagnostics should be (default note)",
+        choices=["error", "warning", "note"],
+        default="note",
+    )
+    format_group.add_argument(
+        "--terse-summary",
+        help="Print the elapsed time and the number of passed tests after all tests have finished (default on)",
+        action="store_true",
+        dest="terse_summary",
+    )
+    format_group.add_argument(
+        "--no-terse-summary",
+        help="Don't show the elapsed time after all tests have finished, and only show the number of failed tests.",
+        action="store_false",
+        dest="terse_summary",
+    )
+    parser.set_defaults(terse_summary=False)
+    format_group.add_argument(
+        "-q", "--quiet", help="Alias for '--diagnostic-level=error --test-output=off --terse-summary'", action=AliasAction,
+        alias=[
+            lambda namespace: TestOutputAction.setOutputLevel(namespace, "print_result_after", "failed"),
+            lambda namespace: TestOutputAction.setOutputLevel(namespace, "test_output", "off"),
+            ("diagnostic_level", "error"),
+            ("terse_summary", True),
+            ],
     )
     format_group.add_argument(
         "-s",
         "--succinct",
-        help="Reduce amount of output."
-        " Additionally, show a progress bar,"
-        " unless --no-progress-bar is specified.",
-        action="store_true",
+        help="Alias for '--progress-bar --print-result-after=failed'",
+        action=AliasAction,
+        alias=[
+            ("useProgressBar", True),
+            lambda namespace: TestOutputAction.setOutputLevel(namespace, "print_result_after", "failed"),
+        ],
     )
     format_group.add_argument(
         "-v",
         "--verbose",
-        dest="showOutput",
         help="For failed tests, show all output. For example, each command is"
         " printed before it is executed, so the last printed command is the one"
-        " that failed.",
-        action="store_true",
+        " that failed. Alias for '--test-output=failed'",
+        action=AliasAction,
+        alias=[
+            lambda namespace: TestOutputAction.setOutputLevel(namespace, "test_output", "failed"),
+        ],
     )
     format_group.add_argument(
         "-vv",
         "--echo-all-commands",
-        dest="showOutput",
         help="Deprecated alias for -v.",
-        action="store_true",
+        action=AliasAction,
+        alias=[
+            lambda namespace: TestOutputAction.setOutputLevel(namespace, "test_output", "all"),
+        ],
     )
     format_group.add_argument(
         "-a",
         "--show-all",
-        dest="showAllOutput",
-        help="Enable -v, but for all tests not just failed tests.",
-        action="store_true",
+        help="Enable -v, but for all tests not just failed tests. Alias for '--test-output=all'",
+        action=AliasAction,
+        alias=[
+            lambda namespace: TestOutputAction.setOutputLevel(namespace, "test_output", "all"),
+        ],
     )
     format_group.add_argument(
         "-r",
@@ -105,10 +202,16 @@ def parse_args():
         help="Write test results to the provided path",
         metavar="PATH",
     )
+    format_group.add_argument(
+        "--progress-bar",
+        dest="useProgressBar",
+        help="Show curses based progress bar",
+        action="store_true",
+    )
     format_group.add_argument(
         "--no-progress-bar",
         dest="useProgressBar",
-        help="Do not use curses based progress bar",
+        help="Do not use curses based progress bar (default)",
         action="store_false",
     )
 
@@ -395,6 +498,8 @@ def parse_args():
 
     for report in opts.reports:
         report.use_unique_output_file_name = opts.use_unique_output_file_name
+    #print("test_output", opts.test_output)
+    #print("print_result_after", opts.print_result_after)
 
     return opts
 

llvm/utils/lit/lit/discovery.py

@@ -63,7 +63,7 @@ def search1(path):
 
         # We found a test suite, create a new config for it and load it.
         if litConfig.debug:
-            litConfig.note("loading suite config %r" % cfgpath)
+            litConfig.dbg("loading suite config %r" % cfgpath)
 
         cfg = TestingConfig.fromdefaults(litConfig)
         cfg.load_from_path(cfgpath, litConfig)
@@ -116,7 +116,7 @@ def search1(path_in_suite):
         # file into it.
         config = copy.deepcopy(parent)
         if litConfig.debug:
-            litConfig.note("loading local config %r" % cfgpath)
+            litConfig.dbg("loading local config %r" % cfgpath)
         config.load_from_path(cfgpath, litConfig)
         return config
 
@@ -138,7 +138,7 @@ def getTests(path, litConfig, testSuiteCache, localConfigCache):
         return (), ()
 
     if litConfig.debug:
-        litConfig.note("resolved input %r to %r::%r" % (path, ts.name, path_in_suite))
+        litConfig.dbg("resolved input %r to %r::%r" % (path, ts.name, path_in_suite))
 
     return ts, getTestsInSuite(
         ts,