Include some skipped jobs in the code-review report

In the recent change to omit skipped jobs when reporting, we may
have swung the pendulum too far.  While it seems that users may
not want to see a list of hundreds of skipped child_jobs, they may
want to see a list of a small number of skipped jobs due to failed
dependencies.

To try to thread the needle, we omit skipped jobs from the report
iff they were skipped due to zuul_return child_jobs; otherwise
we include them.

Change-Id: I66a223da344a93b4691a969876e887b5eec0e67c

4 files changed, 18 insertions, 10 deletions

diff --git a/tests/unit/test_scheduler.py b/tests/unit/test_scheduler.py
index 5d8099686..6bdb4136f 100644
--- a/tests/unit/test_scheduler.py
+++ b/tests/unit/test_scheduler.py
@@ -5788,7 +5788,8 @@ For CI problems and help debugging, contact ci@example.org"""
         self.assertEqual(A.reported, 2)
         self.assertTrue(re.search('project-merge .* NODE_FAILURE',
                                   A.messages[1]))
-        self.assertTrue('Skipped 2 jobs' in A.messages[1])
+        self.assertTrue(
+            'Skipped due to failed job project-merge' in A.messages[1])
 
     def test_nodepool_resources(self):
         "Test that resources are reported"
@@ -6954,7 +6955,7 @@ class TestDependencyGraph(ZuulTestCase):
         self.assertHistory([
             dict(name='build', result='FAILURE', changes='1,1'),
         ], ordered=False)
-        self.assertIn('Skipped 1 job', A.messages[0])
+        self.assertIn('Skipped due to failed job build', A.messages[0])
 
 
 class TestDuplicatePipeline(ZuulTestCase):
diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py
index 541690434..56d969581 100644
--- a/tests/unit/test_v3.py
+++ b/tests/unit/test_v3.py
@@ -6981,7 +6981,7 @@ class TestJobPause(AnsibleZuulTestCase):
             dict(name='compile', result='SUCCESS', changes='1,1'),
         ])
 
-        self.assertTrue('Skipped 1 job' in A.messages[0])
+        self.assertTrue('Skipped due to failed job pre-test' in A.messages[0])
 
     def test_job_pause_pre_skipped_child(self):
         """
@@ -7029,7 +7029,7 @@ class TestJobPause(AnsibleZuulTestCase):
             dict(name='compile', result='SUCCESS', changes='1,1'),
         ])
 
-        self.assertTrue('Skipped 1 job' in A.messages[0])
+        self.assertTrue('Skipped due to failed job pre-test' in A.messages[0])
 
     def test_job_pause_skipped_child_retry(self):
         """
@@ -7876,12 +7876,12 @@ class TestProvidesRequiresMysql(ZuulTestCase):
         self.assertEqual(
             B.messages[0].count(
                 'Job image-user requires artifact(s) images'),
-            2,
+            1,
             B.messages[0])
         self.assertEqual(
             B.messages[0].count(
                 'Job library-user requires artifact(s) libraries'),
-            2,
+            1,
             B.messages[0])
 
     @simple_layout('layouts/provides-requires-single-project.yaml')
@@ -7898,7 +7898,8 @@ class TestProvidesRequiresMysql(ZuulTestCase):
             dict(name='image-builder', result='FAILURE', changes='1,1'),
             dict(name='hold', result='SUCCESS', changes='1,1'),
         ], ordered=False)
-        self.assertTrue('Skipped 1 job' in A.messages[0])
+        self.assertTrue(
+            'Skipped due to failed job image-builder' in A.messages[0])
 
         B = self.fake_gerrit.addFakeChange('org/project1', 'master', 'B')
         B.data['commitMessage'] = '%s\n\nDepends-On: %s\n' % (
@@ -7916,7 +7917,7 @@ class TestProvidesRequiresMysql(ZuulTestCase):
         self.assertEqual(
             B.messages[0].count(
                 'Job image-user requires artifact(s) images'),
-            2, B.messages[0])
+            1, B.messages[0])
 
 
 class TestProvidesRequiresPostgres(TestProvidesRequiresMysql):
diff --git a/zuul/model.py b/zuul/model.py
index acc8813d3..e6ab0f914 100644
--- a/zuul/model.py
+++ b/zuul/model.py
@@ -4916,7 +4916,7 @@ class QueueItem(zkobject.ZKObject):
             if data:
                 job.setArtifactData(data)
         except RequirementsError as e:
-            self.warning(str(e))
+            self.log.info(str(e))
             fakebuild = Build.new(self.pipeline.manager.current_context,
                                   job=job, build_set=self.current_build_set,
                                   error_detail=str(e), result='FAILURE')
diff --git a/zuul/reporter/__init__.py b/zuul/reporter/__init__.py
index 5af48abc6..9a868eac4 100644
--- a/zuul/reporter/__init__.py
+++ b/zuul/reporter/__init__.py
@@ -273,7 +273,13 @@ class BaseReporter(object, metaclass=abc.ABCMeta):
         for job in item.getJobs():
             build = item.current_build_set.getBuild(job.name)
             (result, url) = item.formatJobResult(job)
-            if result == 'SKIPPED':
+            # If child_jobs is being used to skip jobs, then the user
+            # probably has an expectation that some jobs will be
+            # skipped and doesn't need to see all of them.  Otherwise,
+            # it may be a surprise and it may be better to include the
+            # job in the report.
+            if (build.error_detail and
+                'Skipped due to child_jobs' in build.error_detail):
                 skipped += 1
                 continue
             if not job.voting: