spack diff: more flexible tests, restore transitive diff with spec_clauses

In switching to hash facts for concrete specs, we lost the transitive facts
from dependencies. This was fine for solves, because they were implied by
the imposed constraints from every hash. However, for `spack diff`, we want
to see what the hashes mean, so we need another mode for `spec_clauses()` to
show that.

This adds a `expand_hashes` argument to `spec_clauses()` that allows us to
output *both* the hashes and their implications on dependencies. We use
this mode in `spack diff`.

lib/spack/spack/cmd/diff.py

@@ -68,8 +68,8 @@ def compare_specs(a, b, to_string=False, color=None):
     # Prepare a solver setup to parse differences
     setup = asp.SpackSolverSetup()
 
-    a_facts = set(t for t in setup.spec_clauses(a, body=True))
+    a_facts = set(t for t in setup.spec_clauses(a, body=True, expand_hashes=True))
-    b_facts = set(t for t in setup.spec_clauses(b, body=True))
+    b_facts = set(t for t in setup.spec_clauses(b, body=True, expand_hashes=True))
 
     # We want to present them to the user as simple key: values
     intersect = sorted(a_facts.intersection(b_facts))

lib/spack/spack/solver/asp.py

@@ -1081,7 +1081,7 @@ def spec_clauses(self, *args, **kwargs):
             raise RuntimeError(msg)
         return clauses
 
-    def _spec_clauses(self, spec, body=False, transitive=True):
+    def _spec_clauses(self, spec, body=False, transitive=True, expand_hashes=False):
         """Return a list of clauses for a spec mandates are true.
 
         Arguments:
@@ -1089,7 +1089,15 @@ def _spec_clauses(self, spec, body=False, transitive=True):
             body (bool): if True, generate clauses to be used in rule bodies
                 (final values) instead of rule heads (setters).
             transitive (bool): if False, don't generate clauses from
-                 dependencies (default True)
+                dependencies (default True)
+            expand_hashes (bool): if True, descend into hashes of concrete specs
+                (default False)
+
+        Normally, if called with ``transitive=True``, ``spec_clauses()`` just generates
+        hashes for the dependency requirements of concrete specs. If ``expand_hashes``
+        is ``True``, we'll *also* output all the facts implied by transitive hashes,
+        which are redundant during a solve but useful outside of one (e.g.,
+        for spec ``diff``).
         """
         clauses = []
 
@@ -1200,7 +1208,7 @@ class Body(object):
             for dep in spec.traverse(root=False):
                 if spec.concrete:
                     clauses.append(fn.hash(dep.name, dep.dag_hash()))
-                else:
+                if not spec.concrete or expand_hashes:
                     clauses.extend(
                         self._spec_clauses(dep, body, transitive=False)
                     )

lib/spack/spack/test/cmd/diff.py

@@ -61,12 +61,30 @@ def test_load_first(install_mockery, mock_fetch, mock_archive, mock_packages):
     output = diff_cmd('--json', 'mpileaks', 'mpileaks')
     result = sjson.load(output)
 
-    assert len(result['a_not_b']) == 0
+    assert not result['a_not_b']
-    assert len(result['b_not_a']) == 0
+    assert not result['b_not_a']
 
     assert 'mpileaks' in result['a_name']
     assert 'mpileaks' in result['b_name']
-    assert "intersect" in result and len(result['intersect']) > 50
+
+    # spot check attributes in the intersection to ensure they describe the spec
+    assert "intersect" in result
+    assert all(["node", dep] in result["intersect"] for dep in (
+        "mpileaks", "callpath", "dyninst", "libelf", "libdwarf", "mpich"
+    ))
+    assert all(
+        len([diff for diff in result["intersect"] if diff[0] == attr]) == 6
+        for attr in (
+            "version",
+            "node_target",
+            "node_platform",
+            "node_os",
+            "node_compiler",
+            "node_compiler_version",
+            "node",
+            "hash",
+        )
+    )
 
     # After we install another version, it should ask us to disambiguate
     install_cmd('mpileaks+debug')
@@ -87,7 +105,8 @@ def test_load_first(install_mockery, mock_fetch, mock_archive, mock_packages):
                       "mpileaks/{0}".format(no_debug_hash))
     result = sjson.load(output)
 
-    assert len(result['a_not_b']) == 1
+    assert ['hash', 'mpileaks %s' % debug_hash] in result['a_not_b']
-    assert len(result['b_not_a']) == 1
+    assert ['variant_value', 'mpileaks debug True'] in result['a_not_b']
-    assert result['a_not_b'][0] == ['variant_value', 'mpileaks debug True']
+
-    assert result['b_not_a'][0] == ['variant_value', 'mpileaks debug False']
+    assert ['hash', 'mpileaks %s' % no_debug_hash] in result['b_not_a']
+    assert ['variant_value', 'mpileaks debug False'] in result['b_not_a']