JDevlieghere created this revision.
JDevlieghere added a reviewer: LLDB.
Herald added a subscriber: teemperor.
Herald added a project: LLDB.

I propose removing the `-q` (quiet) flag and changing the default behavior. 
Currently the flag serves two purposes that are somewhat contradictory, as 
illustrated by the difference between the argument name (quiet) and the 
configuration flag (parsable). On the one hand it reduces output, but on the 
other hand it prints more output, like the result of individual tests. My 
proposal is to guard the extra output behind the verbose flag and always print 
the individual test results.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D66837

Files:
  lldb/docs/resources/test.rst
  lldb/lit/Suite/lit.cfg
  lldb/packages/Python/lldbsuite/test/configuration.py
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/packages/Python/lldbsuite/test/dotest_args.py
  lldb/packages/Python/lldbsuite/test/test_result.py
  lldb/utils/lldb-dotest/lldb-dotest.in

Index: lldb/utils/lldb-dotest/lldb-dotest.in
===================================================================
--- lldb/utils/lldb-dotest/lldb-dotest.in
+++ lldb/utils/lldb-dotest/lldb-dotest.in
@@ -9,7 +9,7 @@
     wrapper_args = sys.argv[1:]
     dotest_args = dotest_args_str.split(';')
     # Build dotest.py command.
-    cmd = [sys.executable, dotest_path, '-q']
+    cmd = [sys.executable, dotest_path]
     cmd.extend(dotest_args)
     cmd.extend(wrapper_args)
     # Invoke dotest.py and return exit code.
Index: lldb/packages/Python/lldbsuite/test/test_result.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/test_result.py
+++ lldb/packages/Python/lldbsuite/test/test_result.py
@@ -180,10 +180,9 @@
             return
 
         super(LLDBTestResult, self).addSuccess(test)
-        if configuration.parsable:
-            self.stream.write(
-                "PASS: LLDB (%s) :: %s\n" %
-                (self._config_string(test), str(test)))
+        self.stream.write(
+            "PASS: LLDB (%s) :: %s\n" %
+            (self._config_string(test), str(test)))
         if self.results_formatter:
             self.results_formatter.handle_event(
                 EventBuilder.event_for_success(test))
@@ -220,10 +219,9 @@
         method = getattr(test, "markError", None)
         if method:
             method()
-        if configuration.parsable:
-            self.stream.write(
-                "FAIL: LLDB (%s) :: %s\n" %
-                (self._config_string(test), str(test)))
+        self.stream.write(
+            "FAIL: LLDB (%s) :: %s\n" %
+            (self._config_string(test), str(test)))
         if self.results_formatter:
             # Handle build errors as a separate event type
             if self._isBuildError(err):
@@ -238,10 +236,9 @@
         method = getattr(test, "markCleanupError", None)
         if method:
             method()
-        if configuration.parsable:
-            self.stream.write(
-                "CLEANUP ERROR: LLDB (%s) :: %s\n" %
-                (self._config_string(test), str(test)))
+        self.stream.write(
+            "CLEANUP ERROR: LLDB (%s) :: %s\n" %
+            (self._config_string(test), str(test)))
         if self.results_formatter:
             self.results_formatter.handle_event(
                 EventBuilder.event_for_cleanup_error(
@@ -258,10 +255,9 @@
         method = getattr(test, "markFailure", None)
         if method:
             method()
-        if configuration.parsable:
-            self.stream.write(
-                "FAIL: LLDB (%s) :: %s\n" %
-                (self._config_string(test), str(test)))
+        self.stream.write(
+            "FAIL: LLDB (%s) :: %s\n" %
+            (self._config_string(test), str(test)))
         if configuration.useCategories:
             test_categories = self.getCategoriesForTest(test)
             for category in test_categories:
@@ -280,10 +276,9 @@
         method = getattr(test, "markExpectedFailure", None)
         if method:
             method(err, bugnumber)
-        if configuration.parsable:
-            self.stream.write(
-                "XFAIL: LLDB (%s) :: %s\n" %
-                (self._config_string(test), str(test)))
+        self.stream.write(
+            "XFAIL: LLDB (%s) :: %s\n" %
+            (self._config_string(test), str(test)))
         if self.results_formatter:
             self.results_formatter.handle_event(
                 EventBuilder.event_for_expected_failure(
@@ -295,10 +290,9 @@
         method = getattr(test, "markSkippedTest", None)
         if method:
             method()
-        if configuration.parsable:
-            self.stream.write(
-                "UNSUPPORTED: LLDB (%s) :: %s (%s) \n" %
-                (self._config_string(test), str(test), reason))
+        self.stream.write(
+            "UNSUPPORTED: LLDB (%s) :: %s (%s) \n" %
+            (self._config_string(test), str(test), reason))
         if self.results_formatter:
             self.results_formatter.handle_event(
                 EventBuilder.event_for_skip(test, reason))
@@ -309,10 +303,9 @@
         method = getattr(test, "markUnexpectedSuccess", None)
         if method:
             method(bugnumber)
-        if configuration.parsable:
-            self.stream.write(
-                "XPASS: LLDB (%s) :: %s\n" %
-                (self._config_string(test), str(test)))
+        self.stream.write(
+            "XPASS: LLDB (%s) :: %s\n" %
+            (self._config_string(test), str(test)))
         if self.results_formatter:
             self.results_formatter.handle_event(
                 EventBuilder.event_for_unexpected_success(
Index: lldb/packages/Python/lldbsuite/test/dotest_args.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/dotest_args.py
+++ lldb/packages/Python/lldbsuite/test/dotest_args.py
@@ -191,7 +191,6 @@
     # Test-suite behaviour
     group = parser.add_argument_group('Runtime behaviour options')
     X('-d', 'Suspend the process after launch to wait indefinitely for a debugger to attach')
-    X('-q', "Don't print extra output from this script.")
     X('-t', 'Turn on tracing of lldb command and other detailed test executions')
     group.add_argument(
         '-u',
Index: lldb/packages/Python/lldbsuite/test/dotest.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -252,7 +252,7 @@
     if args.set_inferior_env_vars:
         lldbtest_config.inferior_env = ' '.join(args.set_inferior_env_vars)
 
-    # only print the args if being verbose (and parsable is off)
+    # Only print the args if being verbose.
     if args.v and not args.q:
         print(sys.argv)
 
@@ -393,9 +393,6 @@
             usage(parser)
         configuration.regexp = args.p
 
-    if args.q:
-        configuration.parsable = True
-
     if args.s:
         if args.s.startswith('-'):
             usage(parser)
@@ -1268,7 +1265,7 @@
     #
     checkCompiler()
 
-    if not configuration.parsable:
+    if configuration.verbose:
         print("compiler=%s" % configuration.compiler)
 
     # Iterating over all possible architecture and compiler combinations.
@@ -1286,27 +1283,22 @@
     configPostfix = configString.translate(tbl)
 
     # Output the configuration.
-    if not configuration.parsable:
+    if configuration.verbose:
         sys.stderr.write("\nConfiguration: " + configString + "\n")
 
     # First, write out the number of collected test cases.
-    if not configuration.parsable:
+    if configuration.verbose:
         sys.stderr.write(configuration.separator + "\n")
         sys.stderr.write(
             "Collected %d test%s\n\n" %
             (configuration.suite.countTestCases(),
              configuration.suite.countTestCases() != 1 and "s" or ""))
 
-    if configuration.parsable:
-        v = 0
-    else:
-        v = configuration.verbose
-
     # Invoke the test runner.
     if configuration.count == 1:
         result = unittest2.TextTestRunner(
             stream=sys.stderr,
-            verbosity=v,
+            verbosity=configuration.verbose,
             resultclass=test_result.LLDBTestResult).run(
             configuration.suite)
     else:
@@ -1318,13 +1310,13 @@
 
             result = unittest2.TextTestRunner(
                 stream=sys.stderr,
-                verbosity=v,
+                verbosity=configuration.verbose,
                 resultclass=test_result.LLDBTestResult).run(
                 configuration.suite)
 
     configuration.failed = not result.wasSuccessful()
 
-    if configuration.sdir_has_content and not configuration.parsable:
+    if configuration.sdir_has_content and configuration.verbose:
         sys.stderr.write(
             "Session logs for test failures/errors/unexpected successes"
             " can be found in directory '%s'\n" %
Index: lldb/packages/Python/lldbsuite/test/configuration.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/configuration.py
+++ lldb/packages/Python/lldbsuite/test/configuration.py
@@ -57,10 +57,6 @@
 # The filters (testclass.testmethod) used to admit tests into our test suite.
 filters = []
 
-# Parsable mode silences headers, and any other output this script might generate, and instead
-# prints machine-readable output similar to what clang tests produce.
-parsable = False
-
 # The regular expression pattern to match against eligible filenames as
 # our test cases.
 regexp = None
Index: lldb/lit/Suite/lit.cfg
===================================================================
--- lldb/lit/Suite/lit.cfg
+++ lldb/lit/Suite/lit.cfg
@@ -54,7 +54,7 @@
                      .format(platform.system()))
 
 # Build dotest command.
-dotest_cmd = [config.dotest_path, '-q']
+dotest_cmd = [config.dotest_path]
 dotest_cmd.extend(config.dotest_args_str.split(';'))
 
 # We don't want to force users passing arguments to lit to use `;` as a
Index: lldb/docs/resources/test.rst
===================================================================
--- lldb/docs/resources/test.rst
+++ lldb/docs/resources/test.rst
@@ -144,8 +144,6 @@
 
 ::
 
-   # quiet mode
-   -q
    --arch=i686
    # Path to debug lldb.exe
    --executable D:/src/llvmbuild/ninja/bin/lldb.exe
@@ -163,7 +161,7 @@
 
 ::
 
-   -q --arch=i686 --executable D:/src/llvmbuild/ninja/bin/lldb.exe -s D:/src/llvmbuild/ninja/lldb-test-traces -u CXXFLAGS -u CFLAGS --enable-crash-dialog -C d:\src\llvmbuild\ninja_release\bin\clang.exe -p TestPaths.py D:\src\llvm\tools\lldb\packages\Python\lldbsuite\test --no-multiprocess
+   --arch=i686 --executable D:/src/llvmbuild/ninja/bin/lldb.exe -s D:/src/llvmbuild/ninja/lldb-test-traces -u CXXFLAGS -u CFLAGS --enable-crash-dialog -C d:\src\llvmbuild\ninja_release\bin\clang.exe -p TestPaths.py D:\src\llvm\tools\lldb\packages\Python\lldbsuite\test --no-multiprocess
 
 
 
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
  • [Lldb-commits] [PATCH] ... Jonas Devlieghere via Phabricator via lldb-commits

Reply via email to