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
This commit is contained in:
James E. Blair 2022-10-06 14:32:12 -07:00
parent aeb777e77a
commit 4f97f953b3
4 changed files with 18 additions and 10 deletions

View File

@ -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):

View File

@ -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):

View File

@ -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')

View File

@ -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: