zuul/zuul
Code Issues Proposed changes

Hide skipped jobs in status/reports

Browse Source 
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
This commit is contained in:
James E. Blair
2022-07-21 18:16:42 -07:00
parent cce45ec1dd
commit feb032d9b5
8 changed files with 98 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/unit/test_scheduler.py
+2 -3
View File
@@ -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):
tests/unit/test_v3.py
+8 -10
View File
@@ -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' % (
web/src/containers/status/ChangePanel.jsx
+38 -11
View File
@@ -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 (
<div className='progress zuul-change-total-result'>
{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 {
</span>)
}
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 (
<ul className='list-group zuul-patchset-body'>
{jobs.map((job, idx) => (
<li key={idx} className='list-group-item zuul-change-job'>
{this.renderJob(job, times.jobs[job.name])}
</li>
))}
</ul>)
<>
<ul className='list-group zuul-patchset-body'>
{interestingJobs.map((job, idx) => (
<li key={idx} className='list-group-item zuul-change-job'>
{this.renderJob(job, times.jobs[job.name])}
</li>
))}
{(this.state.showSkipped || skippedJobCount) ? (
<li key='last' className='list-group-item zuul-change-job'>
<Button variant="link" className='zuul-skipped-jobs-button'
onClick={this.toggleSkippedJobs}>
{buttonText} {skippedJobCount ? skippedJobCount : ''} skipped job{skippedJobCount === 1 ? '' : 's'}
</Button>
</li>
) : ''}
</ul>
</>
)
}
calculateTimes (change) {
web/src/containers/status/ChangePanel.test.jsx
+28
View File
@@ -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(
<Provider store={store}>
<Router>
<ChangePanel change={fakeChange} globalExpanded={true} />
</Router>
</Provider>
)
const skipButton = application.root.findByType(Button)
expect(skipButton.props.children.includes('skipped job'))
})
web/src/index.css
+5
View File
@@ -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;
}
zuul/driver/github/githubreporter.py
+4 -1
View File
@@ -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):
zuul/driver/pagure/pagurereporter.py
+4 -1
View File
@@ -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):
zuul/reporter/__init__.py
+9 -2
View File
@@ -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