From 40e91713908c1ef96d7b83d8b0dfbbd69bc24e4c Mon Sep 17 00:00:00 2001
From: Todd Gamblin <tgamblin@llnl.gov>
Date: Tue, 22 Aug 2017 14:38:39 -0700
Subject: [PATCH] Add testing for new build output.

- Update handling of ChildError so that its output is capturable from a
  SpackCommand

- Update cmd/install test to make sure Python and build log output is
  being displayed properly.
---
 lib/spack/spack/build_environment.py |  9 +++++++++
 lib/spack/spack/error.py             | 23 ++++++++++++++++++++++-
 lib/spack/spack/test/cmd/install.py  | 25 ++++++++++++++++++++++++-
 3 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py
index ad27b3321d..896cbe879b 100644
--- a/lib/spack/spack/build_environment.py
+++ b/lib/spack/spack/build_environment.py
@@ -610,8 +610,14 @@ def child_process(child_pipe, input_stream):
     child_result = parent_pipe.recv()
     p.join()
 
+    # If the child process raised an error, print its output here rather
+    # than waiting until the call to SpackError.die() in main(). This
+    # allows exception handling output to be logged from within Spack.
+    # see spack.main.SpackCommand.
     if isinstance(child_result, ChildError):
+        child_result.print_context()
         raise child_result
+
     return child_result
 
 
@@ -753,6 +759,9 @@ def long_message(self):
 
         return out.getvalue()
 
+    def __str__(self):
+        return self.message + self.long_message + self.traceback
+
     def __reduce__(self):
         """__reduce__ is used to serialize (pickle) ChildErrors.
 
diff --git a/lib/spack/spack/error.py b/lib/spack/spack/error.py
index 6e48a4e76c..8f54e82b11 100644
--- a/lib/spack/spack/error.py
+++ b/lib/spack/spack/error.py
@@ -45,11 +45,27 @@ def __init__(self, message, long_message=None):
         # traceback as a string and print it in the parent.
         self.traceback = None
 
+        # we allow exceptions to print debug info via print_context()
+        # before they are caught at the top level. If they *haven't*
+        # printed context early, we do it by default when die() is
+        # called, so we need to remember whether it's been called.
+        self.printed = False
+
     @property
     def long_message(self):
         return self._long_message
 
-    def die(self):
+    def print_context(self):
+        """Print extended debug information about this exception.
+
+        This is usually printed when the top-level Spack error handler
+        calls ``die()``, but it acn be called separately beforehand if a
+        lower-level error handler needs to print error context and
+        continue without raising the exception to the top level.
+        """
+        if self.printed:
+            return
+
         # basic debug message
         tty.error(self.message)
         if self.long_message:
@@ -66,6 +82,11 @@ def die(self):
                 # run parent exception hook.
                 sys.excepthook(*sys.exc_info())
 
+        sys.stderr.flush()
+        self.printed = True
+
+    def die(self):
+        self.print_context()
         sys.exit(1)
 
     def __str__(self):
diff --git a/lib/spack/spack/test/cmd/install.py b/lib/spack/spack/test/cmd/install.py
index 1b8549ca3d..eef5605275 100644
--- a/lib/spack/spack/test/cmd/install.py
+++ b/lib/spack/spack/test/cmd/install.py
@@ -27,6 +27,7 @@
 
 import pytest
 
+import spack
 import spack.cmd.install
 from spack.spec import Spec
 from spack.main import SpackCommand
@@ -42,7 +43,7 @@ def parser():
     return parser
 
 
-def _install_package_and_dependency(
+def test_install_package_and_dependency(
         tmpdir, builtin_mock, mock_archive, mock_fetch, config,
         install_mockery):
 
@@ -58,6 +59,9 @@ def _install_package_and_dependency(
     assert 'failures="0"' in content
     assert 'errors="0"' in content
 
+    s = Spec('libdwarf').concretized()
+    assert not spack.repo.get(s).stage.created
+
 
 def test_install_package_already_installed(
         tmpdir, builtin_mock, mock_archive, mock_fetch, config,
@@ -107,3 +111,22 @@ def test_package_output(tmpdir, capsys, install_mockery, mock_fetch):
     # right place in the build log.
     assert "BEFORE INSTALL\n==> './configure'" in out
     assert "'install'\nAFTER INSTALL" in out
+
+
+def _test_install_output_on_build_error(builtin_mock, mock_archive, mock_fetch,
+                                        config, install_mockery, capfd):
+    # capfd interferes with Spack's capturing
+    with capfd.disabled():
+        out = install('build-error', fail_on_error=False)
+    assert isinstance(install.error, spack.build_environment.ChildError)
+    assert install.error.name == 'ProcessError'
+    assert 'configure: error: in /path/to/some/file:' in out
+    assert 'configure: error: cannot run C compiled programs.' in out
+
+
+def test_install_output_on_python_error(builtin_mock, mock_archive, mock_fetch,
+                                        config, install_mockery):
+    out = install('failing-build', fail_on_error=False)
+    assert isinstance(install.error, spack.build_environment.ChildError)
+    assert install.error.name == 'InstallError'
+    assert 'raise InstallError("Expected failure.")' in out