From 645b40b2490cf3e39b462d10d79cb72d2f710cde Mon Sep 17 00:00:00 2001 From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Fri, 21 Jan 2022 03:24:12 -0500 Subject: add six.raise_from() to preserve exception traceback (#28532) * add six.raise_from() to preserve exception traceback * add tests for code coverage --- lib/spack/spack/config.py | 3 +- lib/spack/spack/database.py | 5 +- lib/spack/spack/directory_layout.py | 5 +- lib/spack/spack/install_test.py | 9 ++-- lib/spack/spack/mirror.py | 37 +++++++++++--- lib/spack/spack/s3_handler.py | 7 +-- lib/spack/spack/spec.py | 18 +++++-- lib/spack/spack/test/ci.py | 25 ++++++++++ lib/spack/spack/test/mirror.py | 96 +++++++++++++++++++++++++++++++++++++ lib/spack/spack/test/spec_yaml.py | 30 +++++++++++- 10 files changed, 213 insertions(+), 22 deletions(-) diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py index 5f94937c66..6062634ca5 100644 --- a/lib/spack/spack/config.py +++ b/lib/spack/spack/config.py @@ -39,6 +39,7 @@ from contextlib import contextmanager from typing import List # novm import ruamel.yaml as yaml +import six from ruamel.yaml.error import MarkedYAMLError from six import iteritems @@ -975,7 +976,7 @@ def validate(data, schema, filename=None): line_number = e.instance.lc.line + 1 else: line_number = None - raise ConfigFormatError(e, data, filename, line_number) + raise six.raise_from(ConfigFormatError(e, data, filename, line_number), e) # return the validated data so that we can access the raw data # mostly relevant for environments return test_data diff --git a/lib/spack/spack/database.py b/lib/spack/spack/database.py index 29674d5178..6c2ac7001c 100644 --- a/lib/spack/spack/database.py +++ b/lib/spack/spack/database.py @@ -734,7 +734,10 @@ class Database(object): with open(filename, 'r') as f: fdata = sjson.load(f) except Exception as e: - raise CorruptDatabaseError("error parsing database:", str(e)) + raise six.raise_from( + CorruptDatabaseError("error parsing database:", str(e)), + e, + ) if fdata is None: return diff --git a/lib/spack/spack/directory_layout.py b/lib/spack/spack/directory_layout.py index e009657e7c..be9e7b997b 100644 --- a/lib/spack/spack/directory_layout.py +++ b/lib/spack/spack/directory_layout.py @@ -12,6 +12,7 @@ import tempfile from contextlib import contextmanager import ruamel.yaml as yaml +import six import llnl.util.filesystem as fs import llnl.util.tty as tty @@ -344,13 +345,13 @@ class DirectoryLayout(object): os.unlink(path) os.remove(metapath) except OSError as e: - raise RemoveFailedError(spec, path, e) + raise six.raise_from(RemoveFailedError(spec, path, e), e) elif os.path.exists(path): try: shutil.rmtree(path) except OSError as e: - raise RemoveFailedError(spec, path, e) + raise six.raise_from(RemoveFailedError(spec, path, e), e) path = os.path.dirname(path) while path != self.root: diff --git a/lib/spack/spack/install_test.py b/lib/spack/spack/install_test.py index 0d27869bea..660135422d 100644 --- a/lib/spack/spack/install_test.py +++ b/lib/spack/spack/install_test.py @@ -9,8 +9,9 @@ import re import shutil import sys +import six + import llnl.util.filesystem as fs -import llnl.util.tty as tty import spack.error import spack.paths @@ -292,8 +293,10 @@ class TestSuite(object): test_suite._hash = content_hash return test_suite except Exception as e: - tty.debug(e) - raise sjson.SpackJSONError("error parsing JSON TestSuite:", str(e)) + raise six.raise_from( + sjson.SpackJSONError("error parsing JSON TestSuite:", str(e)), + e, + ) def _add_msg_to_file(filename, msg): diff --git a/lib/spack/spack/mirror.py b/lib/spack/spack/mirror.py index b4ec1f3e78..3b8a60992c 100644 --- a/lib/spack/spack/mirror.py +++ b/lib/spack/spack/mirror.py @@ -69,6 +69,10 @@ class Mirror(object): self._push_url = push_url self._name = name + def __eq__(self, other): + return (self._fetch_url == other._fetch_url and + self._push_url == other._push_url) + def to_json(self, stream=None): return sjson.dump(self.to_dict(), stream) @@ -81,12 +85,21 @@ class Mirror(object): data = syaml.load(stream) return Mirror.from_dict(data, name) except yaml_error.MarkedYAMLError as e: - raise syaml.SpackYAMLError("error parsing YAML spec:", str(e)) + raise six.raise_from( + syaml.SpackYAMLError("error parsing YAML mirror:", str(e)), + e, + ) @staticmethod def from_json(stream, name=None): - d = sjson.load(stream) - return Mirror.from_dict(d, name) + try: + d = sjson.load(stream) + return Mirror.from_dict(d, name) + except Exception as e: + raise six.raise_from( + sjson.SpackJSONError("error parsing JSON mirror:", str(e)), + e, + ) def to_dict(self): if self._push_url is None: @@ -238,6 +251,9 @@ class MirrorCollection(Mapping): mirrors.items() if mirrors is not None else spack.config.get('mirrors', scope=scope).items())) + def __eq__(self, other): + return self._mirrors == other._mirrors + def to_json(self, stream=None): return sjson.dump(self.to_dict(True), stream) @@ -251,12 +267,21 @@ class MirrorCollection(Mapping): data = syaml.load(stream) return MirrorCollection(data) except yaml_error.MarkedYAMLError as e: - raise syaml.SpackYAMLError("error parsing YAML spec:", str(e)) + raise six.raise_from( + syaml.SpackYAMLError("error parsing YAML mirror collection:", str(e)), + e, + ) @staticmethod def from_json(stream, name=None): - d = sjson.load(stream) - return MirrorCollection(d) + try: + d = sjson.load(stream) + return MirrorCollection(d) + except Exception as e: + raise six.raise_from( + sjson.SpackJSONError("error parsing JSON mirror collection:", str(e)), + e, + ) def to_dict(self, recursive=False): return syaml_dict(sorted( diff --git a/lib/spack/spack/s3_handler.py b/lib/spack/spack/s3_handler.py index b7250f5fc5..66fa73e07a 100644 --- a/lib/spack/spack/s3_handler.py +++ b/lib/spack/spack/s3_handler.py @@ -5,6 +5,7 @@ from io import BufferedReader +import six import six.moves.urllib.error as urllib_error import six.moves.urllib.request as urllib_request import six.moves.urllib.response as urllib_response @@ -79,11 +80,11 @@ class UrllibS3Handler(urllib_request.HTTPSHandler): except ClientError as err2: if err.response['Error']['Code'] == 'NoSuchKey': # raise original error - raise urllib_error.URLError(err) + raise six.raise_from(urllib_error.URLError(err), err) - raise urllib_error.URLError(err2) + raise six.raise_from(urllib_error.URLError(err2), err2) - raise urllib_error.URLError(err) + raise six.raise_from(urllib_error.URLError(err), err) S3OpenerDirector = urllib_request.build_opener(UrllibS3Handler()) diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index 580064fca4..aa9cd6a709 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -2183,7 +2183,10 @@ class Spec(object): data = yaml.load(stream) return Spec.from_dict(data) except yaml.error.MarkedYAMLError as e: - raise syaml.SpackYAMLError("error parsing YAML spec:", str(e)) + raise six.raise_from( + syaml.SpackYAMLError("error parsing YAML spec:", str(e)), + e, + ) @staticmethod def from_json(stream): @@ -2196,8 +2199,10 @@ class Spec(object): data = sjson.load(stream) return Spec.from_dict(data) except Exception as e: - tty.debug(e) - raise sjson.SpackJSONError("error parsing JSON spec:", str(e)) + raise six.raise_from( + sjson.SpackJSONError("error parsing JSON spec:", str(e)), + e, + ) @staticmethod def from_detection(spec_str, extra_attributes=None): @@ -2734,7 +2739,10 @@ class Spec(object): # with inconsistent constraints. Users cannot produce # inconsistent specs like this on the command line: the # parser doesn't allow it. Spack must be broken! - raise InconsistentSpecError("Invalid Spec DAG: %s" % e.message) + raise six.raise_from( + InconsistentSpecError("Invalid Spec DAG: %s" % e.message), + e, + ) def index(self, deptype='all'): """Return DependencyMap that points to all the dependencies in this @@ -4767,7 +4775,7 @@ class SpecParser(spack.parse.Parser): self.unexpected_token() except spack.parse.ParseError as e: - raise SpecParseError(e) + raise six.raise_from(SpecParseError(e), e) # Generate lookups for git-commit-based versions for spec in specs: diff --git a/lib/spack/spack/test/ci.py b/lib/spack/spack/test/ci.py index 69f156fd39..e054ccf14f 100644 --- a/lib/spack/spack/test/ci.py +++ b/lib/spack/spack/test/ci.py @@ -365,6 +365,31 @@ def test_setup_spack_repro_version(tmpdir, capfd, last_two_git_commits, assert('Unable to merge {0}'.format(c1) in err) +@pytest.mark.parametrize( + "obj, proto", + [ + ({}, []), + ], +) +def test_ci_opt_argument_checking(obj, proto): + """Check that matches() and subkeys() return False when `proto` is not a dict.""" + assert not ci_opt.matches(obj, proto) + assert not ci_opt.subkeys(obj, proto) + + +@pytest.mark.parametrize( + "yaml", + [ + {'extends': 1}, + ], +) +def test_ci_opt_add_extends_non_sequence(yaml): + """Check that add_extends() exits if 'extends' is not a sequence.""" + yaml_copy = yaml.copy() + ci_opt.add_extends(yaml, None) + assert yaml == yaml_copy + + def test_ci_workarounds(): fake_root_spec = 'x' * 544 fake_spack_ref = 'x' * 40 diff --git a/lib/spack/spack/test/mirror.py b/lib/spack/spack/test/mirror.py index ba04f21d43..d8684d13b9 100644 --- a/lib/spack/spack/test/mirror.py +++ b/lib/spack/spack/test/mirror.py @@ -13,9 +13,11 @@ from llnl.util.filesystem import resolve_link_target_relative_to_the_link import spack.mirror import spack.repo import spack.util.executable +import spack.util.spack_json as sjson from spack.spec import Spec from spack.stage import Stage from spack.util.executable import which +from spack.util.spack_yaml import SpackYAMLError pytestmark = pytest.mark.usefixtures('mutable_config', 'mutable_mock_repo') @@ -149,6 +151,100 @@ def test_all_mirror( repos.clear() +@pytest.mark.parametrize( + "mirror", + [ + spack.mirror.Mirror( + 'https://example.com/fetch', + 'https://example.com/push', + ), + ], +) +def test_roundtrip_mirror(mirror): + mirror_yaml = mirror.to_yaml() + assert spack.mirror.Mirror.from_yaml(mirror_yaml) == mirror + mirror_json = mirror.to_json() + assert spack.mirror.Mirror.from_json(mirror_json) == mirror + + +@pytest.mark.parametrize( + "invalid_yaml", + [ + "playing_playlist: {{ action }} playlist {{ playlist_name }}" + ] +) +def test_invalid_yaml_mirror(invalid_yaml): + with pytest.raises(SpackYAMLError) as e: + spack.mirror.Mirror.from_yaml(invalid_yaml) + exc_msg = str(e.value) + assert exc_msg.startswith("error parsing YAML mirror:") + assert invalid_yaml in exc_msg + + +@pytest.mark.parametrize( + "invalid_json, error_message", + [ + ("{13:", "Expecting property name") + ] +) +def test_invalid_json_mirror(invalid_json, error_message): + with pytest.raises(sjson.SpackJSONError) as e: + spack.mirror.Mirror.from_json(invalid_json) + exc_msg = str(e.value) + assert exc_msg.startswith("error parsing JSON mirror:") + assert error_message in exc_msg + + +@pytest.mark.parametrize( + "mirror_collection", + [ + spack.mirror.MirrorCollection( + mirrors={ + 'example-mirror': spack.mirror.Mirror( + 'https://example.com/fetch', + 'https://example.com/push', + ).to_dict(), + }, + ), + ], +) +def test_roundtrip_mirror_collection(mirror_collection): + mirror_collection_yaml = mirror_collection.to_yaml() + assert (spack.mirror.MirrorCollection.from_yaml(mirror_collection_yaml) == + mirror_collection) + mirror_collection_json = mirror_collection.to_json() + assert (spack.mirror.MirrorCollection.from_json(mirror_collection_json) == + mirror_collection) + + +@pytest.mark.parametrize( + "invalid_yaml", + [ + "playing_playlist: {{ action }} playlist {{ playlist_name }}" + ] +) +def test_invalid_yaml_mirror_collection(invalid_yaml): + with pytest.raises(SpackYAMLError) as e: + spack.mirror.MirrorCollection.from_yaml(invalid_yaml) + exc_msg = str(e.value) + assert exc_msg.startswith("error parsing YAML mirror collection:") + assert invalid_yaml in exc_msg + + +@pytest.mark.parametrize( + "invalid_json, error_message", + [ + ("{13:", "Expecting property name") + ] +) +def test_invalid_json_mirror_collection(invalid_json, error_message): + with pytest.raises(sjson.SpackJSONError) as e: + spack.mirror.MirrorCollection.from_json(invalid_json) + exc_msg = str(e.value) + assert exc_msg.startswith("error parsing JSON mirror collection:") + assert error_message in exc_msg + + def test_mirror_archive_paths_no_version(mock_packages, config, mock_archive): spec = Spec('trivial-install-test-package@nonexistingversion') fetcher = spack.fetch_strategy.URLFetchStrategy(mock_archive.url) diff --git a/lib/spack/spack/test/spec_yaml.py b/lib/spack/spack/test/spec_yaml.py index ee4b8bc99b..ce27c24629 100644 --- a/lib/spack/spack/test/spec_yaml.py +++ b/lib/spack/spack/test/spec_yaml.py @@ -23,7 +23,7 @@ import spack.version from spack import repo from spack.spec import Spec, save_dependency_specfiles from spack.util.mock_package import MockPackageMultiRepo -from spack.util.spack_yaml import syaml_dict +from spack.util.spack_yaml import SpackYAMLError, syaml_dict if sys.version_info >= (3, 3): from collections.abc import Iterable, Mapping # novm @@ -56,6 +56,34 @@ def test_normal_spec(mock_packages): check_json_round_trip(spec) +@pytest.mark.parametrize( + "invalid_yaml", + [ + "playing_playlist: {{ action }} playlist {{ playlist_name }}" + ] +) +def test_invalid_yaml_spec(invalid_yaml): + with pytest.raises(SpackYAMLError) as e: + Spec.from_yaml(invalid_yaml) + exc_msg = str(e.value) + assert exc_msg.startswith("error parsing YAML spec:") + assert invalid_yaml in exc_msg + + +@pytest.mark.parametrize( + "invalid_json, error_message", + [ + ("{13:", "Expecting property name") + ] +) +def test_invalid_json_spec(invalid_json, error_message): + with pytest.raises(sjson.SpackJSONError) as e: + Spec.from_json(invalid_json) + exc_msg = str(e.value) + assert exc_msg.startswith("error parsing JSON spec:") + assert error_message in exc_msg + + def test_external_spec(config, mock_packages): spec = Spec('externaltool') spec.concretize() -- cgit v1.2.3-60-g2f50