From 0b4f40ab7917d6201766e916a5fb8c1908c2741f Mon Sep 17 00:00:00 2001
From: Tamara Dahlgren <35777542+tldahlgren@users.noreply.github.com>
Date: Wed, 23 Feb 2022 18:36:21 -0800
Subject: Testing: Summarize test results and add verbose output (#28700)

---
 lib/spack/spack/cmd/test.py        | 11 +++++++++++
 lib/spack/spack/install_test.py    | 18 +++++++++++++++---
 lib/spack/spack/package.py         | 31 ++++++++++++++++++++++--------
 lib/spack/spack/report.py          |  3 +--
 lib/spack/spack/test/cmd/test.py   | 39 ++++++++++++++++++++++++++++++++++++++
 lib/spack/spack/test/test_suite.py | 17 +++++++++++++++++
 6 files changed, 106 insertions(+), 13 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/cmd/test.py b/lib/spack/spack/cmd/test.py
index ddb4990e4c..30679d438a 100644
--- a/lib/spack/spack/cmd/test.py
+++ b/lib/spack/spack/cmd/test.py
@@ -337,9 +337,17 @@ def _report_suite_results(test_suite, args, constraints):
                 pkg_id, status = line.split()
                 results[pkg_id] = status
 
+        failed, skipped, untested = 0, 0, 0
         for pkg_id in test_specs:
             if pkg_id in results:
                 status = results[pkg_id]
+                if status == 'FAILED':
+                    failed += 1
+                elif status == 'NO-TESTS':
+                    untested += 1
+                elif status == 'SKIPPED':
+                    skipped += 1
+
                 if args.failed and status != 'FAILED':
                     continue
 
@@ -351,6 +359,9 @@ def _report_suite_results(test_suite, args, constraints):
                         with open(log_file, 'r') as f:
                             msg += '\n{0}'.format(''.join(f.readlines()))
                 tty.msg(msg)
+
+        spack.install_test.write_test_summary(
+            failed, skipped, untested, len(test_specs))
     else:
         msg = "Test %s has no results.\n" % test_suite.name
         msg += "        Check if it is running with "
diff --git a/lib/spack/spack/install_test.py b/lib/spack/spack/install_test.py
index 1857f26e6d..71561d28a6 100644
--- a/lib/spack/spack/install_test.py
+++ b/lib/spack/spack/install_test.py
@@ -94,6 +94,16 @@ def write_test_suite_file(suite):
         sjson.dump(suite.to_dict(), stream=f)
 
 
+def write_test_summary(num_failed, num_skipped, num_untested, num_specs):
+    failed = "{0} failed, ".format(num_failed) if num_failed else ''
+    skipped = "{0} skipped, ".format(num_skipped) if num_skipped else ''
+    no_tests = "{0} no-tests, ".format(num_untested) if num_untested else ''
+    num_passed = num_specs - num_failed - num_untested - num_skipped
+
+    print("{:=^80}".format(" {0}{1}{2}{3} passed of {4} specs "
+          .format(failed, no_tests, skipped, num_passed, num_specs)))
+
+
 class TestSuite(object):
     def __init__(self, specs, alias=None):
         # copy so that different test suites have different package objects
@@ -130,6 +140,7 @@ class TestSuite(object):
         fail_first = kwargs.get('fail_first', False)
         externals = kwargs.get('externals', False)
 
+        skipped, untested = 0, 0
         for spec in self.specs:
             try:
                 if spec.package.test_suite:
@@ -165,11 +176,10 @@ class TestSuite(object):
                     self.ensure_stage()
                     if spec.external and not externals:
                         status = 'SKIPPED'
-                        msg = 'Skipped external package'
+                        skipped += 1
                     else:
                         status = 'NO-TESTS'
-                        msg = 'No tests to run'
-                    _add_msg_to_file(self.log_file_for_spec(spec), msg)
+                        untested += 1
 
                 self.write_test_result(spec, status)
             except BaseException as exc:
@@ -189,6 +199,8 @@ class TestSuite(object):
                 self.current_test_spec = None
                 self.current_base_spec = None
 
+        write_test_summary(self.fails, skipped, untested, len(self.specs))
+
         if self.fails:
             raise TestSuiteFailure(self.fails)
 
diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index c30747edeb..3bd52eb817 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -1813,13 +1813,12 @@ class PackageBase(six.with_metaclass(PackageMeta, PackageViewMixin, object)):
         self.tested_file = self.test_suite.tested_file_for_spec(self.spec)
         fsys.touch(self.test_log_file)  # Otherwise log_parse complains
 
-        if self.spec.external and not externals:
-            with open(self.test_log_file, 'w') as ofd:
-                ofd.write('Testing package {0}\n'
-                          .format(self.test_suite.test_pkg_id(self.spec)))
-            return
-
-        kwargs = {'dirty': dirty, 'fake': False, 'context': 'test'}
+        kwargs = {
+            'dirty': dirty, 'fake': False, 'context': 'test',
+            'externals': externals
+        }
+        if tty.is_verbose():
+            kwargs['verbose'] = True
         spack.build_environment.start_build_process(self, test_process, kwargs)
 
     def test(self):
@@ -2644,12 +2643,26 @@ def has_test_method(pkg):
     )
 
 
+def print_test_message(logger, msg, verbose):
+    if verbose:
+        with logger.force_echo():
+            print(msg)
+    else:
+        print(msg)
+
+
 def test_process(pkg, kwargs):
-    with tty.log.log_output(pkg.test_log_file) as logger:
+    verbose = kwargs.get('verbose', False)
+    externals = kwargs.get('externals', False)
+    with tty.log.log_output(pkg.test_log_file, verbose) as logger:
         with logger.force_echo():
             tty.msg('Testing package {0}'
                     .format(pkg.test_suite.test_pkg_id(pkg.spec)))
 
+        if pkg.spec.external and not externals:
+            print_test_message(logger, 'Skipped external package', verbose)
+            return
+
         # use debug print levels for log file to record commands
         old_debug = tty.is_debug()
         tty.set_debug(True)
@@ -2730,6 +2743,8 @@ def test_process(pkg, kwargs):
             # non-pass-only methods
             if ran_actual_test_function:
                 fsys.touch(pkg.tested_file)
+            else:
+                print_test_message(logger, 'No tests to run',  verbose)
 
 
 inject_flags = PackageBase.inject_flags
diff --git a/lib/spack/spack/report.py b/lib/spack/spack/report.py
index 8697d98f5e..a31c011fc4 100644
--- a/lib/spack/spack/report.py
+++ b/lib/spack/spack/report.py
@@ -176,10 +176,9 @@ class InfoCollector(object):
                     skip_externals = pkg.spec.external and not externals
                     if do_fn.__name__ == 'do_test' and skip_externals:
                         package['result'] = 'skipped'
-                        package['stdout'] = 'Skipped external package'
                     else:
                         package['result'] = 'success'
-                        package['stdout'] = fetch_log(pkg, do_fn, self.dir)
+                    package['stdout'] = fetch_log(pkg, do_fn, self.dir)
                     package['installed_from_binary_cache'] = \
                         pkg.installed_from_binary_cache
                     if do_fn.__name__ == '_install_task' and installed_already:
diff --git a/lib/spack/spack/test/cmd/test.py b/lib/spack/spack/test/cmd/test.py
index 46b4477cb3..aad8389a03 100644
--- a/lib/spack/spack/test/cmd/test.py
+++ b/lib/spack/spack/test/cmd/test.py
@@ -211,6 +211,7 @@ def test_test_list_all(mock_packages):
         "printing-package",
         "py-extension1",
         "py-extension2",
+        "simple-standalone-test",
         "test-error",
         "test-fail",
     ])
@@ -251,3 +252,41 @@ def test_hash_change(mock_test_stage, mock_packages, mock_archive, mock_fetch,
     # The results should be obtainable
     results_output = spack_test('results')
     assert 'PASSED' in results_output
+
+
+def test_test_results_none(mock_packages, mock_test_stage):
+    name = 'trivial'
+    spec = spack.spec.Spec('trivial-smoke-test').concretized()
+    suite = spack.install_test.TestSuite([spec], name)
+    suite.ensure_stage()
+    spack.install_test.write_test_suite_file(suite)
+    results = spack_test('results', name)
+    assert 'has no results' in results
+    assert 'if it is running' in results
+
+
+@pytest.mark.parametrize('status,expected', [
+    ('FAILED', '1 failed'),
+    ('NO-TESTS', '1 no-tests'),
+    ('SKIPPED', '1 skipped'),
+    ('PASSED', '1 passed'),
+])
+def test_test_results_status(mock_packages, mock_test_stage, status, expected):
+    name = 'trivial'
+    spec = spack.spec.Spec('trivial-smoke-test').concretized()
+    suite = spack.install_test.TestSuite([spec], name)
+    suite.ensure_stage()
+    spack.install_test.write_test_suite_file(suite)
+    suite.write_test_result(spec, status)
+
+    for opt in ['', '--failed', '--log']:
+        args = ['results', name]
+        if opt:
+            args.insert(1, opt)
+
+        results = spack_test(*args)
+        if opt == '--failed' and status != 'FAILED':
+            assert status not in results
+        else:
+            assert status in results
+        assert expected in results
diff --git a/lib/spack/spack/test/test_suite.py b/lib/spack/spack/test/test_suite.py
index 7641dfcb0b..7235eb9453 100644
--- a/lib/spack/spack/test/test_suite.py
+++ b/lib/spack/spack/test/test_suite.py
@@ -149,6 +149,23 @@ def test_test_spec_run_once(mock_packages, install_mockery, mock_test_stage):
         test_suite()
 
 
+def test_test_spec_verbose(mock_packages, install_mockery, mock_test_stage):
+    spec = spack.spec.Spec('simple-standalone-test').concretized()
+    test_suite = spack.install_test.TestSuite([spec])
+
+    test_suite(verbose=True)
+    passed, msg = False, False
+    with open(test_suite.log_file_for_spec(spec), 'r') as fd:
+        for line in fd:
+            if 'simple stand-alone test' in line:
+                msg = True
+            elif 'PASSED' in line:
+                passed = True
+
+    assert msg
+    assert passed
+
+
 def test_get_test_suite():
     assert not spack.install_test.get_test_suite('nothing')
 
-- 
cgit v1.2.3-70-g09d2