From 0738d31b08605c406da90009cb3e4792f2bdf029 Mon Sep 17 00:00:00 2001 From: "James E. Blair" Date: Wed, 5 Oct 2022 15:49:58 -0700 Subject: [PATCH] Include skipped builds in database and web ui We have had an on-and-off relationship with skipped builds in the database. Generally we have attempted to exclude them from the db, but we have occasionally (accidentally?) included them. The status quo is that builds with a result of SKIPPED (as well as several other results which don't come from the executor) are not recorded in the database. With a greater interest in being able to determine which jobs ran or did not run for a change after the fact, this job deliberately adds all builds (whether they touch an executor or not, whether real or not) to the database. This means than anything that could potentially show up on the status page or in a code-review report will be in the database, and can therefore be seen in the web UI. It is still the case that we are not actually interested in seeing a page full of SKIPPED builds when we visit the "Builds" tab in the web ui (which is the principal reason we have not included them in the database so far). To address this, we set the default query in the builds tab to exclude skipped builds (it is easy to add other types of builds to exclude in the future if we wish). If a user then specifies a query filter to *include* specific results, we drop the exclusion from the query string. This allows for the expected behavior of not showing SKIPPED by default, then as specific results are added to the filter, we show only those, and if the user selects that they want to see SKIPPED, they will then be included. On the buildset page, we add a switch similar to the current "show retried jobs" switch that selects whether skipped builds in a buildset should be displayed (again, it hides them by default). Change-Id: I1835965101299bc7a95c952e99f6b0b095def085 --- tests/unit/test_web.py | 39 ++++++++++++++++++++++++++ web/src/containers/FilterToolbar.jsx | 7 +++++ web/src/containers/build/BuildList.jsx | 32 +++++++++++++++++++-- zuul/driver/sql/sqlconnection.py | 12 +++++++- zuul/manager/__init__.py | 8 +----- zuul/model.py | 21 +++++++++++++- zuul/web/__init__.py | 6 ++-- 7 files changed, 112 insertions(+), 13 deletions(-) diff --git a/tests/unit/test_web.py b/tests/unit/test_web.py index d8901fabb8..dab60efc68 100644 --- a/tests/unit/test_web.py +++ b/tests/unit/test_web.py @@ -1783,6 +1783,45 @@ class TestBuildInfo(BaseTestWeb): "idx_min=%i" % idx_max).json() self.assertEqual(len(builds_query), 1, builds_query) + def test_web_list_skipped_builds(self): + # Test the exclude_result filter + # Generate some build records in the db. + A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A') + self.executor_server.failJob('project-merge', A) + A.addApproval('Code-Review', 2) + self.fake_gerrit.addEvent(A.addApproval('Approved', 1)) + self.waitUntilSettled() + self.executor_server.hold_jobs_in_build = False + self.executor_server.release() + self.waitUntilSettled() + + builds = self.get_url("api/tenant/tenant-one/builds").json() + builds.sort(key=lambda x: x['job_name']) + self.assertEqual(len(builds), 3) + self.assertEqual(builds[0]['job_name'], 'project-merge') + self.assertEqual(builds[1]['job_name'], 'project-test1') + self.assertEqual(builds[2]['job_name'], 'project-test2') + self.assertEqual(builds[0]['result'], 'FAILURE') + self.assertEqual(builds[1]['result'], 'SKIPPED') + self.assertEqual(builds[2]['result'], 'SKIPPED') + + builds = self.get_url("api/tenant/tenant-one/builds?" + "exclude_result=SKIPPED").json() + self.assertEqual(len(builds), 1) + self.assertEqual(builds[0]['job_name'], 'project-merge') + self.assertEqual(builds[0]['result'], 'FAILURE') + + builds = self.get_url("api/tenant/tenant-one/builds?" + "result=SKIPPED&result=FAILURE").json() + builds.sort(key=lambda x: x['job_name']) + self.assertEqual(len(builds), 3) + self.assertEqual(builds[0]['job_name'], 'project-merge') + self.assertEqual(builds[1]['job_name'], 'project-test1') + self.assertEqual(builds[2]['job_name'], 'project-test2') + self.assertEqual(builds[0]['result'], 'FAILURE') + self.assertEqual(builds[1]['result'], 'SKIPPED') + self.assertEqual(builds[2]['result'], 'SKIPPED') + def test_web_badge(self): # Generate some build records in the db. self.add_base_changes() diff --git a/web/src/containers/FilterToolbar.jsx b/web/src/containers/FilterToolbar.jsx index 9568997cc5..4cc1e3f25e 100644 --- a/web/src/containers/FilterToolbar.jsx +++ b/web/src/containers/FilterToolbar.jsx @@ -319,14 +319,21 @@ function writeFiltersToUrl(filters, location, history) { function buildQueryString(filters) { let queryString = '&complete=true' + let resultFilter = false if (filters) { Object.keys(filters).map((key) => { filters[key].forEach((value) => { + if (value === 'result') { + resultFilter = true + } queryString += '&' + key + '=' + value }) return queryString }) } + if (!resultFilter) { + queryString += '&exclude_result=SKIPPED' + } return queryString } diff --git a/web/src/containers/build/BuildList.jsx b/web/src/containers/build/BuildList.jsx index 69f1795bf5..ccb7346794 100644 --- a/web/src/containers/build/BuildList.jsx +++ b/web/src/containers/build/BuildList.jsx @@ -57,15 +57,24 @@ class BuildList extends React.Component { return self.indexOf(build) === idx }) + let skippedJobs = builds.filter((build) => { + return build.result === 'SKIPPED' + }).map((build) => (build.job_name) + ).filter((build, idx, self) => { + return self.indexOf(build) === idx + }) + this.state = { visibleNonFinalBuilds: retriedJobs, retriedJobs: retriedJobs, + skippedJobs: skippedJobs, + showSkipped: false, } } sortedBuilds = () => { const { builds } = this.props - const { visibleNonFinalBuilds } = this.state + const { visibleNonFinalBuilds, showSkipped } = this.state return builds.sort((a, b) => { // Group builds by job name, then order by decreasing start time; this will ensure retries are together @@ -85,6 +94,9 @@ class BuildList extends React.Component { } }).filter((build) => { if (build.final || visibleNonFinalBuilds.indexOf(build.job_name) >= 0) { + if (build.result === 'SKIPPED' && !showSkipped) { + return false + } return true } else { @@ -98,6 +110,10 @@ class BuildList extends React.Component { this.setState({ visibleNonFinalBuilds: (isChecked ? retriedJobs : []) }) } + handleSkippedSwitch = isChecked => { + this.setState({ showSkipped: isChecked }) + } + handleToggleVisibleNonFinalBuilds = (jobName) => { const { visibleNonFinalBuilds } = this.state const index = visibleNonFinalBuilds.indexOf(jobName) @@ -138,7 +154,7 @@ class BuildList extends React.Component { render() { const { tenant } = this.props - const { visibleNonFinalBuilds, retriedJobs } = this.state + const { visibleNonFinalBuilds, retriedJobs, skippedJobs, showSkipped } = this.state let retrySwitch = retriedJobs.length > 0 ? @@ -151,10 +167,22 @@ class BuildList extends React.Component { : <> + let skippedSwitch = skippedJobs.length > 0 ? + + Show skipped jobs   + + : + <> + const sortedBuilds = this.sortedBuilds() return ( + {skippedSwitch} {retrySwitch}