Stand-alone tests: disallow re-using an alias (#25881)

It can be frustrating to successfully run `spack test run --alias <name>` only to find you cannot get the results because you already use `<name>` in some previous stand-alone test execution.  This PR prevents that from happening.

4 files changed, 82 insertions, 8 deletions

diff --git a/lib/spack/spack/cmd/test.py b/lib/spack/spack/cmd/test.py
index d2a5e7214b..3eff089ac8 100644
--- a/lib/spack/spack/cmd/test.py
+++ b/lib/spack/spack/cmd/test.py
@@ -137,6 +137,12 @@ def test_run(args):
     If no specs are listed, run tests for all packages in the current
     environment or all installed packages if there is no active environment.
     """
+    if args.alias:
+        suites = spack.install_test.get_named_test_suites(args.alias)
+        if suites:
+            tty.die('Test suite "{0}" already exists. Try another alias.'
+                    .format(args.alias))
+
     # cdash help option
     if args.help_cdash:
         parser = argparse.ArgumentParser(
diff --git a/lib/spack/spack/install_test.py b/lib/spack/spack/install_test.py
index 1fc112d896..53fd07d7bb 100644
--- a/lib/spack/spack/install_test.py
+++ b/lib/spack/spack/install_test.py
@@ -64,18 +64,33 @@ def get_all_test_suites():
     return test_suites
 
 
-def get_test_suite(name):
-    assert name, "Cannot search for empty test name or 'None'"
+def get_named_test_suites(name):
+    """Return a list of the names of any test suites with that name."""
+    if not name:
+        raise TestSuiteNameError('Test suite name is required.')
+
     test_suites = get_all_test_suites()
-    names = [ts for ts in test_suites
-             if ts.name == name]
-    assert len(names) < 2, "alias shadows test suite hash"
+    return [ts for ts in test_suites if ts.name == name]
+
+
+def get_test_suite(name):
+    names = get_named_test_suites(name)
+    if len(names) > 1:
+        raise TestSuiteNameError(
+            'Too many suites named "{0}".  May shadow hash.'.format(name)
+        )
 
     if not names:
         return None
     return names[0]
 
 
+def write_test_suite_file(suite):
+    """Write the test suite to its lock file."""
+    with open(suite.stage.join(test_suite_filename), 'w') as f:
+        sjson.dump(suite.to_dict(), stream=f)
+
+
 class TestSuite(object):
     def __init__(self, specs, alias=None):
         # copy so that different test suites have different package objects
@@ -250,8 +265,7 @@ class TestSuite(object):
                     except spack.repo.UnknownPackageError:
                         pass  # not all virtuals have package files
 
-        with open(self.stage.join(test_suite_filename), 'w') as f:
-            sjson.dump(self.to_dict(), stream=f)
+        write_test_suite_file(self)
 
     def to_dict(self):
         specs = [s.to_dict() for s in self.specs]
@@ -310,3 +324,7 @@ class TestSuiteFailure(spack.error.SpackError):
 
 class TestSuiteSpecError(spack.error.SpackError):
     """Raised when there is an issue associated with the spec being tested."""
+
+
+class TestSuiteNameError(spack.error.SpackError):
+    """Raised when there is an issue with the naming of the test suite."""
diff --git a/lib/spack/spack/test/cmd/test.py b/lib/spack/spack/test/cmd/test.py
index 33dd81816b..df9fb35db1 100644
--- a/lib/spack/spack/test/cmd/test.py
+++ b/lib/spack/spack/test/cmd/test.py
@@ -39,11 +39,29 @@ def test_test_dirty_flag(arguments, expected):
     assert args.dirty == expected
 
 
+def test_test_dup_alias(
+        mock_test_stage, mock_packages, mock_archive, mock_fetch,
+        install_mockery_mutable_config, capfd):
+    """Ensure re-using an alias fails with suggestion to change."""
+    install('libdwarf')
+
+    # Run the tests with the alias once
+    out = spack_test('run', '--alias', 'libdwarf', 'libdwarf')
+    assert "Spack test libdwarf" in out
+
+    # Try again with the alias but don't let it fail on the error
+    with capfd.disabled():
+        out = spack_test(
+            'run', '--alias', 'libdwarf', 'libdwarf', fail_on_error=False)
+
+    assert "already exists" in out
+
+
 def test_test_output(mock_test_stage, mock_packages, mock_archive, mock_fetch,
                      install_mockery_mutable_config):
     """Ensure output printed from pkgs is captured by output redirection."""
     install('printing-package')
-    spack_test('run', 'printing-package')
+    spack_test('run', '--alias', 'printpkg', 'printing-package')
 
     stage_files = os.listdir(mock_test_stage)
     assert len(stage_files) == 1
diff --git a/lib/spack/spack/test/test_suite.py b/lib/spack/spack/test/test_suite.py
index c71a063c50..27413b88e7 100644
--- a/lib/spack/spack/test/test_suite.py
+++ b/lib/spack/spack/test/test_suite.py
@@ -119,3 +119,35 @@ def test_test_spec_run_once(mock_packages, install_mockery, mock_test_stage):
 
     with pytest.raises(spack.install_test.TestSuiteFailure):
         test_suite()
+
+
+def test_get_test_suite():
+    assert not spack.install_test.get_test_suite('nothing')
+
+
+def test_get_test_suite_no_name(mock_packages, mock_test_stage):
+    with pytest.raises(spack.install_test.TestSuiteNameError) as exc_info:
+        spack.install_test.get_test_suite('')
+
+    assert 'name is required' in str(exc_info)
+
+
+def test_get_test_suite_too_many(mock_packages, mock_test_stage):
+    test_suites = []
+    name = 'duplicate-alias'
+
+    def add_suite(package):
+        spec = spack.spec.Spec(package).concretized()
+        suite = spack.install_test.TestSuite([spec], name)
+        suite.ensure_stage()
+        spack.install_test.write_test_suite_file(suite)
+        test_suites.append(suite)
+
+    add_suite('libdwarf')
+    suite = spack.install_test.get_test_suite(name)
+    assert suite.alias == name
+
+    add_suite('libelf')
+    with pytest.raises(spack.install_test.TestSuiteNameError) as exc_info:
+        spack.install_test.get_test_suite(name)
+    assert 'many suites named' in str(exc_info)