tests: add a test to make sure that patched specs can be round-tripped

- previously, if a concrete sub-DAG with patched specs was written out
  and read back in, its patches would not be found because the dependent
  that patched it was no longer in the DAG.

- Add a test to ensure that the PatchCache handles this case.

- Also add tests to ensure that patch objects are properly created from
  Specs -- previously we only checked that the patches were on the Spec.

1 files changed, 101 insertions, 30 deletions

diff --git a/lib/spack/spack/test/patch.py b/lib/spack/spack/test/patch.py
index 061d12b3bc..33db0d2a39 100644
--- a/lib/spack/spack/test/patch.py
+++ b/lib/spack/spack/test/patch.py
@@ -16,6 +16,18 @@ from spack.util.executable import Executable
 from spack.stage import Stage
 from spack.spec import Spec
 
+# various sha256 sums (using variables for legibility)
+
+# files with contents 'foo', 'bar', and 'baz'
+foo_sha256 = 'b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c'
+bar_sha256 = '7d865e959b2466918c9863afca942d0fb89d7c9ac0c99bafc3749504ded97730'
+baz_sha256 = 'bf07a7fbb825fc0aae7bf4a1177b2b31fcf8a3feeaf7092761e18c859ee52a9c'
+
+# url patches
+url1_sha256 = 'abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234'
+url2_sha256 = '1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd'
+url2_archive_sha256 = 'abcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcd'
+
 
 @pytest.fixture()
 def mock_stage(tmpdir, monkeypatch):
@@ -84,9 +96,9 @@ def test_patch_in_spec(mock_packages, config):
     # Here the order is bar, foo, baz. Note that MV variants order
     # lexicographically based on the hash, not on the position of the
     # patch directive.
-    assert (('7d865e959b2466918c9863afca942d0fb89d7c9ac0c99bafc3749504ded97730',
-             'b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c',
-             'bf07a7fbb825fc0aae7bf4a1177b2b31fcf8a3feeaf7092761e18c859ee52a9c') ==
+    assert ((bar_sha256,
+             foo_sha256,
+             baz_sha256) ==
             spec.variants['patches'].value)
 
 
@@ -125,15 +137,14 @@ def test_multiple_patched_dependencies(mock_packages, config):
     # basic patch on libelf
     assert 'patches' in list(spec['libelf'].variants.keys())
     # foo
-    assert (('b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c',) ==
+    assert ((foo_sha256,) ==
             spec['libelf'].variants['patches'].value)
 
     # URL patches
     assert 'patches' in list(spec['fake'].variants.keys())
     # urlpatch.patch, urlpatch.patch.gz
-    assert (('1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd',
-             'abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234') ==
-            spec['fake'].variants['patches'].value)
+    assert (
+        (url2_sha256, url1_sha256) == spec['fake'].variants['patches'].value)
 
 
 def test_conditional_patched_dependencies(mock_packages, config):
@@ -144,49 +155,109 @@ def test_conditional_patched_dependencies(mock_packages, config):
     # basic patch on libelf
     assert 'patches' in list(spec['libelf'].variants.keys())
     # foo
-    assert (('b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c',) ==
+    assert ((foo_sha256,) ==
             spec['libelf'].variants['patches'].value)
 
     # conditional patch on libdwarf
     assert 'patches' in list(spec['libdwarf'].variants.keys())
     # bar
-    assert (('7d865e959b2466918c9863afca942d0fb89d7c9ac0c99bafc3749504ded97730',) ==
+    assert ((bar_sha256,) ==
             spec['libdwarf'].variants['patches'].value)
     # baz is conditional on libdwarf version
-    assert ('bf07a7fbb825fc0aae7bf4a1177b2b31fcf8a3feeaf7092761e18c859ee52a9c'
+    assert (baz_sha256
             not in spec['libdwarf'].variants['patches'].value)
 
     # URL patches
     assert 'patches' in list(spec['fake'].variants.keys())
     # urlpatch.patch, urlpatch.patch.gz
-    assert (('1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd',
-             'abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234') ==
-            spec['fake'].variants['patches'].value)
+    assert (
+        (url2_sha256, url1_sha256) == spec['fake'].variants['patches'].value)
 
 
-def test_conditional_patched_deps_with_conditions(mock_packages, config):
-    """Test whether conditional patched dependencies with conditions work."""
-    spec = Spec('patch-several-dependencies @1.0 ^libdwarf@20111030')
-    spec.concretize()
-
+def check_multi_dependency_patch_specs(
+        libelf, libdwarf, fake,  # specs
+        owner, package_dir):     # parent spec properties
+    """Validate patches on dependencies of patch-several-dependencies."""
     # basic patch on libelf
-    assert 'patches' in list(spec['libelf'].variants.keys())
+    assert 'patches' in list(libelf.variants.keys())
     # foo
-    assert ('b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c'
-            in spec['libelf'].variants['patches'].value)
+    assert (foo_sha256 in libelf.variants['patches'].value)
 
     # conditional patch on libdwarf
-    assert 'patches' in list(spec['libdwarf'].variants.keys())
+    assert 'patches' in list(libdwarf.variants.keys())
     # bar
-    assert ('7d865e959b2466918c9863afca942d0fb89d7c9ac0c99bafc3749504ded97730'
-            in spec['libdwarf'].variants['patches'].value)
+    assert (bar_sha256 in libdwarf.variants['patches'].value)
     # baz is conditional on libdwarf version (no guarantee on order w/conds)
-    assert ('bf07a7fbb825fc0aae7bf4a1177b2b31fcf8a3feeaf7092761e18c859ee52a9c'
-            in spec['libdwarf'].variants['patches'].value)
+    assert (baz_sha256 in libdwarf.variants['patches'].value)
+
+    def get_patch(spec, ending):
+        return next(p for p in spec.patches if p.path_or_url.endswith(ending))
+
+    # make sure file patches are reconstructed properly
+    foo_patch = get_patch(libelf, 'foo.patch')
+    bar_patch = get_patch(libdwarf, 'bar.patch')
+    baz_patch = get_patch(libdwarf, 'baz.patch')
+
+    assert foo_patch.owner == owner
+    assert foo_patch.path == os.path.join(package_dir, 'foo.patch')
+    assert foo_patch.sha256 == foo_sha256
+
+    assert bar_patch.owner == 'builtin.mock.patch-several-dependencies'
+    assert bar_patch.path == os.path.join(package_dir, 'bar.patch')
+    assert bar_patch.sha256 == bar_sha256
+
+    assert baz_patch.owner == 'builtin.mock.patch-several-dependencies'
+    assert baz_patch.path == os.path.join(package_dir, 'baz.patch')
+    assert baz_patch.sha256 == baz_sha256
 
     # URL patches
-    assert 'patches' in list(spec['fake'].variants.keys())
+    assert 'patches' in list(fake.variants.keys())
     # urlpatch.patch, urlpatch.patch.gz
-    assert (('1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd',
-             'abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234') ==
-            spec['fake'].variants['patches'].value)
+    assert (url2_sha256, url1_sha256) == fake.variants['patches'].value
+
+    url1_patch = get_patch(fake, 'urlpatch.patch')
+    url2_patch = get_patch(fake, 'urlpatch2.patch.gz')
+
+    assert url1_patch.owner == 'builtin.mock.patch-several-dependencies'
+    assert url1_patch.url == 'http://example.com/urlpatch.patch'
+    assert url1_patch.sha256 == url1_sha256
+
+    assert url2_patch.owner == 'builtin.mock.patch-several-dependencies'
+    assert url2_patch.url == 'http://example.com/urlpatch2.patch.gz'
+    assert url2_patch.sha256 == url2_sha256
+    assert url2_patch.archive_sha256 == url2_archive_sha256
+
+
+def test_conditional_patched_deps_with_conditions(mock_packages, config):
+    """Test whether conditional patched dependencies with conditions work."""
+    spec = Spec('patch-several-dependencies @1.0 ^libdwarf@20111030')
+    spec.concretize()
+
+    libelf = spec['libelf']
+    libdwarf = spec['libdwarf']
+    fake = spec['fake']
+
+    check_multi_dependency_patch_specs(
+        libelf, libdwarf, fake,
+        'builtin.mock.patch-several-dependencies',
+        spec.package.package_dir)
+
+
+def test_write_and_read_sub_dags_with_patched_deps(mock_packages, config):
+    """Test whether patched dependencies are still correct after writing and
+       reading a sub-DAG of a concretized Spec.
+    """
+    spec = Spec('patch-several-dependencies @1.0 ^libdwarf@20111030')
+    spec.concretize()
+
+    # write to YAML and read back in -- new specs will *only* contain
+    # their sub-DAGs, and won't contain the dependent that patched them
+    libelf = spack.spec.Spec.from_yaml(spec['libelf'].to_yaml())
+    libdwarf = spack.spec.Spec.from_yaml(spec['libdwarf'].to_yaml())
+    fake = spack.spec.Spec.from_yaml(spec['fake'].to_yaml())
+
+    # make sure we can still read patches correctly for these specs
+    check_multi_dependency_patch_specs(
+        libelf, libdwarf, fake,
+        'builtin.mock.patch-several-dependencies',
+        spec.package.package_dir)