fix sorting issue with ci workarounds (#17277)

2 files changed, 38 insertions, 30 deletions

diff --git a/lib/spack/spack/ci_optimization.py b/lib/spack/spack/ci_optimization.py
index d938a681e3..c5e69c649c 100644
--- a/lib/spack/spack/ci_optimization.py
+++ b/lib/spack/spack/ci_optimization.py
@@ -17,6 +17,19 @@ import hashlib
 import spack.util.spack_yaml as syaml
 
 
+def sort_yaml_obj(obj):
+    if isinstance(obj, collections_abc.Mapping):
+        return syaml.syaml_dict(
+            (k, sort_yaml_obj(v))
+            for k, v in
+            sorted(obj.items(), key=(lambda item: str(item[0]))))
+
+    if isinstance(obj, collections_abc.Sequence) and not isinstance(obj, str):
+        return syaml.syaml_list(sort_yaml_obj(x) for x in obj)
+
+    return obj
+
+
 def matches(obj, proto):
     """Returns True if the test object "obj" matches the prototype object
     "proto".
@@ -235,8 +248,10 @@ def try_optimization_pass(name, yaml, optimization_pass, *args, **kwargs):
         # pass was not applied
         return (yaml, new_yaml, False, other_results)
 
-    pre_size = len(syaml.dump_config(yaml, default_flow_style=True))
-    post_size = len(syaml.dump_config(new_yaml, default_flow_style=True))
+    pre_size = len(syaml.dump_config(
+        sort_yaml_obj(yaml), default_flow_style=True))
+    post_size = len(syaml.dump_config(
+        sort_yaml_obj(new_yaml), default_flow_style=True))
 
     # pass makes the size worse: not applying
     applied = (post_size <= pre_size)
@@ -281,7 +296,7 @@ def build_histogram(iterator, key):
             continue
 
         value_hash = hashlib.sha1()
-        value_hash.update(syaml.dump_config(val).encode())
+        value_hash.update(syaml.dump_config(sort_yaml_obj(val)).encode())
         value_hash = value_hash.hexdigest()
 
         buckets[value_hash] += 1
@@ -292,7 +307,8 @@ def build_histogram(iterator, key):
 
 
 def optimizer(yaml):
-    original_size = len(syaml.dump_config(yaml, default_flow_style=True))
+    original_size = len(syaml.dump_config(
+        sort_yaml_obj(yaml), default_flow_style=True))
 
     # try factoring out commonly repeated portions
     common_job = {
@@ -369,7 +385,8 @@ def optimizer(yaml):
             common_subobject,
             {'variables': {'SPACK_ROOT_SPEC': spec}})
 
-    new_size = len(syaml.dump_config(yaml, default_flow_style=True))
+    new_size = len(syaml.dump_config(
+        sort_yaml_obj(yaml), default_flow_style=True))
 
     print('\n')
     print_delta('overall summary', original_size, new_size)
diff --git a/lib/spack/spack/test/ci.py b/lib/spack/spack/test/ci.py
index 9b3c48654a..a2ad2708a1 100644
--- a/lib/spack/spack/test/ci.py
+++ b/lib/spack/spack/test/ci.py
@@ -188,8 +188,6 @@ def test_ci_workarounds():
         'SPACK_IS_PR_PIPELINE': 'False',
     }
 
-    common_script = ['spack ci rebuild']
-
     common_before_script = [
         'git clone "https://github.com/spack/spack"',
         ' && '.join((
@@ -219,14 +217,14 @@ def test_ci_workarounds():
             },
             'retry': {'max': 2, 'when': ['always']},
             'after_script': ['rm -rf "./spack"'],
-            'image': {'name': 'spack/centos7', 'entrypoint': ['']},
+            'script': ['spack ci rebuild'],
+            'image': {'name': 'spack/centos7', 'entrypoint': ['']}
         }
 
         if optimize:
-            result['extends'] = ['.c0', '.c1', '.c2']
+            result['extends'] = ['.c0', '.c1']
         else:
             variables['SPACK_ROOT_SPEC'] = fake_root_spec
-            result['script'] = common_script
             result['before_script'] = common_before_script
 
         result['variables'] = variables
@@ -254,7 +252,7 @@ def test_ci_workarounds():
         }
 
         if optimize:
-            result['extends'] = '.c1'
+            result['extends'] = '.c0'
         else:
             result['before_script'] = common_before_script
 
@@ -262,11 +260,16 @@ def test_ci_workarounds():
 
     def make_factored_jobs(optimize):
         return {
-            '.c0': {'script': common_script},
-            '.c1': {'before_script': common_before_script},
-            '.c2': {'variables': {'SPACK_ROOT_SPEC': fake_root_spec}}
+            '.c0': {'before_script': common_before_script},
+            '.c1': {'variables': {'SPACK_ROOT_SPEC': fake_root_spec}}
         } if optimize else {}
 
+    def make_stage_list(num_build_stages):
+        return {
+            'stages': (
+                ['-'.join(('stage', str(i))) for i in range(num_build_stages)]
+                + ['stage-rebuild-index'])}
+
     def make_yaml_obj(use_artifact_buildcache, optimize, use_dependencies):
         result = {}
 
@@ -287,21 +290,9 @@ def test_ci_workarounds():
 
         result.update(make_factored_jobs(optimize))
 
-        return result
-
-    def sort_yaml_obj(obj):
-        if isinstance(obj, collections_abc.Mapping):
-            result = syaml.syaml_dict()
-            for k in sorted(obj.keys(), key=str):
-                result[k] = sort_yaml_obj(obj[k])
-            return result
+        result.update(make_stage_list(3))
 
-        if (isinstance(obj, collections_abc.Sequence) and
-                not isinstance(obj, str)):
-            return syaml.syaml_list(sorted(
-                (sort_yaml_obj(x) for x in obj), key=str))
-
-        return obj
+        return result
 
     # test every combination of:
     #     use artifact buildcache: true or false
@@ -331,8 +322,8 @@ def test_ci_workarounds():
                 actual = cinw.needs_to_dependencies(actual)
 
             predicted = syaml.dump_config(
-                sort_yaml_obj(predicted), default_flow_style=True)
+                ci_opt.sort_yaml_obj(predicted), default_flow_style=True)
             actual = syaml.dump_config(
-                sort_yaml_obj(actual), default_flow_style=True)
+                ci_opt.sort_yaml_obj(actual), default_flow_style=True)
 
             assert(predicted == actual)