From dfb02e6d45cbee33e76dab0b7006ea5873287b3d Mon Sep 17 00:00:00 2001
From: Gregory Becker <becker33@llnl.gov>
Date: Thu, 2 Apr 2020 12:30:52 -0700
Subject: [PATCH] test runner: add options to check installation dir and print
 purpose

---
 lib/spack/spack/package.py                    | 16 +++++++--
 .../builtin/packages/patchelf/package.py      | 33 ++++++++-----------
 2 files changed, 28 insertions(+), 21 deletions(-)

diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index f08e4f45972..4b8d35e1c6c 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -1670,7 +1670,8 @@ def test_process():
     def test(self):
         pass
 
-    def run_test(self, exe, options=[], expected=[], status=None):
+    def run_test(self, exe, options=[], expected=[], status=None,
+                 installed=False, purpose=''):
         """Run the test and confirm obtain the expected results
 
         Args:
@@ -1679,11 +1680,22 @@ def run_test(self, exe, options=[], expected=[], status=None):
             expected (list of str): list of expected output strings
             status (int or None): the expected process status if int or None
                 if the test is expected to succeed
+            installed (bool): the executable should be in the install prefix
+            purpose (str): message to display before running test
         """
         result = 'fail with status {0}'.format(status) if status else 'succeed'
         tty.debug('test: {0}: expect to {1}'.format(exe, result))
+
+        if purpose:
+            tty.msg(purpose)
+
         runner = which(exe)
-        assert runner is not None
+        assert runner is not None, "Failed to find executable '%s'" % exe
+
+        if installed:
+            msg = "Executable '%s' expected in prefix" % exe
+            msg += ", found in %s instead" % runner.path
+            assert self.spec.prefix in runner.path, msg
 
         try:
             output = runner(*options, output=str.split, error=str.split)
diff --git a/var/spack/repos/builtin/packages/patchelf/package.py b/var/spack/repos/builtin/packages/patchelf/package.py
index a71a1209484..1e1db37265f 100644
--- a/var/spack/repos/builtin/packages/patchelf/package.py
+++ b/var/spack/repos/builtin/packages/patchelf/package.py
@@ -2,13 +2,9 @@
 # Spack Project Developers. See the top-level COPYRIGHT file for details.
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
-
 from spack import *
-
 import os
 
-import llnl.util.tty as tty
-
 
 class Patchelf(AutotoolsPackage):
     """PatchELF is a small utility to modify the dynamic linker and RPATH of
@@ -24,21 +20,20 @@ class Patchelf(AutotoolsPackage):
     version('0.8',  sha256='14af06a2da688d577d64ff8dac065bb8903bbffbe01d30c62df7af9bf4ce72fe')
 
     def test(self):
-        patchelf = which('patchelf')
-        assert patchelf is not None
+        # Check patchelf in prefix and reports correct version
+        purpose_str = 'test patchelf is in prefix and reports correct version'
+        self.run_test('patchelf',
+                      options=['--version'],
+                      expected=['patchelf %s' % self.spec.version],
+                      installed=True,
+                      purpose=purpose_str)
 
-        tty.msg('test: Ensuring use of the installed executable')
-        patchelf_dir = os.path.dirname(patchelf.path)
-        assert patchelf_dir == self.prefix.bin
-
-        tty.msg('test: Checking version')
-        output = patchelf('--version', output=str.split, error=str.split)
-        assert output.strip() == 'patchelf {0}'.format(self.spec.version)
-
-        tty.msg('test: Ensuring the rpath is changed')
+        # Check the rpath is changed
         currdir = os.getcwd()
         hello_file = os.path.join(currdir, 'data', 'hello')
-        patchelf('--set-rpath', currdir, hello_file)
-        output = patchelf('--print-rpath', hello_file,
-                          output=str.split, error=str.split)
-        assert output.strip() == currdir
+        self.run_test('patchelf', ['--set-rpath', currdir, hello_file],
+                      purpose='test that patchelf can change rpath')
+        self.run_test('patchelf',
+                      options=['--print-rpath', hello_file],
+                      expected=[currdir],
+                      purpose='test that patchelf actually changed rpath')