From b7249d66b3b47f710ddbb1719f433b507322b5a3 Mon Sep 17 00:00:00 2001
From: Peter Scheibel <scheibel1@llnl.gov>
Date: Mon, 12 Oct 2015 19:09:11 -0700
Subject: [PATCH 01/26] Adding command testinstall. See "spack testinstall -h"
 for documentation. Still need to add output formatting (in a commonly
 parse-able format like Junit or TAP). May want to adjust how the build log is
 accessed in case of a build failure.

---
 lib/spack/spack/cmd/testinstall.py | 129 +++++++++++++++++++++++++++++
 1 file changed, 129 insertions(+)
 create mode 100644 lib/spack/spack/cmd/testinstall.py

diff --git a/lib/spack/spack/cmd/testinstall.py b/lib/spack/spack/cmd/testinstall.py
new file mode 100644
index 00000000000..d3a2cae3c2a
--- /dev/null
+++ b/lib/spack/spack/cmd/testinstall.py
@@ -0,0 +1,129 @@
+##############################################################################
+# Copyright (c) 2013, Lawrence Livermore National Security, LLC.
+# Produced at the Lawrence Livermore National Laboratory.
+#
+# This file is part of Spack.
+# Written by Todd Gamblin, tgamblin@llnl.gov, All rights reserved.
+# LLNL-CODE-647188
+#
+# For details, see https://scalability-llnl.github.io/spack
+# Please also see the LICENSE file for our notice and the LGPL.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License (as published by
+# the Free Software Foundation) version 2.1 dated February 1999.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the IMPLIED WARRANTY OF
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the terms and
+# conditions of the GNU General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public License
+# along with this program; if not, write to the Free Software Foundation,
+# Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+##############################################################################
+from external import argparse
+import xml.etree.ElementTree as ET
+
+import llnl.util.tty as tty
+from llnl.util.filesystem import *
+
+import spack
+import spack.cmd
+
+description = "Build and install packages"
+
+def setup_parser(subparser):
+    #subparser.add_argument(
+    #    '-i', '--ignore-dependencies', action='store_true', dest='ignore_deps',
+    #    help="Do not try to install dependencies of requested packages.")
+    
+    subparser.add_argument(
+        '-j', '--jobs', action='store', type=int,
+        help="Explicitly set number of make jobs.  Default is #cpus.")
+        
+    #always false for test
+    #subparser.add_argument(
+    #    '--keep-prefix', action='store_true', dest='keep_prefix',
+    #    help="Don't remove the install prefix if installation fails.")
+    
+    #always true for test
+    #subparser.add_argument(
+    #    '--keep-stage', action='store_true', dest='keep_stage',
+    #    help="Don't remove the build stage if installation succeeds.")
+    
+    subparser.add_argument(
+        '-n', '--no-checksum', action='store_true', dest='no_checksum',
+        help="Do not check packages against checksum")
+    subparser.add_argument(
+        '-v', '--verbose', action='store_true', dest='verbose',
+        help="Display verbose build output while installing.")
+    
+    #subparser.add_argument(
+    #    '--fake', action='store_true', dest='fake',
+    #    help="Fake install.  Just remove the prefix and touch a fake file in it.")
+    
+    subparser.add_argument(
+        'outputdir', help="test output goes in this directory, 1 file per package")
+    
+    subparser.add_argument(
+        'packages', nargs=argparse.REMAINDER, help="specs of packages to install")
+
+
+class JunitTestResult(object):
+    def __init__(self):
+        self.root = Element('testsuite')
+        self.tests = []
+        
+    def addTest(self, identifier, passed=True, output=None):
+        self.tests.append((identifier, passed, output))
+    
+    def output(self):
+        self.root.set('tests', '{0}'.format(len(self.tests)))
+        
+        
+def testinstall(parser, args):
+    if not args.packages:
+        tty.die("install requires at least one package argument")
+
+    if args.jobs is not None:
+        if args.jobs <= 0:
+            tty.die("The -j option must be a positive integer!")
+
+    if args.no_checksum:
+        spack.do_checksum = False        # TODO: remove this global.
+
+    print "Output to:", args.outputdir
+
+    specs = spack.cmd.parse_specs(args.packages, concretize=True)
+    try:
+        for spec in specs:
+            #import pdb; pdb.set_trace()
+            package = spack.db.get(spec)
+            package.do_install(
+                keep_prefix=False,
+                keep_stage=False,
+                ignore_deps=False,
+                make_jobs=args.jobs,
+                verbose=args.verbose,
+                fake=False)
+    finally:
+        for spec in specs:
+            package = spack.db.get(spec)
+            #import pdb; pdb.set_trace()
+
+            print spec.name
+            print spec.version
+            print spec.dag_hash()
+
+            if package.installed:
+                installLog = spack.install_layout.build_log_path(spec)
+            else:
+                #TODO: search recursively under stage.path instead of only within
+                #    stage.source_path
+                installLog = join_path(package.stage.source_path, 'spack-build.out')            
+
+            with open(installLog, 'rb') as F:
+                for line in F.readlines()[:10]:
+                    print "\t{0}".format(line.strip())
+            

From 6cd22e5786dbf40f1261b9b0410bdafbb6dd6f29 Mon Sep 17 00:00:00 2001
From: Peter Scheibel <scheibel1@llnl.gov>
Date: Mon, 12 Oct 2015 20:49:23 -0700
Subject: [PATCH 02/26] 1. Added Junit XML format 2. Specify output to a file
 vs. a directory 3. Use [1] and [2] to write an XML file tracking success of
 package installs in Junit XML format

---
 lib/spack/spack/cmd/testinstall.py | 50 ++++++++++++++++++++----------
 1 file changed, 34 insertions(+), 16 deletions(-)

diff --git a/lib/spack/spack/cmd/testinstall.py b/lib/spack/spack/cmd/testinstall.py
index d3a2cae3c2a..7ebadbd3444 100644
--- a/lib/spack/spack/cmd/testinstall.py
+++ b/lib/spack/spack/cmd/testinstall.py
@@ -64,24 +64,43 @@ def setup_parser(subparser):
     #    help="Fake install.  Just remove the prefix and touch a fake file in it.")
     
     subparser.add_argument(
-        'outputdir', help="test output goes in this directory, 1 file per package")
+        'output', help="test output goes in this file")
     
     subparser.add_argument(
         'packages', nargs=argparse.REMAINDER, help="specs of packages to install")
 
 
-class JunitTestResult(object):
+class JunitResultFormat(object):
     def __init__(self):
-        self.root = Element('testsuite')
+        self.root = ET.Element('testsuite')
         self.tests = []
         
-    def addTest(self, identifier, passed=True, output=None):
-        self.tests.append((identifier, passed, output))
+    def addTest(self, buildId, passed=True, buildLog=None):
+        self.tests.append((buildId, passed, buildLog))
     
-    def output(self):
+    def writeTo(self, stream):
         self.root.set('tests', '{0}'.format(len(self.tests)))
-        
-        
+        for buildId, passed, buildLog in self.tests:
+            testcase = ET.SubElement(self.root, 'testcase')
+            testcase.set('classname', buildId.name)
+            testcase.set('name', buildId.stringId())
+            if not passed:
+                failure = ET.SubElement(testcase, 'failure')
+                failure.set('type', "Build Error")
+                failure.text = buildLog
+        ET.ElementTree(self.root).write(stream)
+
+
+class BuildId(object):
+    def __init__(self, name, version, hashId):
+        self.name = name
+        self.version = version
+        self.hashId = hashId
+    
+    def stringId(self):
+        return "-".join(str(x) for x in (self.name, self.version, self.hashId))
+
+     
 def testinstall(parser, args):
     if not args.packages:
         tty.die("install requires at least one package argument")
@@ -93,8 +112,6 @@ def testinstall(parser, args):
     if args.no_checksum:
         spack.do_checksum = False        # TODO: remove this global.
 
-    print "Output to:", args.outputdir
-
     specs = spack.cmd.parse_specs(args.packages, concretize=True)
     try:
         for spec in specs:
@@ -108,13 +125,12 @@ def testinstall(parser, args):
                 verbose=args.verbose,
                 fake=False)
     finally:
+        jrf = JunitResultFormat()
         for spec in specs:
             package = spack.db.get(spec)
             #import pdb; pdb.set_trace()
 
-            print spec.name
-            print spec.version
-            print spec.dag_hash()
+            bId = BuildId(spec.name, spec.version, spec.dag_hash())
 
             if package.installed:
                 installLog = spack.install_layout.build_log_path(spec)
@@ -124,6 +140,8 @@ def testinstall(parser, args):
                 installLog = join_path(package.stage.source_path, 'spack-build.out')            
 
             with open(installLog, 'rb') as F:
-                for line in F.readlines()[:10]:
-                    print "\t{0}".format(line.strip())
-            
+                buildLog = F.read() #TODO: this may not return all output
+                jrf.addTest(bId, package.installed, buildLog)
+
+        with open(args.output, 'wb') as F:
+            jrf.writeTo(F)

From 9f56d9c807d9d3efc7cde0591bd53d3db404dacc Mon Sep 17 00:00:00 2001
From: Peter Scheibel <scheibel1@llnl.gov>
Date: Mon, 12 Oct 2015 20:56:03 -0700
Subject: [PATCH 03/26] Don't create test output for any package that was
 already installed.

---
 lib/spack/spack/cmd/testinstall.py | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/lib/spack/spack/cmd/testinstall.py b/lib/spack/spack/cmd/testinstall.py
index 7ebadbd3444..04e594c0b85 100644
--- a/lib/spack/spack/cmd/testinstall.py
+++ b/lib/spack/spack/cmd/testinstall.py
@@ -113,23 +113,23 @@ def testinstall(parser, args):
         spack.do_checksum = False        # TODO: remove this global.
 
     specs = spack.cmd.parse_specs(args.packages, concretize=True)
+    newInstalls = list()
     try:
         for spec in specs:
-            #import pdb; pdb.set_trace()
             package = spack.db.get(spec)
-            package.do_install(
-                keep_prefix=False,
-                keep_stage=False,
-                ignore_deps=False,
-                make_jobs=args.jobs,
-                verbose=args.verbose,
-                fake=False)
+            if not package.installed:
+                newInstalls.append(spec)
+                package.do_install(
+                    keep_prefix=False,
+                    keep_stage=False,
+                    ignore_deps=False,
+                    make_jobs=args.jobs,
+                    verbose=args.verbose,
+                    fake=False)
     finally:
         jrf = JunitResultFormat()
-        for spec in specs:
+        for spec in newInstalls:
             package = spack.db.get(spec)
-            #import pdb; pdb.set_trace()
-
             bId = BuildId(spec.name, spec.version, spec.dag_hash())
 
             if package.installed:

From 1ce6d8b627fac204fc4a4500ea28b4733dd172dd Mon Sep 17 00:00:00 2001
From: Peter Scheibel <scheibel1@llnl.gov>
Date: Tue, 13 Oct 2015 10:41:47 -0700
Subject: [PATCH 04/26] Add spec YAML format to test output.

---
 lib/spack/spack/cmd/testinstall.py | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/lib/spack/spack/cmd/testinstall.py b/lib/spack/spack/cmd/testinstall.py
index 04e594c0b85..3a9e1e14e16 100644
--- a/lib/spack/spack/cmd/testinstall.py
+++ b/lib/spack/spack/cmd/testinstall.py
@@ -75,19 +75,19 @@ def __init__(self):
         self.root = ET.Element('testsuite')
         self.tests = []
         
-    def addTest(self, buildId, passed=True, buildLog=None):
-        self.tests.append((buildId, passed, buildLog))
+    def addTest(self, buildId, passed=True, buildInfo=None):
+        self.tests.append((buildId, passed, buildInfo))
     
     def writeTo(self, stream):
         self.root.set('tests', '{0}'.format(len(self.tests)))
-        for buildId, passed, buildLog in self.tests:
+        for buildId, passed, buildInfo in self.tests:
             testcase = ET.SubElement(self.root, 'testcase')
             testcase.set('classname', buildId.name)
             testcase.set('name', buildId.stringId())
             if not passed:
                 failure = ET.SubElement(testcase, 'failure')
                 failure.set('type', "Build Error")
-                failure.text = buildLog
+                failure.text = buildInfo
         ET.ElementTree(self.root).write(stream)
 
 
@@ -133,15 +133,18 @@ def testinstall(parser, args):
             bId = BuildId(spec.name, spec.version, spec.dag_hash())
 
             if package.installed:
-                installLog = spack.install_layout.build_log_path(spec)
+                buildLogPath = spack.install_layout.build_log_path(spec)
             else:
                 #TODO: search recursively under stage.path instead of only within
                 #    stage.source_path
-                installLog = join_path(package.stage.source_path, 'spack-build.out')            
+                buildLogPath = join_path(package.stage.source_path, 'spack-build.out')            
 
-            with open(installLog, 'rb') as F:
+            with open(buildLogPath, 'rb') as F:
                 buildLog = F.read() #TODO: this may not return all output
-                jrf.addTest(bId, package.installed, buildLog)
+                #TODO: add the whole build log? it could be several thousand
+                #    lines. It may be better to look for errors.
+                jrf.addTest(bId, package.installed, buildLogPath + '\n' +
+                    spec.to_yaml() + buildLog)
 
         with open(args.output, 'wb') as F:
             jrf.writeTo(F)

From 71dcf8833c263a2f87b8bfcce6f2eaf7a14014a3 Mon Sep 17 00:00:00 2001
From: Peter Scheibel <scheibel1@llnl.gov>
Date: Tue, 13 Oct 2015 19:02:41 -0700
Subject: [PATCH 05/26] Make sure to generate output for dependencies as if
 they were separate tests: the original intent was to generate output as if
 each package was a unit test, but I noticed that I was only generating test
 output for top-level packages.

---
 lib/spack/spack/cmd/testinstall.py | 71 +++++++++++++++++++++---------
 1 file changed, 51 insertions(+), 20 deletions(-)

diff --git a/lib/spack/spack/cmd/testinstall.py b/lib/spack/spack/cmd/testinstall.py
index 3a9e1e14e16..6f3514bc346 100644
--- a/lib/spack/spack/cmd/testinstall.py
+++ b/lib/spack/spack/cmd/testinstall.py
@@ -24,6 +24,7 @@
 ##############################################################################
 from external import argparse
 import xml.etree.ElementTree as ET
+import itertools
 
 import llnl.util.tty as tty
 from llnl.util.filesystem import *
@@ -100,7 +101,37 @@ def __init__(self, name, version, hashId):
     def stringId(self):
         return "-".join(str(x) for x in (self.name, self.version, self.hashId))
 
-     
+
+def createTestOutput(spec, handled, output):
+    if spec in handled:
+        return handled[spec]
+    
+    childSuccesses = list(createTestOutput(dep, handled, output) 
+            for dep in spec.dependencies.itervalues())
+    package = spack.db.get(spec)
+    handled[spec] = package.installed
+    
+    if all(childSuccesses):
+        bId = BuildId(spec.name, spec.version, spec.dag_hash())
+
+        if package.installed:
+            buildLogPath = spack.install_layout.build_log_path(spec)
+        else:
+            #TODO: search recursively under stage.path instead of only within
+            #    stage.source_path
+            buildLogPath = join_path(package.stage.source_path, 'spack-build.out')            
+
+        with open(buildLogPath, 'rb') as F:
+            buildLog = F.read() #TODO: this may not return all output
+            #TODO: add the whole build log? it could be several thousand
+            #    lines. It may be better to look for errors.
+            output.addTest(bId, package.installed, buildLogPath + '\n' +
+                spec.to_yaml() + buildLog)
+    #TODO: create a failed test if a dependency didn't install?
+
+    return handled[spec]
+
+
 def testinstall(parser, args):
     if not args.packages:
         tty.die("install requires at least one package argument")
@@ -113,12 +144,17 @@ def testinstall(parser, args):
         spack.do_checksum = False        # TODO: remove this global.
 
     specs = spack.cmd.parse_specs(args.packages, concretize=True)
-    newInstalls = list()
+    newInstalls = set()
+    for spec in itertools.chain.from_iterable(spec.traverse() 
+            for spec in specs):
+        package = spack.db.get(spec)
+        if not package.installed:
+            newInstalls.add(spec)
+    
     try:
         for spec in specs:
             package = spack.db.get(spec)
             if not package.installed:
-                newInstalls.append(spec)
                 package.do_install(
                     keep_prefix=False,
                     keep_stage=False,
@@ -127,24 +163,19 @@ def testinstall(parser, args):
                     verbose=args.verbose,
                     fake=False)
     finally:
+        #TODO: note if multiple packages are specified and a prior one fails, 
+        #    it will prevent any of the others from succeeding even if they
+        #    don't share any dependencies. i.e. the results may be strange if
+        #    you run testinstall with >1 top-level package
+        
+        #Find all packages that are not a dependency of another package
+        topLevelNewInstalls = newInstalls - set(itertools.chain.from_iterable(
+                spec.dependencies for spec in newInstalls))
+        
         jrf = JunitResultFormat()
-        for spec in newInstalls:
-            package = spack.db.get(spec)
-            bId = BuildId(spec.name, spec.version, spec.dag_hash())
-
-            if package.installed:
-                buildLogPath = spack.install_layout.build_log_path(spec)
-            else:
-                #TODO: search recursively under stage.path instead of only within
-                #    stage.source_path
-                buildLogPath = join_path(package.stage.source_path, 'spack-build.out')            
-
-            with open(buildLogPath, 'rb') as F:
-                buildLog = F.read() #TODO: this may not return all output
-                #TODO: add the whole build log? it could be several thousand
-                #    lines. It may be better to look for errors.
-                jrf.addTest(bId, package.installed, buildLogPath + '\n' +
-                    spec.to_yaml() + buildLog)
+        handled = {}
+        for spec in topLevelNewInstalls:
+            createTestOutput(spec, handled, jrf)
 
         with open(args.output, 'wb') as F:
             jrf.writeTo(F)

From 0d66362cee9849db77a9f3297aa697f21fe1acdd Mon Sep 17 00:00:00 2001
From: Peter Scheibel <scheibel1@llnl.gov>
Date: Thu, 15 Oct 2015 10:17:08 -0700
Subject: [PATCH 06/26] Only install 1 top-level package with testinstall.
 Otherwise if multiple packages are specified and a prior one fails, it will
 prevent any of the others from succeeding (and generating test output) even
 if they don't share dependencies.

---
 lib/spack/spack/cmd/testinstall.py | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/lib/spack/spack/cmd/testinstall.py b/lib/spack/spack/cmd/testinstall.py
index 6f3514bc346..5e5288bfbd2 100644
--- a/lib/spack/spack/cmd/testinstall.py
+++ b/lib/spack/spack/cmd/testinstall.py
@@ -68,7 +68,7 @@ def setup_parser(subparser):
         'output', help="test output goes in this file")
     
     subparser.add_argument(
-        'packages', nargs=argparse.REMAINDER, help="specs of packages to install")
+        'package', help="spec of package to install")
 
 
 class JunitResultFormat(object):
@@ -133,8 +133,8 @@ def createTestOutput(spec, handled, output):
 
 
 def testinstall(parser, args):
-    if not args.packages:
-        tty.die("install requires at least one package argument")
+    if not args.package:
+        tty.die("install requires a package argument")
 
     if args.jobs is not None:
         if args.jobs <= 0:
@@ -143,7 +143,8 @@ def testinstall(parser, args):
     if args.no_checksum:
         spack.do_checksum = False        # TODO: remove this global.
 
-    specs = spack.cmd.parse_specs(args.packages, concretize=True)
+    #TODO: should a single argument be wrapped in a list?
+    specs = spack.cmd.parse_specs(args.package, concretize=True)
     newInstalls = set()
     for spec in itertools.chain.from_iterable(spec.traverse() 
             for spec in specs):
@@ -162,12 +163,7 @@ def testinstall(parser, args):
                     make_jobs=args.jobs,
                     verbose=args.verbose,
                     fake=False)
-    finally:
-        #TODO: note if multiple packages are specified and a prior one fails, 
-        #    it will prevent any of the others from succeeding even if they
-        #    don't share any dependencies. i.e. the results may be strange if
-        #    you run testinstall with >1 top-level package
-        
+    finally:        
         #Find all packages that are not a dependency of another package
         topLevelNewInstalls = newInstalls - set(itertools.chain.from_iterable(
                 spec.dependencies for spec in newInstalls))

From 2ae7839b666592e3acaf370d52f69376b80b28a7 Mon Sep 17 00:00:00 2001
From: Peter Scheibel <scheibel1@llnl.gov>
Date: Thu, 15 Oct 2015 10:26:13 -0700
Subject: [PATCH 07/26] Edit function names to conform to naming conventions.

---
 lib/spack/spack/cmd/testinstall.py | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/lib/spack/spack/cmd/testinstall.py b/lib/spack/spack/cmd/testinstall.py
index 5e5288bfbd2..ea092727fe6 100644
--- a/lib/spack/spack/cmd/testinstall.py
+++ b/lib/spack/spack/cmd/testinstall.py
@@ -76,10 +76,10 @@ def __init__(self):
         self.root = ET.Element('testsuite')
         self.tests = []
         
-    def addTest(self, buildId, passed=True, buildInfo=None):
+    def add_test(self, buildId, passed=True, buildInfo=None):
         self.tests.append((buildId, passed, buildInfo))
     
-    def writeTo(self, stream):
+    def write_to(self, stream):
         self.root.set('tests', '{0}'.format(len(self.tests)))
         for buildId, passed, buildInfo in self.tests:
             testcase = ET.SubElement(self.root, 'testcase')
@@ -102,11 +102,11 @@ def stringId(self):
         return "-".join(str(x) for x in (self.name, self.version, self.hashId))
 
 
-def createTestOutput(spec, handled, output):
+def create_test_output(spec, handled, output):
     if spec in handled:
         return handled[spec]
     
-    childSuccesses = list(createTestOutput(dep, handled, output) 
+    childSuccesses = list(create_test_output(dep, handled, output) 
             for dep in spec.dependencies.itervalues())
     package = spack.db.get(spec)
     handled[spec] = package.installed
@@ -125,7 +125,7 @@ def createTestOutput(spec, handled, output):
             buildLog = F.read() #TODO: this may not return all output
             #TODO: add the whole build log? it could be several thousand
             #    lines. It may be better to look for errors.
-            output.addTest(bId, package.installed, buildLogPath + '\n' +
+            output.add_test(bId, package.installed, buildLogPath + '\n' +
                 spec.to_yaml() + buildLog)
     #TODO: create a failed test if a dependency didn't install?
 
@@ -171,7 +171,7 @@ def testinstall(parser, args):
         jrf = JunitResultFormat()
         handled = {}
         for spec in topLevelNewInstalls:
-            createTestOutput(spec, handled, jrf)
+            create_test_output(spec, handled, jrf)
 
         with open(args.output, 'wb') as F:
-            jrf.writeTo(F)
+            jrf.write_to(F)

From e3d703b80ffe442d83879c070e0bbeafa4f4ef20 Mon Sep 17 00:00:00 2001
From: Peter Scheibel <scheibel1@llnl.gov>
Date: Thu, 15 Oct 2015 10:33:39 -0700
Subject: [PATCH 08/26] Change name of file to conform to conventions.

---
 lib/spack/spack/cmd/{testinstall.py => test-install.py} | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename lib/spack/spack/cmd/{testinstall.py => test-install.py} (100%)

diff --git a/lib/spack/spack/cmd/testinstall.py b/lib/spack/spack/cmd/test-install.py
similarity index 100%
rename from lib/spack/spack/cmd/testinstall.py
rename to lib/spack/spack/cmd/test-install.py

From 11861fb8d717623c4f0014194c81fa86041fbe10 Mon Sep 17 00:00:00 2001
From: Peter Scheibel <scheibel1@llnl.gov>
Date: Thu, 15 Oct 2015 10:35:42 -0700
Subject: [PATCH 09/26] Changing name of file requires changing function name
 to be invoked as a command

---
 lib/spack/spack/cmd/test-install.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/spack/spack/cmd/test-install.py b/lib/spack/spack/cmd/test-install.py
index ea092727fe6..eff4bcc46b6 100644
--- a/lib/spack/spack/cmd/test-install.py
+++ b/lib/spack/spack/cmd/test-install.py
@@ -132,7 +132,7 @@ def create_test_output(spec, handled, output):
     return handled[spec]
 
 
-def testinstall(parser, args):
+def test_install(parser, args):
     if not args.package:
         tty.die("install requires a package argument")
 

From f2b4341ad6d9dc925364214863440dde29d61b50 Mon Sep 17 00:00:00 2001
From: Peter Scheibel <scheibel1@llnl.gov>
Date: Thu, 15 Oct 2015 10:45:03 -0700
Subject: [PATCH 10/26] Always run with verbose output (so eliminate it as an
 option). Also remove other commented options.

---
 lib/spack/spack/cmd/test-install.py | 23 +----------------------
 1 file changed, 1 insertion(+), 22 deletions(-)

diff --git a/lib/spack/spack/cmd/test-install.py b/lib/spack/spack/cmd/test-install.py
index eff4bcc46b6..ec413115c01 100644
--- a/lib/spack/spack/cmd/test-install.py
+++ b/lib/spack/spack/cmd/test-install.py
@@ -35,34 +35,13 @@
 description = "Build and install packages"
 
 def setup_parser(subparser):
-    #subparser.add_argument(
-    #    '-i', '--ignore-dependencies', action='store_true', dest='ignore_deps',
-    #    help="Do not try to install dependencies of requested packages.")
-    
     subparser.add_argument(
         '-j', '--jobs', action='store', type=int,
         help="Explicitly set number of make jobs.  Default is #cpus.")
-        
-    #always false for test
-    #subparser.add_argument(
-    #    '--keep-prefix', action='store_true', dest='keep_prefix',
-    #    help="Don't remove the install prefix if installation fails.")
-    
-    #always true for test
-    #subparser.add_argument(
-    #    '--keep-stage', action='store_true', dest='keep_stage',
-    #    help="Don't remove the build stage if installation succeeds.")
     
     subparser.add_argument(
         '-n', '--no-checksum', action='store_true', dest='no_checksum',
         help="Do not check packages against checksum")
-    subparser.add_argument(
-        '-v', '--verbose', action='store_true', dest='verbose',
-        help="Display verbose build output while installing.")
-    
-    #subparser.add_argument(
-    #    '--fake', action='store_true', dest='fake',
-    #    help="Fake install.  Just remove the prefix and touch a fake file in it.")
     
     subparser.add_argument(
         'output', help="test output goes in this file")
@@ -161,7 +140,7 @@ def test_install(parser, args):
                     keep_stage=False,
                     ignore_deps=False,
                     make_jobs=args.jobs,
-                    verbose=args.verbose,
+                    verbose=True,
                     fake=False)
     finally:        
         #Find all packages that are not a dependency of another package

From b9bf0b942c4e2770de65282311876d02d00e7f5b Mon Sep 17 00:00:00 2001
From: Peter Scheibel <scheibel1@llnl.gov>
Date: Thu, 15 Oct 2015 11:52:08 -0700
Subject: [PATCH 11/26] Use spec.traverse vs. recursive function.

Also even though I calculated which installs are new (e.g. vs. packages that
have already been installed by a previous command) I forgot to make use of that
in create_test_output (so I was always generating test output even if a package
had been installed before running the test-install command).

Note to avoid confusion: the 'handled' variable (removed in this commit) did not
serve the same purpose as 'newInstalls': it was originally required because the
recursive approach would visit the same dependency twice if more than one
package depended on it.
---
 lib/spack/spack/cmd/test-install.py | 36 +++++++++++++----------------
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/lib/spack/spack/cmd/test-install.py b/lib/spack/spack/cmd/test-install.py
index ec413115c01..6652a204fbb 100644
--- a/lib/spack/spack/cmd/test-install.py
+++ b/lib/spack/spack/cmd/test-install.py
@@ -81,18 +81,21 @@ def stringId(self):
         return "-".join(str(x) for x in (self.name, self.version, self.hashId))
 
 
-def create_test_output(spec, handled, output):
-    if spec in handled:
-        return handled[spec]
-    
-    childSuccesses = list(create_test_output(dep, handled, output) 
-            for dep in spec.dependencies.itervalues())
-    package = spack.db.get(spec)
-    handled[spec] = package.installed
-    
-    if all(childSuccesses):
+def create_test_output(topSpec, newInstalls, output):
+    # Post-order traversal is not strictly required but it makes sense to output 
+    # tests for dependencies first.
+    for spec in topSpec.traverse(order='post'):
+        if spec not in newInstalls:
+            continue
+
+        if not all(spack.db.get(childSpec).installed for childSpec in 
+                spec.dependencies.itervalues()):
+            #TODO: create a failed test if a dependency didn't install?
+            continue
+                
         bId = BuildId(spec.name, spec.version, spec.dag_hash())
 
+        package = spack.db.get(spec)
         if package.installed:
             buildLogPath = spack.install_layout.build_log_path(spec)
         else:
@@ -106,10 +109,7 @@ def create_test_output(spec, handled, output):
             #    lines. It may be better to look for errors.
             output.add_test(bId, package.installed, buildLogPath + '\n' +
                 spec.to_yaml() + buildLog)
-    #TODO: create a failed test if a dependency didn't install?
-
-    return handled[spec]
-
+        
 
 def test_install(parser, args):
     if not args.package:
@@ -143,14 +143,10 @@ def test_install(parser, args):
                     verbose=True,
                     fake=False)
     finally:        
-        #Find all packages that are not a dependency of another package
-        topLevelNewInstalls = newInstalls - set(itertools.chain.from_iterable(
-                spec.dependencies for spec in newInstalls))
-        
         jrf = JunitResultFormat()
         handled = {}
-        for spec in topLevelNewInstalls:
-            create_test_output(spec, handled, jrf)
+        for spec in specs:
+            create_test_output(spec, newInstalls, jrf)
 
         with open(args.output, 'wb') as F:
             jrf.write_to(F)

From c985ad7644e00fa2fd5253d6b3f761a2596c6c4b Mon Sep 17 00:00:00 2001
From: Peter Scheibel <scheibel1@llnl.gov>
Date: Thu, 15 Oct 2015 12:23:56 -0700
Subject: [PATCH 12/26] Update test failure output: don't include the entire
 build log, just lines which mention errors (or if no such lines can be found,
 output the last 10 lines from the log).

---
 lib/spack/spack/cmd/test-install.py | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/lib/spack/spack/cmd/test-install.py b/lib/spack/spack/cmd/test-install.py
index 6652a204fbb..f2c40e57e49 100644
--- a/lib/spack/spack/cmd/test-install.py
+++ b/lib/spack/spack/cmd/test-install.py
@@ -25,6 +25,7 @@
 from external import argparse
 import xml.etree.ElementTree as ET
 import itertools
+import re
 
 import llnl.util.tty as tty
 from llnl.util.filesystem import *
@@ -104,11 +105,15 @@ def create_test_output(topSpec, newInstalls, output):
             buildLogPath = join_path(package.stage.source_path, 'spack-build.out')            
 
         with open(buildLogPath, 'rb') as F:
-            buildLog = F.read() #TODO: this may not return all output
-            #TODO: add the whole build log? it could be several thousand
-            #    lines. It may be better to look for errors.
-            output.add_test(bId, package.installed, buildLogPath + '\n' +
-                spec.to_yaml() + buildLog)
+            lines = F.readlines()
+            errMessages = list(line for line in lines if
+                re.search('error:', line, re.IGNORECASE))
+            errOutput = errMessages if errMessages else lines[-10:]
+            errOutput = '\n'.join(itertools.chain(
+                    [spec.to_yaml(), "Errors:"], errOutput, 
+                    ["Build Log:", buildLogPath]))
+            
+            output.add_test(bId, package.installed, errOutput)
         
 
 def test_install(parser, args):

From 4997f0fe57e65002d8122da05a4f203f51ac4345 Mon Sep 17 00:00:00 2001
From: Peter Scheibel <scheibel1@llnl.gov>
Date: Thu, 15 Oct 2015 12:44:02 -0700
Subject: [PATCH 13/26] Move logic for tracking the build log into package.py
 (since that is what is managing the build log) and expose as
 package.build_log_path.

---
 lib/spack/spack/cmd/test-install.py | 11 ++---------
 lib/spack/spack/package.py          |  8 ++++++++
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/lib/spack/spack/cmd/test-install.py b/lib/spack/spack/cmd/test-install.py
index f2c40e57e49..85140422394 100644
--- a/lib/spack/spack/cmd/test-install.py
+++ b/lib/spack/spack/cmd/test-install.py
@@ -97,21 +97,14 @@ def create_test_output(topSpec, newInstalls, output):
         bId = BuildId(spec.name, spec.version, spec.dag_hash())
 
         package = spack.db.get(spec)
-        if package.installed:
-            buildLogPath = spack.install_layout.build_log_path(spec)
-        else:
-            #TODO: search recursively under stage.path instead of only within
-            #    stage.source_path
-            buildLogPath = join_path(package.stage.source_path, 'spack-build.out')            
-
-        with open(buildLogPath, 'rb') as F:
+        with open(package.build_log_path, 'rb') as F:
             lines = F.readlines()
             errMessages = list(line for line in lines if
                 re.search('error:', line, re.IGNORECASE))
             errOutput = errMessages if errMessages else lines[-10:]
             errOutput = '\n'.join(itertools.chain(
                     [spec.to_yaml(), "Errors:"], errOutput, 
-                    ["Build Log:", buildLogPath]))
+                    ["Build Log:", package.build_log_path]))
             
             output.add_test(bId, package.installed, errOutput)
         
diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index 61606d05905..da19a7c3987 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -863,6 +863,14 @@ def do_install_dependencies(self, **kwargs):
             dep.package.do_install(**kwargs)
 
 
+    @property
+    def build_log_path(self):
+        if self.installed:
+            return spack.install_layout.build_log_path(spec)
+        else:
+            return join_path(self.stage.source_path, 'spack-build.out')   
+
+
     @property
     def module(self):
         """Use this to add variables to the class's module's scope.

From e451421db32b5e2059d9da293c70baa4b3374449 Mon Sep 17 00:00:00 2001
From: Peter Scheibel <scheibel1@llnl.gov>
Date: Thu, 15 Oct 2015 19:22:36 -0700
Subject: [PATCH 14/26] 1. Specifying the output file path for test-install is
 now an option (vs. an argument). The default path is [package id].xml in the
 CWD where test-install is called from.

2. Fixed a bug with package.build_log_path (which was added in this branch).

3. keep_stage for package.do_install is now set. This allows uninstalling and
reinstalling packages without (re) downloading them.
---
 lib/spack/spack/cmd/test-install.py | 32 +++++++++++++++++++----------
 lib/spack/spack/package.py          |  2 +-
 2 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/lib/spack/spack/cmd/test-install.py b/lib/spack/spack/cmd/test-install.py
index 85140422394..e2072958106 100644
--- a/lib/spack/spack/cmd/test-install.py
+++ b/lib/spack/spack/cmd/test-install.py
@@ -26,6 +26,7 @@
 import xml.etree.ElementTree as ET
 import itertools
 import re
+import os
 
 import llnl.util.tty as tty
 from llnl.util.filesystem import *
@@ -45,7 +46,7 @@ def setup_parser(subparser):
         help="Do not check packages against checksum")
     
     subparser.add_argument(
-        'output', help="test output goes in this file")
+        '-o', '--output', action='store', help="test output goes in this file")
     
     subparser.add_argument(
         'package', help="spec of package to install")
@@ -73,10 +74,10 @@ def write_to(self, stream):
 
 
 class BuildId(object):
-    def __init__(self, name, version, hashId):
-        self.name = name
-        self.version = version
-        self.hashId = hashId
+    def __init__(self, spec):
+        self.name = spec.name
+        self.version = spec.version
+        self.hashId = spec.dag_hash()
     
     def stringId(self):
         return "-".join(str(x) for x in (self.name, self.version, self.hashId))
@@ -94,7 +95,7 @@ def create_test_output(topSpec, newInstalls, output):
             #TODO: create a failed test if a dependency didn't install?
             continue
                 
-        bId = BuildId(spec.name, spec.version, spec.dag_hash())
+        bId = BuildId(spec)
 
         package = spack.db.get(spec)
         with open(package.build_log_path, 'rb') as F:
@@ -120,22 +121,31 @@ def test_install(parser, args):
     if args.no_checksum:
         spack.do_checksum = False        # TODO: remove this global.
 
-    #TODO: should a single argument be wrapped in a list?
+    # TODO: should a single argument be wrapped in a list?
     specs = spack.cmd.parse_specs(args.package, concretize=True)
+    
+    # There is 1 top-level package
+    topSpec = iter(specs).next()
+    
     newInstalls = set()
-    for spec in itertools.chain.from_iterable(spec.traverse() 
-            for spec in specs):
+    for spec in topSpec.traverse():
         package = spack.db.get(spec)
         if not package.installed:
             newInstalls.add(spec)
     
+    if not args.output:
+        bId = BuildId(topSpec)
+        outputFpath = join_path(os.getcwd(), "{0}.xml".format(bId.stringId()))
+    else:
+        outputFpath = args.output
+    
     try:
         for spec in specs:
             package = spack.db.get(spec)
             if not package.installed:
                 package.do_install(
                     keep_prefix=False,
-                    keep_stage=False,
+                    keep_stage=True,
                     ignore_deps=False,
                     make_jobs=args.jobs,
                     verbose=True,
@@ -146,5 +156,5 @@ def test_install(parser, args):
         for spec in specs:
             create_test_output(spec, newInstalls, jrf)
 
-        with open(args.output, 'wb') as F:
+        with open(outputFpath, 'wb') as F:
             jrf.write_to(F)
diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index da19a7c3987..b1257a092fb 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -866,7 +866,7 @@ def do_install_dependencies(self, **kwargs):
     @property
     def build_log_path(self):
         if self.installed:
-            return spack.install_layout.build_log_path(spec)
+            return spack.install_layout.build_log_path(self.spec)
         else:
             return join_path(self.stage.source_path, 'spack-build.out')   
 

From 82ed1bc34397542394ea7fc4a23f3b827546809a Mon Sep 17 00:00:00 2001
From: Peter Scheibel <scheibel1@llnl.gov>
Date: Thu, 15 Oct 2015 19:38:47 -0700
Subject: [PATCH 15/26] Originally I enforced specifying 1 top-level package
 with the test-install command by having it consume exactly 1 positional
 argument (i.e. by removing "nargs=argparse.REMAINDER") but this does not work
 when configuring dependencies of a top-level package (which show up as
 additional positional args). Instead now there is an explicit check to ensure
 there is only 1 top-level package.

---
 lib/spack/spack/cmd/test-install.py | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/lib/spack/spack/cmd/test-install.py b/lib/spack/spack/cmd/test-install.py
index e2072958106..962af939f27 100644
--- a/lib/spack/spack/cmd/test-install.py
+++ b/lib/spack/spack/cmd/test-install.py
@@ -49,7 +49,7 @@ def setup_parser(subparser):
         '-o', '--output', action='store', help="test output goes in this file")
     
     subparser.add_argument(
-        'package', help="spec of package to install")
+        'package', nargs=argparse.REMAINDER, help="spec of package to install")
 
 
 class JunitResultFormat(object):
@@ -120,11 +120,10 @@ def test_install(parser, args):
 
     if args.no_checksum:
         spack.do_checksum = False        # TODO: remove this global.
-
-    # TODO: should a single argument be wrapped in a list?
-    specs = spack.cmd.parse_specs(args.package, concretize=True)
     
-    # There is 1 top-level package
+    specs = spack.cmd.parse_specs(args.package, concretize=True)
+    if len(specs) > 1:
+        tty.die("Only 1 top-level package can be specified")
     topSpec = iter(specs).next()
     
     newInstalls = set()

From 49b91235bb8522a79d5a0b213718af2a6f81f501 Mon Sep 17 00:00:00 2001
From: Peter Scheibel <scheibel1@llnl.gov>
Date: Thu, 15 Oct 2015 19:59:57 -0700
Subject: [PATCH 16/26] Minor edit for clarity (generate output for single top
 level spec vs. iterating through collection of size 1)

---
 lib/spack/spack/cmd/test-install.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/spack/spack/cmd/test-install.py b/lib/spack/spack/cmd/test-install.py
index 962af939f27..ee9580c1283 100644
--- a/lib/spack/spack/cmd/test-install.py
+++ b/lib/spack/spack/cmd/test-install.py
@@ -152,8 +152,7 @@ def test_install(parser, args):
     finally:        
         jrf = JunitResultFormat()
         handled = {}
-        for spec in specs:
-            create_test_output(spec, newInstalls, jrf)
+        create_test_output(topSpec, newInstalls, jrf)
 
         with open(outputFpath, 'wb') as F:
             jrf.write_to(F)

From 6cd976d036cce518d899202baeebc7103a0a6f2a Mon Sep 17 00:00:00 2001
From: Peter Scheibel <scheibel1@llnl.gov>
Date: Thu, 15 Oct 2015 20:13:08 -0700
Subject: [PATCH 17/26] Better description for test-install command

---
 lib/spack/spack/cmd/test-install.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/spack/spack/cmd/test-install.py b/lib/spack/spack/cmd/test-install.py
index ee9580c1283..0facf59b943 100644
--- a/lib/spack/spack/cmd/test-install.py
+++ b/lib/spack/spack/cmd/test-install.py
@@ -34,7 +34,7 @@
 import spack
 import spack.cmd
 
-description = "Build and install packages"
+description = "Treat package installations as unit tests and output formatted test results"
 
 def setup_parser(subparser):
     subparser.add_argument(

From 39f0f000f89f40a32b9e25d9ba681d6d032d025a Mon Sep 17 00:00:00 2001
From: Peter Scheibel <scheibel1@llnl.gov>
Date: Thu, 15 Oct 2015 22:02:14 -0700
Subject: [PATCH 18/26] Created unit test for core logic in test-install
 command.

---
 lib/spack/spack/cmd/test-install.py  |  36 +++++---
 lib/spack/spack/test/__init__.py     |   3 +-
 lib/spack/spack/test/unit_install.py | 118 +++++++++++++++++++++++++++
 3 files changed, 145 insertions(+), 12 deletions(-)
 create mode 100644 lib/spack/spack/test/unit_install.py

diff --git a/lib/spack/spack/cmd/test-install.py b/lib/spack/spack/cmd/test-install.py
index 0facf59b943..7b37f66967a 100644
--- a/lib/spack/spack/cmd/test-install.py
+++ b/lib/spack/spack/cmd/test-install.py
@@ -82,8 +82,23 @@ def __init__(self, spec):
     def stringId(self):
         return "-".join(str(x) for x in (self.name, self.version, self.hashId))
 
+    def __hash__(self):
+        return hash((self.name, self.version, self.hashId))
+    
+    def __eq__(self, other):
+        if not isinstance(other, BuildId):
+            return False
+            
+        return ((self.name, self.version, self.hashId) == 
+            (other.name, other.version, other.hashId))
 
-def create_test_output(topSpec, newInstalls, output):
+
+def fetch_log(path):
+    with open(path, 'rb') as F:
+        return list(F.readlines())
+
+
+def create_test_output(topSpec, newInstalls, output, getLogFunc=fetch_log):
     # Post-order traversal is not strictly required but it makes sense to output 
     # tests for dependencies first.
     for spec in topSpec.traverse(order='post'):
@@ -98,17 +113,16 @@ def create_test_output(topSpec, newInstalls, output):
         bId = BuildId(spec)
 
         package = spack.db.get(spec)
-        with open(package.build_log_path, 'rb') as F:
-            lines = F.readlines()
-            errMessages = list(line for line in lines if
-                re.search('error:', line, re.IGNORECASE))
-            errOutput = errMessages if errMessages else lines[-10:]
-            errOutput = '\n'.join(itertools.chain(
-                    [spec.to_yaml(), "Errors:"], errOutput, 
-                    ["Build Log:", package.build_log_path]))
-            
-            output.add_test(bId, package.installed, errOutput)
+        lines = getLogFunc(package.build_log_path)
+        errMessages = list(line for line in lines if
+            re.search('error:', line, re.IGNORECASE))
+        errOutput = errMessages if errMessages else lines[-10:]
+        errOutput = '\n'.join(itertools.chain(
+                [spec.to_yaml(), "Errors:"], errOutput, 
+                ["Build Log:", package.build_log_path]))
         
+        output.add_test(bId, package.installed, errOutput)
+
 
 def test_install(parser, args):
     if not args.package:
diff --git a/lib/spack/spack/test/__init__.py b/lib/spack/spack/test/__init__.py
index 6b3715be6f9..6fd80d10848 100644
--- a/lib/spack/spack/test/__init__.py
+++ b/lib/spack/spack/test/__init__.py
@@ -56,7 +56,8 @@
               'spec_yaml',
               'optional_deps',
               'make_executable',
-              'configure_guess']
+              'configure_guess',
+              'unit_install']
 
 
 def list_tests():
diff --git a/lib/spack/spack/test/unit_install.py b/lib/spack/spack/test/unit_install.py
new file mode 100644
index 00000000000..ab7d4902d09
--- /dev/null
+++ b/lib/spack/spack/test/unit_install.py
@@ -0,0 +1,118 @@
+##############################################################################
+# Copyright (c) 2013, Lawrence Livermore National Security, LLC.
+# Produced at the Lawrence Livermore National Laboratory.
+#
+# This file is part of Spack.
+# Written by Todd Gamblin, tgamblin@llnl.gov, All rights reserved.
+# LLNL-CODE-647188
+#
+# For details, see https://scalability-llnl.github.io/spack
+# Please also see the LICENSE file for our notice and the LGPL.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License (as published by
+# the Free Software Foundation) version 2.1 dated February 1999.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the IMPLIED WARRANTY OF
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the terms and
+# conditions of the GNU General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public License
+# along with this program; if not, write to the Free Software Foundation,
+# Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+##############################################################################
+import unittest
+import itertools
+
+import spack
+test_install = __import__("spack.cmd.test-install", 
+    fromlist=["BuildId", "create_test_output"])
+
+class MockOutput(object):
+    def __init__(self):
+        self.results = {}
+    
+    def add_test(self, buildId, passed=True, buildInfo=None):
+        self.results[buildId] = passed
+    
+    def write_to(self, stream):
+        pass
+
+class MockSpec(object):
+    def __init__(self, name, version, hashStr=None):
+        self.dependencies = {}
+        self.name = name
+        self.version = version
+        self.hash = hashStr if hashStr else hash((name, version))
+    
+    def traverse(self, order=None):
+        allDeps = itertools.chain.from_iterable(i.traverse() for i in 
+            self.dependencies.itervalues())
+        return set(itertools.chain([self], allDeps))
+    
+    def dag_hash(self):
+        return self.hash 
+
+    def to_yaml(self):
+        return "<<<MOCK YAML {0}>>>".format(test_install.BuildId(self).stringId())
+
+class MockPackage(object):
+    def __init__(self, buildLogPath):
+        self.installed = False
+        self.build_log_path = buildLogPath
+
+specX = MockSpec("X", "1.2.0")
+specY = MockSpec("Y", "2.3.8")
+specX.dependencies['Y'] = specY
+pkgX = MockPackage('logX')
+pkgY = MockPackage('logY')
+bIdX = test_install.BuildId(specX)
+bIdY = test_install.BuildId(specY)
+
+class UnitInstallTest(unittest.TestCase):
+    """Tests test-install where X->Y"""
+
+    def setUp(self):
+        super(UnitInstallTest, self).setUp()
+    
+        #import pdb; pdb.set_trace()
+        pkgX.installed = False
+        pkgY.installed = False
+
+        pkgDb = MockPackageDb({specX:pkgX, specY:pkgY})
+        spack.db = pkgDb
+
+    def tearDown(self):
+        super(UnitInstallTest, self).tearDown()
+        
+    def test_installing_both(self):
+        mo = MockOutput()
+        
+        pkgX.installed = True
+        pkgY.installed = True
+        test_install.create_test_output(specX, [specX, specY], mo, getLogFunc=test_fetch_log)
+        
+        self.assertEqual(mo.results, {bIdX:True, bIdY:True})
+
+    def test_dependency_already_installed(self):
+        mo = MockOutput()
+        
+        pkgX.installed = True
+        pkgY.installed = True
+        test_install.create_test_output(specX, [specX], mo, getLogFunc=test_fetch_log)
+        
+        self.assertEqual(mo.results, {bIdX:True})
+
+class MockPackageDb(object):
+    def __init__(self, init=None):
+        self.specToPkg = {}
+        if init:
+            self.specToPkg.update(init)
+        
+    def get(self, spec):
+        return self.specToPkg[spec]
+
+def test_fetch_log(path):
+    return []
+

From 246423b4b472ae0d53488c33fbca9faa035402d7 Mon Sep 17 00:00:00 2001
From: Peter Scheibel <scheibel1@llnl.gov>
Date: Thu, 22 Oct 2015 16:00:03 -0700
Subject: [PATCH 19/26] Generate test results (designated as skipped) for
 parents of failed dependencies

---
 lib/spack/spack/cmd/test-install.py  | 58 +++++++++++++++++++---------
 lib/spack/spack/test/unit_install.py | 11 +++---
 2 files changed, 45 insertions(+), 24 deletions(-)

diff --git a/lib/spack/spack/cmd/test-install.py b/lib/spack/spack/cmd/test-install.py
index 7b37f66967a..406a6d7d33c 100644
--- a/lib/spack/spack/cmd/test-install.py
+++ b/lib/spack/spack/cmd/test-install.py
@@ -57,22 +57,32 @@ def __init__(self):
         self.root = ET.Element('testsuite')
         self.tests = []
         
-    def add_test(self, buildId, passed=True, buildInfo=None):
-        self.tests.append((buildId, passed, buildInfo))
+    def add_test(self, buildId, testResult, buildInfo=None):
+        self.tests.append((buildId, testResult, buildInfo))
     
     def write_to(self, stream):
         self.root.set('tests', '{0}'.format(len(self.tests)))
-        for buildId, passed, buildInfo in self.tests:
+        for buildId, testResult, buildInfo in self.tests:
             testcase = ET.SubElement(self.root, 'testcase')
             testcase.set('classname', buildId.name)
             testcase.set('name', buildId.stringId())
-            if not passed:
+            if testResult == TestResult.FAILED:
                 failure = ET.SubElement(testcase, 'failure')
                 failure.set('type', "Build Error")
                 failure.text = buildInfo
+            elif testResult == TestResult.SKIPPED:
+                skipped = ET.SubElement(testcase, 'skipped')
+                skipped.set('type', "Skipped Build")
+                skipped.text = buildInfo
         ET.ElementTree(self.root).write(stream)
 
 
+class TestResult(object):
+    PASSED = 0
+    FAILED = 1
+    SKIPPED = 2
+    
+
 class BuildId(object):
     def __init__(self, spec):
         self.name = spec.name
@@ -94,6 +104,8 @@ def __eq__(self, other):
 
 
 def fetch_log(path):
+    if not os.path.exists(path):
+        return list()
     with open(path, 'rb') as F:
         return list(F.readlines())
 
@@ -105,23 +117,31 @@ def create_test_output(topSpec, newInstalls, output, getLogFunc=fetch_log):
         if spec not in newInstalls:
             continue
 
-        if not all(spack.db.get(childSpec).installed for childSpec in 
-                spec.dependencies.itervalues()):
-            #TODO: create a failed test if a dependency didn't install?
-            continue
-                
-        bId = BuildId(spec)
-
+        failedDeps = set(childSpec for childSpec in 
+            spec.dependencies.itervalues() if not 
+            spack.db.get(childSpec).installed)
         package = spack.db.get(spec)
-        lines = getLogFunc(package.build_log_path)
-        errMessages = list(line for line in lines if
-            re.search('error:', line, re.IGNORECASE))
-        errOutput = errMessages if errMessages else lines[-10:]
-        errOutput = '\n'.join(itertools.chain(
-                [spec.to_yaml(), "Errors:"], errOutput, 
-                ["Build Log:", package.build_log_path]))
+        if failedDeps:
+            result = TestResult.SKIPPED
+            dep = iter(failedDeps).next()
+            depBID = BuildId(dep)
+            errOutput = "Skipped due to failed dependency: {0}".format(
+                depBID.stringId())
+        elif not package.installed:
+            result = TestResult.FAILED
+            lines = getLogFunc(package.build_log_path)
+            errMessages = list(line for line in lines if
+                re.search('error:', line, re.IGNORECASE))
+            errOutput = errMessages if errMessages else lines[-10:]
+            errOutput = '\n'.join(itertools.chain(
+                    [spec.to_yaml(), "Errors:"], errOutput, 
+                    ["Build Log:", package.build_log_path]))
+        else:
+            result = TestResult.PASSED
+            errOutput = None
         
-        output.add_test(bId, package.installed, errOutput)
+        bId = BuildId(spec)
+        output.add_test(bId, result, errOutput)
 
 
 def test_install(parser, args):
diff --git a/lib/spack/spack/test/unit_install.py b/lib/spack/spack/test/unit_install.py
index ab7d4902d09..2a94f8fec0a 100644
--- a/lib/spack/spack/test/unit_install.py
+++ b/lib/spack/spack/test/unit_install.py
@@ -27,7 +27,7 @@
 
 import spack
 test_install = __import__("spack.cmd.test-install", 
-    fromlist=["BuildId", "create_test_output"])
+    fromlist=["BuildId", "create_test_output", "TestResult"])
 
 class MockOutput(object):
     def __init__(self):
@@ -75,8 +75,7 @@ class UnitInstallTest(unittest.TestCase):
 
     def setUp(self):
         super(UnitInstallTest, self).setUp()
-    
-        #import pdb; pdb.set_trace()
+        
         pkgX.installed = False
         pkgY.installed = False
 
@@ -93,7 +92,9 @@ def test_installing_both(self):
         pkgY.installed = True
         test_install.create_test_output(specX, [specX, specY], mo, getLogFunc=test_fetch_log)
         
-        self.assertEqual(mo.results, {bIdX:True, bIdY:True})
+        self.assertEqual(mo.results, 
+            {bIdX:test_install.TestResult.PASSED, 
+            bIdY:test_install.TestResult.PASSED})
 
     def test_dependency_already_installed(self):
         mo = MockOutput()
@@ -102,7 +103,7 @@ def test_dependency_already_installed(self):
         pkgY.installed = True
         test_install.create_test_output(specX, [specX], mo, getLogFunc=test_fetch_log)
         
-        self.assertEqual(mo.results, {bIdX:True})
+        self.assertEqual(mo.results, {bIdX:test_install.TestResult.PASSED})
 
 class MockPackageDb(object):
     def __init__(self, init=None):

From ea872f8098af3525f0d3e9e0d2fd2efa41466e87 Mon Sep 17 00:00:00 2001
From: Peter Scheibel <scheibel1@llnl.gov>
Date: Thu, 22 Oct 2015 17:44:16 -0700
Subject: [PATCH 20/26] 1. Added CommandError exception to build_environment 2.
 The parent of a failed child process in build_environment.fork no longer
 calls sys.exit - instead it raises a CommandError (from [1]) 3. test-install
 command now attempts to install all packages even if one fails

---
 lib/spack/spack/build_environment.py |  6 ++++-
 lib/spack/spack/cmd/test-install.py  | 35 +++++++++++++++++-----------
 2 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py
index a133faa6295..0d179f563bd 100644
--- a/lib/spack/spack/build_environment.py
+++ b/lib/spack/spack/build_environment.py
@@ -296,4 +296,8 @@ def child_fun():
         # message.  Just make the parent exit with an error code.
         pid, returncode = os.waitpid(pid, 0)
         if returncode != 0:
-            sys.exit(1)
+            raise CommandError(returncode)
+
+
+class CommandError(StandardError):
+    pass
diff --git a/lib/spack/spack/cmd/test-install.py b/lib/spack/spack/cmd/test-install.py
index 406a6d7d33c..aeb90ae7336 100644
--- a/lib/spack/spack/cmd/test-install.py
+++ b/lib/spack/spack/cmd/test-install.py
@@ -32,6 +32,7 @@
 from llnl.util.filesystem import *
 
 import spack
+from spack.build_environment import CommandError
 import spack.cmd
 
 description = "Treat package installations as unit tests and output formatted test results"
@@ -107,7 +108,12 @@ def fetch_log(path):
     if not os.path.exists(path):
         return list()
     with open(path, 'rb') as F:
-        return list(F.readlines())
+        return list(line.strip() for line in F.readlines())
+
+
+def failed_dependencies(spec):
+    return set(childSpec for childSpec in spec.dependencies.itervalues() if not 
+        spack.db.get(childSpec).installed)
 
 
 def create_test_output(topSpec, newInstalls, output, getLogFunc=fetch_log):
@@ -117,9 +123,7 @@ def create_test_output(topSpec, newInstalls, output, getLogFunc=fetch_log):
         if spec not in newInstalls:
             continue
 
-        failedDeps = set(childSpec for childSpec in 
-            spec.dependencies.itervalues() if not 
-            spack.db.get(childSpec).installed)
+        failedDeps = failed_dependencies(spec)
         package = spack.db.get(spec)
         if failedDeps:
             result = TestResult.SKIPPED
@@ -172,10 +176,13 @@ def test_install(parser, args):
     else:
         outputFpath = args.output
     
-    try:
-        for spec in specs:
-            package = spack.db.get(spec)
-            if not package.installed:
+    for spec in topSpec.traverse(order='post'):
+        # Calling do_install for the top-level package would be sufficient but
+        # this attempts to keep going if any package fails (other packages which
+        # are not dependents may succeed)
+        package = spack.db.get(spec)
+        if (not failed_dependencies(spec)) and (not package.installed):
+            try:
                 package.do_install(
                     keep_prefix=False,
                     keep_stage=True,
@@ -183,10 +190,12 @@ def test_install(parser, args):
                     make_jobs=args.jobs,
                     verbose=True,
                     fake=False)
-    finally:        
-        jrf = JunitResultFormat()
-        handled = {}
-        create_test_output(topSpec, newInstalls, jrf)
+            except CommandError:
+                pass
+   
+    jrf = JunitResultFormat()
+    handled = {}
+    create_test_output(topSpec, newInstalls, jrf)
 
-        with open(outputFpath, 'wb') as F:
+    with open(outputFpath, 'wb') as F:
             jrf.write_to(F)

From d76c9236236747a2a19a10941b1efd497f0202e0 Mon Sep 17 00:00:00 2001
From: Peter Scheibel <scheibel1@llnl.gov>
Date: Fri, 23 Oct 2015 16:18:06 -0700
Subject: [PATCH 21/26] 1. Rename CommandError -> InstallError 2. InstallError
 now subclasses SpackError vs. StandardError (so it is now handled by the
 spack shell script)

---
 lib/spack/spack/build_environment.py | 8 +++++---
 lib/spack/spack/cmd/test-install.py  | 4 ++--
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py
index 0d179f563bd..620ad5be9e0 100644
--- a/lib/spack/spack/build_environment.py
+++ b/lib/spack/spack/build_environment.py
@@ -36,6 +36,7 @@
 
 import spack
 import spack.compilers as compilers
+from spack.error import SpackError
 from spack.util.executable import Executable, which
 from spack.util.environment import *
 
@@ -296,8 +297,9 @@ def child_fun():
         # message.  Just make the parent exit with an error code.
         pid, returncode = os.waitpid(pid, 0)
         if returncode != 0:
-            raise CommandError(returncode)
+            raise InstallError("Installation process had nonzero exit code."
+                .format(str(returncode)))
 
 
-class CommandError(StandardError):
-    pass
+class InstallError(SpackError):
+    """Raised when a package fails to install"""
diff --git a/lib/spack/spack/cmd/test-install.py b/lib/spack/spack/cmd/test-install.py
index aeb90ae7336..a9f9331fcb7 100644
--- a/lib/spack/spack/cmd/test-install.py
+++ b/lib/spack/spack/cmd/test-install.py
@@ -32,7 +32,7 @@
 from llnl.util.filesystem import *
 
 import spack
-from spack.build_environment import CommandError
+from spack.build_environment import InstallError
 import spack.cmd
 
 description = "Treat package installations as unit tests and output formatted test results"
@@ -190,7 +190,7 @@ def test_install(parser, args):
                     make_jobs=args.jobs,
                     verbose=True,
                     fake=False)
-            except CommandError:
+            except InstallError:
                 pass
    
     jrf = JunitResultFormat()

From cc0ee3dc29516d6ceb49c4144fa3cb75d0120b0d Mon Sep 17 00:00:00 2001
From: Peter Scheibel <scheibel1@llnl.gov>
Date: Fri, 23 Oct 2015 20:56:06 -0700
Subject: [PATCH 22/26] The HTML number conversion regex operating against a
 byte string will only convert individual bytes, so therefore incorrectly
 converts utf-8 encoded characters. Decoding byte strings to unicode objects
 results in correct HTML number encodings.

---
 lib/spack/spack/cmd/test-install.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/spack/spack/cmd/test-install.py b/lib/spack/spack/cmd/test-install.py
index a9f9331fcb7..d9165192278 100644
--- a/lib/spack/spack/cmd/test-install.py
+++ b/lib/spack/spack/cmd/test-install.py
@@ -27,6 +27,7 @@
 import itertools
 import re
 import os
+import codecs
 
 import llnl.util.tty as tty
 from llnl.util.filesystem import *
@@ -107,7 +108,7 @@ def __eq__(self, other):
 def fetch_log(path):
     if not os.path.exists(path):
         return list()
-    with open(path, 'rb') as F:
+    with codecs.open(path, 'rb', 'utf-8') as F:
         return list(line.strip() for line in F.readlines())
 
 

From 6a16040462b49acb356236a5f8114b055d680851 Mon Sep 17 00:00:00 2001
From: Peter Scheibel <scheibel1@llnl.gov>
Date: Mon, 26 Oct 2015 11:58:52 -0700
Subject: [PATCH 23/26] Automatically create a 'test-output' directory in the
 current directory if no output path is specified. Test output files are
 placed in this directory. Furthermore the filenames now have the prefix
 "test" (but otherwise are the string representation of the spec ID as
 before).

---
 lib/spack/spack/cmd/test-install.py | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/spack/spack/cmd/test-install.py b/lib/spack/spack/cmd/test-install.py
index d9165192278..76602474a41 100644
--- a/lib/spack/spack/cmd/test-install.py
+++ b/lib/spack/spack/cmd/test-install.py
@@ -173,7 +173,10 @@ def test_install(parser, args):
     
     if not args.output:
         bId = BuildId(topSpec)
-        outputFpath = join_path(os.getcwd(), "{0}.xml".format(bId.stringId()))
+        outputDir = join_path(os.getcwd(), "test-output")
+        if not os.path.exists(outputDir):
+            os.mkdir(outputDir)
+        outputFpath = join_path(outputDir, "test-{0}.xml".format(bId.stringId()))
     else:
         outputFpath = args.output
     

From 9576860f8c97360eddaffe316450ed2f3b87e876 Mon Sep 17 00:00:00 2001
From: Peter Scheibel <scheibel1@llnl.gov>
Date: Mon, 26 Oct 2015 14:27:44 -0700
Subject: [PATCH 24/26] Making SpackError reference consistent.

---
 lib/spack/spack/build_environment.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py
index 620ad5be9e0..dac25d99401 100644
--- a/lib/spack/spack/build_environment.py
+++ b/lib/spack/spack/build_environment.py
@@ -36,7 +36,6 @@
 
 import spack
 import spack.compilers as compilers
-from spack.error import SpackError
 from spack.util.executable import Executable, which
 from spack.util.environment import *
 
@@ -301,5 +300,5 @@ def child_fun():
                 .format(str(returncode)))
 
 
-class InstallError(SpackError):
+class InstallError(spack.error.SpackError):
     """Raised when a package fails to install"""

From 3b554c709bb1dc3704939964ac1265ccf8597718 Mon Sep 17 00:00:00 2001
From: Peter Scheibel <scheibel1@llnl.gov>
Date: Mon, 26 Oct 2015 15:26:08 -0700
Subject: [PATCH 25/26] Fetch errors were also terminating runs of test-install
 with system exit, so stage.fetch() was updated to raise a FetchError instead
 of calling tty.die(). Output is the same for spack install in case of a fetch
 error.

---
 lib/spack/spack/cmd/test-install.py  | 6 ++++++
 lib/spack/spack/stage.py             | 3 ++-
 lib/spack/spack/test/unit_install.py | 2 ++
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/lib/spack/spack/cmd/test-install.py b/lib/spack/spack/cmd/test-install.py
index 76602474a41..58ab40aa7b0 100644
--- a/lib/spack/spack/cmd/test-install.py
+++ b/lib/spack/spack/cmd/test-install.py
@@ -34,6 +34,7 @@
 
 import spack
 from spack.build_environment import InstallError
+from spack.fetch_strategy import FetchError
 import spack.cmd
 
 description = "Treat package installations as unit tests and output formatted test results"
@@ -132,6 +133,9 @@ def create_test_output(topSpec, newInstalls, output, getLogFunc=fetch_log):
             depBID = BuildId(dep)
             errOutput = "Skipped due to failed dependency: {0}".format(
                 depBID.stringId())
+        elif (not package.installed) and (not package.stage.archive_file):
+            result = TestResult.FAILED
+            errOutput = "Failure to fetch package resources."
         elif not package.installed:
             result = TestResult.FAILED
             lines = getLogFunc(package.build_log_path)
@@ -196,6 +200,8 @@ def test_install(parser, args):
                     fake=False)
             except InstallError:
                 pass
+            except FetchError:
+                pass
    
     jrf = JunitResultFormat()
     handled = {}
diff --git a/lib/spack/spack/stage.py b/lib/spack/spack/stage.py
index 008c5f04298..78930ecb5b2 100644
--- a/lib/spack/spack/stage.py
+++ b/lib/spack/spack/stage.py
@@ -261,7 +261,8 @@ def fetch(self):
                 tty.debug(e)
                 continue
         else:
-            tty.die("All fetchers failed for %s" % self.name)
+            errMessage = "All fetchers failed for %s" % self.name
+            raise fs.FetchError(errMessage, None)
 
 
     def check(self):
diff --git a/lib/spack/spack/test/unit_install.py b/lib/spack/spack/test/unit_install.py
index 2a94f8fec0a..c4b9092f051 100644
--- a/lib/spack/spack/test/unit_install.py
+++ b/lib/spack/spack/test/unit_install.py
@@ -105,6 +105,8 @@ def test_dependency_already_installed(self):
         
         self.assertEqual(mo.results, {bIdX:test_install.TestResult.PASSED})
 
+    #TODO: add test(s) where Y fails to install
+
 class MockPackageDb(object):
     def __init__(self, init=None):
         self.specToPkg = {}

From 50d0a2643bbef81ec2e97da209ae1974b6b77993 Mon Sep 17 00:00:00 2001
From: Peter Scheibel <scheibel1@llnl.gov>
Date: Tue, 27 Oct 2015 13:34:46 -0700
Subject: [PATCH 26/26] Not all package stages have an archive file (e.g.
 source code repos) but all of them do have a source_path: use this instead to
 check whether the package resources were successfully retrieved.

---
 lib/spack/spack/cmd/test-install.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/spack/spack/cmd/test-install.py b/lib/spack/spack/cmd/test-install.py
index 58ab40aa7b0..68b761d5dc8 100644
--- a/lib/spack/spack/cmd/test-install.py
+++ b/lib/spack/spack/cmd/test-install.py
@@ -133,7 +133,7 @@ def create_test_output(topSpec, newInstalls, output, getLogFunc=fetch_log):
             depBID = BuildId(dep)
             errOutput = "Skipped due to failed dependency: {0}".format(
                 depBID.stringId())
-        elif (not package.installed) and (not package.stage.archive_file):
+        elif (not package.installed) and (not package.stage.source_path):
             result = TestResult.FAILED
             errOutput = "Failure to fetch package resources."
         elif not package.installed: