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

@@ -39,6 +39,7 @@
 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

lib/spack/spack/database.py

@@ -734,7 +734,10 @@ def _read_from_file(self, filename):
             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

lib/spack/spack/directory_layout.py

@@ -12,6 +12,7 @@
 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 @@ def remove_install_directory(self, spec, deprecated=False):
                     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:

lib/spack/spack/install_test.py

@@ -9,8 +9,9 @@
 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 @@ def from_file(filename):
                 test_suite._hash = content_hash
                 return test_suite
         except Exception as e:
-            tty.debug(e)
+            raise six.raise_from(
-            raise sjson.SpackJSONError("error parsing JSON TestSuite:", str(e))
+                sjson.SpackJSONError("error parsing JSON TestSuite:", str(e)),
+                e,
+            )
 
 
 def _add_msg_to_file(filename, msg):

lib/spack/spack/mirror.py

@@ -69,6 +69,10 @@ def __init__(self, fetch_url, push_url=None,
         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 @@ def from_yaml(stream, name=None):
             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)
+        try:
-        return Mirror.from_dict(d, name)
+            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 @@ def __init__(self, mirrors=None, scope=None):
                 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 @@ def from_yaml(stream, name=None):
             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)
+        try:
-        return MirrorCollection(d)
+            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(

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 @@ def s3_open(self, req):
                 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())

lib/spack/spack/spec.py

@@ -2183,7 +2183,10 @@ def from_yaml(stream):
             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 @@ def from_json(stream):
             data = sjson.load(stream)
             return Spec.from_dict(data)
         except Exception as e:
-            tty.debug(e)
+            raise six.raise_from(
-            raise sjson.SpackJSONError("error parsing JSON spec:", str(e))
+                sjson.SpackJSONError("error parsing JSON spec:", str(e)),
+                e,
+            )
 
     @staticmethod
     def from_detection(spec_str, extra_attributes=None):
@@ -2734,7 +2739,10 @@ def flat_dependencies(self, **kwargs):
             # 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 @@ def do_parse(self):
                         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:

lib/spack/spack/test/ci.py

@@ -365,6 +365,31 @@ def __call__(self, *args, **kwargs):
     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

lib/spack/spack/test/mirror.py

@@ -13,9 +13,11 @@
 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)

lib/spack/spack/test/spec_yaml.py

@@ -23,7 +23,7 @@
 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()