From 41d375f6a47d94b59e7a48e8a8033eb1e4749aff Mon Sep 17 00:00:00 2001 From: Tamara Dahlgren <35777542+tldahlgren@users.noreply.github.com> Date: Thu, 14 Oct 2021 15:08:00 -0700 Subject: Stand-alone tests: disallow re-using an alias (#25881) It can be frustrating to successfully run `spack test run --alias ` only to find you cannot get the results because you already use `` in some previous stand-alone test execution. This PR prevents that from happening. --- lib/spack/spack/cmd/test.py | 6 ++++++ lib/spack/spack/install_test.py | 32 +++++++++++++++++++++++++------- lib/spack/spack/test/cmd/test.py | 20 +++++++++++++++++++- lib/spack/spack/test/test_suite.py | 32 ++++++++++++++++++++++++++++++++ 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) -- cgit v1.2.3-70-g09d2