Smoke tests: handle package syntax errors (#17919)

lib/spack/spack/cmd/test.py

@@ -172,13 +172,18 @@ def test_run(args):
                         name=test_name,
                         remove_directory=not args.keep_stage,
                         dirty=args.dirty)
-                    with open(_get_results_file(test_name), 'a') as f:
-                        f.write("%s PASSED\n" %
-                                spec.format("{name}-{version}-{hash:7}"))
-                except BaseException:
-                    with open(_get_results_file(test_name), 'a') as f:
-                        f.write("%s FAILED\n" %
-                                spec.format("{name}-{version}-{hash:7}"))
+                    _write_test_result(spec, test_name, 'PASSED')
+                except BaseException as err:
+                    if isinstance(err, SyntaxError):
+                        # Create the test log file and report the error.
+                        test_stage = spack.package.setup_test_stage(test_name)
+                        logfile = spack.package.test_log_pathname(test_stage,
+                                                                  spec)
+                        msg = 'Testing package {0}\n{1}'\
+                            .format(spack.package.test_pkg_id(spec), str(err))
+                        _add_msg_to_file(logfile, msg)
+
+                    _write_test_result(spec, test_name, 'FAILED')
                     if args.fail_first:
                         break
         else:
@@ -278,9 +283,36 @@ def get_stage(name=None):
     return os.path.join(stage_dir, name) if name else stage_dir
 
 
-def _get_results_file(name):
+def _add_msg_to_file(filename, msg):
+    """Add the message to the specified file
+
+    Args:
+        filename (str): path to the file
+        msg (str): message to be appended to the file
     """
-    Return the results file for the named test.
+    with open(filename, 'a+') as f:
+        f.write('{0}\n'.format(msg))
+
+
+def _get_results_file(test_name):
+    """Return the results pathname for the test
+
+    Args:
+        test_name (str): name of the test
+
+    Returns:
+        (str): path to the test results file
     """
-    stage = get_stage(name)
-    return os.path.join(stage, 'results.txt')
+    return os.path.join(get_stage(test_name), 'results.txt')
+
+
+def _write_test_result(spec, test_name, result):
+    """Write the result to the test results file
+
+    Args:
+        spec (Spec): spec of the package under test
+        test_name (str): name of the test
+        result (str): test result (i.e., 'PASSED' or 'FAILED')
+    """
+    msg = "{0} {1}".format(spack.package.test_pkg_id(spec), result)
+    _add_msg_to_file(_get_results_file(test_name), msg)

lib/spack/spack/package.py

@@ -444,6 +444,47 @@ def remove_files_from_view(self, view, merge_map):
             view.remove_file(src, dst)
 
 
+def test_pkg_id(spec):
+    """Build the standard install test package identifier
+
+    Args:
+        spec (Spec): instance of the spec under test
+
+    Returns:
+        (str): the install test package identifier
+    """
+    return spec.format('{name}-{version}-{hash:7}')
+
+
+def test_log_pathname(test_stage, spec):
+    """Build the pathname of the test log file
+
+    Args:
+        test_stage (str): path to the test stage directory
+        spec (Spec): instance of the spec under test
+
+    Returns:
+        (str): the pathname of the test log file
+    """
+    return os.path.join(test_stage,
+                        'test-{0}-out.txt'.format(test_pkg_id(spec)))
+
+
+def setup_test_stage(test_name):
+    """Set up the test stage directory.
+
+    Args:
+        test_name (str):  the name of the test
+
+    Returns:
+        (str): the path to the test stage directory
+    """
+    test_stage = Prefix(spack.cmd.test.get_stage(test_name))
+    if not os.path.exists(test_stage):
+        fsys.mkdirp(test_stage)
+    return test_stage
+
+
 class PackageBase(six.with_metaclass(PackageMeta, PackageViewMixin, object)):
     """This is the superclass for all spack packages.
 
@@ -1654,16 +1695,14 @@ def do_test(self, name, remove_directory=False, dirty=False):
         # Clear test failures
         self.test_failures = []
 
-        self.test_stage = Prefix(spack.cmd.test.get_stage(name))
-        if not os.path.exists(self.test_stage):
-            fsys.mkdirp(self.test_stage)
-        self.test_log_file = os.path.join(self.test_stage, self.test_log_name)
+        self.test_stage = setup_test_stage(name)
+        self.test_log_file = test_log_pathname(self.test_stage, self.spec)
 
         def test_process():
             with tty.log.log_output(self.test_log_file) as logger:
                 with logger.force_echo():
-                    tty.msg('Testing package %s' %
-                            self.spec.format('{name}-{version}-{hash:7}'))
+                    tty.msg('Testing package {0}'
+                            .format(test_pkg_id(self.spec)))
 
                 # use debug print levels for log file to record commands
                 old_debug = tty.is_debug()

lib/spack/spack/repo.py

@@ -1077,7 +1077,8 @@ def _get_pkg_module(self, pkg_name):
                 # manually construct the error message in order to give the
                 # user the correct package.py where the syntax error is located
                 raise SyntaxError('invalid syntax in {0:}, line {1:}'
-                                  ''.format(file_path, e.lineno))
+                                  .format(file_path, e.lineno))
+
             module.__package__ = self.full_namespace
             module.__loader__ = self
             self._modules[pkg_name] = module

lib/spack/spack/test/cmd/test.py

@@ -8,6 +8,8 @@
 
 import pytest
 
+from llnl.util.filesystem import mkdirp
+
 import spack.config
 import spack.package
 import spack.cmd.install
@@ -30,7 +32,7 @@ def mock_test_stage(mutable_config, tmpdir):
 
 def test_test_package_not_installed(
         tmpdir, mock_packages, mock_archive, mock_fetch, config,
-        install_mockery_mutable_config):
+        install_mockery_mutable_config, mock_test_stage):
 
     output = spack_test('run', 'libdwarf')
 
@@ -178,7 +180,7 @@ def test_cdash_upload_clean_test(
         assert '<Text>' not in content
 
 
-def test_test_help_does_not_show_cdash_options(capsys):
+def test_test_help_does_not_show_cdash_options(mock_test_stage, capsys):
     """Make sure `spack test --help` does not describe CDash arguments"""
     with pytest.raises(SystemExit):
         spack_test('run', '--help')
@@ -186,7 +188,49 @@ def test_test_help_does_not_show_cdash_options(capsys):
         assert 'CDash URL' not in captured.out
 
 
-def test_test_help_cdash():
+def test_test_help_cdash(mock_test_stage):
     """Make sure `spack test --help-cdash` describes CDash arguments"""
     out = spack_test('run', '--help-cdash')
     assert 'CDash URL' in out
+
+
+def test_test_log_pathname(mock_packages, config):
+    """Ensure test log path is reasonable."""
+    spec_name = 'libdwarf'
+    spec = spack.spec.Spec(spec_name).concretized()
+
+    test_stage = 'log_test_stage'
+    logfile = spack.package.test_log_pathname(test_stage, spec)
+    assert test_stage in logfile
+    assert spec_name in logfile
+    assert 'out.txt' in logfile
+
+
+def test_setup_test_stage(mock_test_stage):
+    """Make sure test stage directory is properly set up."""
+    test_mock_stage = mock_test_stage
+    test_name = 'test-name'
+    test_stage = spack.package.setup_test_stage(test_name)
+
+    assert os.path.exists(test_mock_stage)
+    assert test_mock_stage in test_stage
+    assert test_stage.endswith(test_name)
+
+
+def test_write_test_result(mock_packages, mock_test_stage):
+    """Ensure test results written to a results file."""
+    result = 'TEST'
+    spec_name = 'libdwarf'
+    test_name = 'write-test'
+
+    spec = spack.spec.Spec(spec_name).concretized()
+    results_fn = spack.cmd.test._get_results_file(test_name)
+    mkdirp(os.path.dirname(results_fn))
+    spack.cmd.test._write_test_result(spec, test_name, result)
+    with open(results_fn, 'r') as ifd:
+        lines = ifd.readlines()
+        assert len(lines) == 1
+
+        msg = lines[0]
+        assert result in msg
+        assert spec_name in msg