From feb032d9b50fee81420e7ab0340daad360386c01 Mon Sep 17 00:00:00 2001 From: "James E. Blair" Date: Sat, 9 Jul 2022 10:58:52 -0700 Subject: Hide skipped jobs in status/reports For heavy users of "dispatch jobs" (where many jobs are declared as dependencies of a single job which then mutates the child_jobs return value to indicate which few of those should be run), there may be large numbers of "SKIPPED" jobs in the status page and in the final job report, which reduces the usability of both of those. Yet it is important for users to be able to see skipped jobs since they may represent an error (they may be inadvertently skipped). To address this, we remove "SKIPPED" jobs from the status page by default, but add a button at the bottom of the change box which can toggle their display. We remove "SKIPPED" jobs from the report, but add a note at the bottom which says "Skipped X jobs". Users can follow the buildset link to see which ones were skipped. The buildset page will continue to show all the jobs for the buildset. Change-Id: Ie297168cdf5b39d1d6f219e9b2efc44c01e87f35 --- tests/unit/test_scheduler.py | 5 ++- tests/unit/test_v3.py | 18 +++++----- web/src/containers/status/ChangePanel.jsx | 49 ++++++++++++++++++++------ web/src/containers/status/ChangePanel.test.jsx | 28 +++++++++++++++ web/src/index.css | 5 +++ zuul/driver/github/githubreporter.py | 5 ++- zuul/driver/pagure/pagurereporter.py | 5 ++- zuul/reporter/__init__.py | 11 ++++-- 8 files changed, 98 insertions(+), 28 deletions(-) diff --git a/tests/unit/test_scheduler.py b/tests/unit/test_scheduler.py index c6865a8d7..e0c8b611f 100644 --- a/tests/unit/test_scheduler.py +++ b/tests/unit/test_scheduler.py @@ -5680,8 +5680,7 @@ 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(re.search('project-test1 .* SKIPPED', A.messages[1])) - self.assertTrue(re.search('project-test2 .* SKIPPED', A.messages[1])) + self.assertTrue('Skipped 2 jobs' in A.messages[1]) def test_nodepool_resources(self): "Test that resources are reported" @@ -6803,7 +6802,7 @@ class TestDependencyGraph(ZuulTestCase): self.assertHistory([ dict(name='build', result='FAILURE', changes='1,1'), ], ordered=False) - self.assertIn('SKIPPED', A.messages[0]) + self.assertIn('Skipped 1 job', A.messages[0]) class TestDuplicatePipeline(ZuulTestCase): diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py index 9fd8ea1d3..a89bb3007 100644 --- a/tests/unit/test_v3.py +++ b/tests/unit/test_v3.py @@ -4930,7 +4930,7 @@ class TestDataReturn(AnsibleZuulTestCase): self.assertIn( '- data-return https://zuul.example.com/', A.messages[-1]) - self.assertTrue(re.search('child .* SKIPPED', A.messages[-1])) + self.assertTrue('Skipped 1 job' in A.messages[-1]) self.assertIn('Build succeeded', A.messages[-1]) def test_data_return_invalid_child_job(self): @@ -4943,7 +4943,7 @@ class TestDataReturn(AnsibleZuulTestCase): self.assertIn( '- data-return-invalid-child-job https://zuul.example.com', A.messages[-1]) - self.assertTrue(re.search('data-return .* SKIPPED', A.messages[-1])) + self.assertTrue('Skipped 1 job' in A.messages[-1]) self.assertIn('Build succeeded', A.messages[-1]) def test_data_return_skip_all_child_jobs(self): @@ -4957,8 +4957,7 @@ class TestDataReturn(AnsibleZuulTestCase): self.assertIn( '- data-return-skip-all https://zuul.example.com/', A.messages[-1]) - self.assertTrue(re.search('child .* SKIPPED', A.messages[-1])) - self.assertTrue(re.search('data-return .* SKIPPED', A.messages[-1])) + self.assertTrue('Skipped 2 jobs' in A.messages[-1]) self.assertIn('Build succeeded', A.messages[-1]) def test_data_return_skip_all_child_jobs_with_soft_dependencies(self): @@ -4972,8 +4971,7 @@ class TestDataReturn(AnsibleZuulTestCase): ]) self.assertIn('- data-return-cd https://zuul.example.com/', A.messages[-1]) - self.assertTrue(re.search('data-return-a .* SKIPPED', A.messages[-1])) - self.assertTrue(re.search('data-return-b .* SKIPPED', A.messages[-1])) + self.assertTrue('Skipped 2 jobs' in A.messages[-1]) self.assertIn('Build succeeded', A.messages[-1]) def test_several_zuul_return(self): @@ -4987,7 +4985,7 @@ class TestDataReturn(AnsibleZuulTestCase): self.assertIn( '- several-zuul-return-child https://zuul.example.com/', A.messages[-1]) - self.assertTrue(re.search('data-return .* SKIPPED', A.messages[-1])) + self.assertTrue('Skipped 1 job' in A.messages[-1]) self.assertIn('Build succeeded', A.messages[-1]) def test_data_return_skip_retry(self): @@ -6905,7 +6903,7 @@ class TestJobPause(AnsibleZuulTestCase): dict(name='compile', result='SUCCESS', changes='1,1'), ]) - self.assertTrue(re.search('test .* SKIPPED', A.messages[0])) + self.assertTrue('Skipped 1 job' in A.messages[0]) def test_job_pause_pre_skipped_child(self): """ @@ -6953,7 +6951,7 @@ class TestJobPause(AnsibleZuulTestCase): dict(name='compile', result='SUCCESS', changes='1,1'), ]) - self.assertTrue(re.search('test .* SKIPPED', A.messages[0])) + self.assertTrue('Skipped 1 job' in A.messages[0]) def test_job_pause_skipped_child_retry(self): """ @@ -7822,7 +7820,7 @@ class TestProvidesRequiresMysql(ZuulTestCase): dict(name='image-builder', result='FAILURE', changes='1,1'), dict(name='hold', result='SUCCESS', changes='1,1'), ], ordered=False) - self.assertTrue(re.search('image-user .* SKIPPED', A.messages[0])) + self.assertTrue('Skipped 1 job' in A.messages[0]) B = self.fake_gerrit.addFakeChange('org/project1', 'master', 'B') B.data['commitMessage'] = '%s\n\nDepends-On: %s\n' % ( diff --git a/web/src/containers/status/ChangePanel.jsx b/web/src/containers/status/ChangePanel.jsx index 33fc4687c..dd4fc27e5 100644 --- a/web/src/containers/status/ChangePanel.jsx +++ b/web/src/containers/status/ChangePanel.jsx @@ -18,6 +18,7 @@ import { connect } from 'react-redux' import { Link } from 'react-router-dom' import * as moment from 'moment' import 'moment-duration-format' +import { Button } from '@patternfly/react-core' class ChangePanel extends React.Component { @@ -30,9 +31,11 @@ class ChangePanel extends React.Component { constructor () { super() this.state = { - expanded: false + expanded: false, + showSkipped: false, } this.onClick = this.onClick.bind(this) + this.toggleSkippedJobs = this.toggleSkippedJobs.bind(this) this.clicked = false } @@ -120,12 +123,13 @@ class ChangePanel extends React.Component { } renderProgressBar (change) { - let jobPercent = (100 / change.jobs.length).toFixed(2) + const interesting_jobs = change.jobs.filter(j => this.jobStrResult(j) !== 'skipped') + let jobPercent = (100 / interesting_jobs.length).toFixed(2) return (
{change.jobs.map((job, idx) => { let result = this.jobStrResult(job) - if (['queued', 'waiting'].includes(result)) { + if (['queued', 'waiting', 'skipped'].includes(result)) { return '' } let className = '' @@ -144,7 +148,6 @@ class ChangePanel extends React.Component { className = ' progress-bar-warning' break case 'paused': - case 'skipped': className = ' progress-bar-info' break default: @@ -302,15 +305,39 @@ class ChangePanel extends React.Component { ) } + toggleSkippedJobs (e) { + // Skip middle mouse button + if (e.button === 1) { + return + } + this.setState({ showSkipped: !this.state.showSkipped }) + } + renderJobList (jobs, times) { + const [buttonText, interestingJobs] = this.state.showSkipped ? + ['Hide', jobs] : + ['Show', jobs.filter(j => this.jobStrResult(j) !== 'skipped')] + const skippedJobCount = jobs.length - interestingJobs.length + return ( - ) + <> + + + ) } calculateTimes (change) { diff --git a/web/src/containers/status/ChangePanel.test.jsx b/web/src/containers/status/ChangePanel.test.jsx index 5a4be8602..cd27edc73 100644 --- a/web/src/containers/status/ChangePanel.test.jsx +++ b/web/src/containers/status/ChangePanel.test.jsx @@ -16,6 +16,7 @@ import React from 'react' import { Link, BrowserRouter as Router } from 'react-router-dom' import { Provider } from 'react-redux' import { create } from 'react-test-renderer' +import { Button } from '@patternfly/react-core' import { setTenantAction } from '../../actions/tenant' import configureStore from '../../store' @@ -45,6 +46,8 @@ it('change panel render multi tenant links', () => { const jobLink = application.root.findByType(Link) expect(jobLink.props.to).toEqual( '/t/tenant-one/stream/42') + const skipButton = application.root.findAllByType(Button) + expect(skipButton === undefined) }) it('change panel render white-label tenant links', () => { @@ -60,4 +63,29 @@ it('change panel render white-label tenant links', () => { const jobLink = application.root.findByType(Link) expect(jobLink.props.to).toEqual( '/stream/42') + const skipButton = application.root.findAllByType(Button) + expect(skipButton === undefined) +}) + +it('change panel skip jobs', () => { + const fakeChange = { + project: 'org-project', + jobs: [{ + name: 'job-name', + url: 'stream/42', + result: 'skipped' + }] + } + + const store = configureStore() + store.dispatch(setTenantAction('tenant-one', true)) + const application = create( + + + + + + ) + const skipButton = application.root.findByType(Button) + expect(skipButton.props.children.includes('skipped job')) }) diff --git a/web/src/index.css b/web/src/index.css index e8c67a372..eddbca673 100644 --- a/web/src/index.css +++ b/web/src/index.css @@ -189,6 +189,11 @@ a.refresh { font-size: small; } +.zuul-skipped-jobs-button { + font-size: small; + padding: 0; +} + .zuul-non-voting-desc { font-size: smaller; } diff --git a/zuul/driver/github/githubreporter.py b/zuul/driver/github/githubreporter.py index de62f2565..73d3a6f13 100644 --- a/zuul/driver/github/githubreporter.py +++ b/zuul/driver/github/githubreporter.py @@ -135,9 +135,12 @@ class GithubReporter(BaseReporter): def _formatItemReportJobs(self, item): # Return the list of jobs portion of the report ret = '' - jobs_fields = self._getItemReportJobsFields(item) + jobs_fields, skipped = self._getItemReportJobsFields(item) for job_fields in jobs_fields: ret += self._formatJobResult(job_fields) + if skipped: + jobtext = 'job' if skipped == 1 else 'jobs' + ret += 'Skipped %i %s\n' % (skipped, jobtext) return ret def addPullComment(self, item, comment=None): diff --git a/zuul/driver/pagure/pagurereporter.py b/zuul/driver/pagure/pagurereporter.py index 0bfdbc9b8..b38035752 100644 --- a/zuul/driver/pagure/pagurereporter.py +++ b/zuul/driver/pagure/pagurereporter.py @@ -67,9 +67,12 @@ class PagureReporter(BaseReporter): def _formatItemReportJobs(self, item): # Return the list of jobs portion of the report ret = '' - jobs_fields = self._getItemReportJobsFields(item) + jobs_fields, skipped = self._getItemReportJobsFields(item) for job_fields in jobs_fields: ret += '- [%s](%s) : %s%s%s%s\n' % job_fields[:6] + if skipped: + jobtext = 'job' if skipped == 1 else 'jobs' + ret += 'Skipped %i %s\n' % (skipped, jobtext) return ret def addPullComment(self, item, comment=None): diff --git a/zuul/reporter/__init__.py b/zuul/reporter/__init__.py index 9b8f2c11c..5723316a3 100644 --- a/zuul/reporter/__init__.py +++ b/zuul/reporter/__init__.py @@ -257,9 +257,13 @@ class BaseReporter(object, metaclass=abc.ABCMeta): # Extract the report elements from an item config = self.connection.sched.config jobs_fields = [] + skipped = 0 for job in item.getJobs(): build = item.current_build_set.getBuild(job.name) (result, url) = item.formatJobResult(job) + if result == 'SKIPPED': + skipped += 1 + continue if not job.voting: voting = ' (non-voting)' else: @@ -300,12 +304,15 @@ class BaseReporter(object, metaclass=abc.ABCMeta): success_message = job.success_message jobs_fields.append( (name, url, result, error, elapsed, voting, success_message)) - return jobs_fields + return jobs_fields, skipped def _formatItemReportJobs(self, item): # Return the list of jobs portion of the report ret = '' - jobs_fields = self._getItemReportJobsFields(item) + jobs_fields, skipped = self._getItemReportJobsFields(item) for job_fields in jobs_fields: ret += '- %s%s : %s%s%s%s\n' % job_fields[:6] + if skipped: + jobtext = 'job' if skipped == 1 else 'jobs' + ret += 'Skipped %i %s\n' % (skipped, jobtext) return ret -- cgit v1.2.1