Merge "Fix a bug that re-enqueue may skip soft dependent jobs"

tests/fixtures/layouts/not-skip-when-reenqueue.yaml

@@ -0,0 +1,47 @@
+- pipeline:
+    name: check
+    manager: independent
+    trigger:
+      gerrit:
+        - event: patchset-created
+    success:
+      gerrit:
+        Verified: 1
+    failure:
+      gerrit:
+        Verified: -1
+
+- job:
+    name: base
+    parent: null
+    run: playbooks/base.yaml
+
+- job:
+    name: grand-parent
+
+- job:
+    name: parent1
+
+- job:
+    name: parent2
+
+- job:
+    name: child
+
+- project:
+    name: org/project
+    check:
+      jobs:
+        - grand-parent
+        - parent1:
+            dependencies:
+              - grand-parent
+        - parent2:
+            dependencies:
+              - grand-parent
+        - child:
+            dependencies:
+              - name: parent1
+                soft: true
+              - name: parent2
+                soft: true

tests/unit/test_scheduler.py

@@ -6741,6 +6741,52 @@ class TestDependencyGraph(ZuulTestCase):
             dict(name='deploy', result='SUCCESS', changes='1,1'),
         ], ordered=False)
 
+    @simple_layout('layouts/not-skip-when-reenqueue.yaml')
+    def test_child_with_soft_dependency_should_not_skip(self):
+        A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A')
+        self.executor_server.hold_jobs_in_build = True
+        self.executor_server.returnData(
+            'grand-parent', A,
+            {'zuul':
+                {'child_jobs': ['parent2']}
+            }
+        )
+
+        self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
+        self.waitUntilSettled()
+
+        self.executor_server.release('grand-parent')
+        self.waitUntilSettled()
+        # grand-parent success, parent1 skipped, parent2 running
+        self.assertHistory([
+            dict(name='grand-parent', result='SUCCESS', changes='1,1'),
+        ], ordered=False)
+        self.assertBuilds([dict(name='parent2')])
+
+        # Reconfigure to trigger a re-enqueue, this should not cause job
+        # 'child' to be skipped because parent1 was skipped
+        self.scheds.execute(lambda app: app.sched.reconfigure(app.config))
+
+        self.executor_server.release('parent1')
+        self.executor_server.release('parent2')
+        self.waitUntilSettled()
+        # grand-parent success, parent1 skipped, parent2 success, child running
+        self.assertHistory([
+            dict(name='grand-parent', result='SUCCESS', changes='1,1'),
+            dict(name='parent2', result='SUCCESS', changes='1,1'),
+        ], ordered=False)
+        self.assertBuilds([dict(name='child')])
+
+        self.executor_server.release('child')
+        self.waitUntilSettled()
+        # grand-parent success, parent1 skipped, parent2 success, child success
+        self.assertHistory([
+            dict(name='grand-parent', result='SUCCESS', changes='1,1'),
+            dict(name='parent2', result='SUCCESS', changes='1,1'),
+            dict(name='child', result='SUCCESS', changes='1,1'),
+        ], ordered=False)
+        self.assertBuilds([])
+
     @simple_layout('layouts/soft-dependencies.yaml')
     def test_soft_dependencies_failure(self):
         file_dict = {'main.c': 'test'}

zuul/model.py

@@ -4859,7 +4859,7 @@ class QueueItem(zkobject.ZKObject):
                         skip, skip_soft=True)
                     skipped += to_skip
 
-        elif build.result != 'SUCCESS' and not build.paused:
+        elif build.result not in ('SUCCESS', 'SKIPPED') and not build.paused:
             to_skip = job_graph.getDependentJobsRecursively(
                 build.job.name)
             skipped += to_skip