From 4a2063a51e5d0abd236bceec4d6ee88a8ed728a9 Mon Sep 17 00:00:00 2001 From: Adam Williamson Date: Tue, 5 Sep 2023 17:18:35 -0700 Subject: [PATCH] Fix display of waived failures: consider subject and scenario (#5397) On the Automated Tests tab, the web UI attempts to indicate when a failed test has been waived. However, it can get this wrong in several ways. It stores the waivers only by testcase - not by subject or scenario - so if greenwave returns any waiver for the testcase, it will mark any failure of that testcase for the update as 'waived'. This can be incorrect if: * The waiver was filed against the wrong subject and is invalid * The waiver is for a different subject (different Koji build) * The waiver is for a different scenario This should fix all those problems. Instead of just parsing the waivers, we primarily parse the satisfied requirements, and when one is of type "test-result-failed-waived" - meaning it's a failure that was waived - we store it in a nested hash of waived failures. The outer hash's keys are subject identifiers, and its values are hashes. In these inner hashes, the keys are the testcase and scenario combined, and the values are the IDs of waivers for that subject/testcase/scenario combination. We keep a separate hash of waivers, the keys being the waiver IDs and the values being the waivers. So when we are constructing the result rows, we can check whether there is a waived failure for the result's testcase, scenario and subject, and only if so, we add the 'waived' marker, constructing the necessary info from the waiver retrieved from the waivers hash. Signed-off-by: Adam Williamson (cherry picked from commit 8ae22c4a2b888be04f7eccc7fb8ec39412f55682) --- .../bodhi/server/templates/update.html | 82 ++++++++++--------- 1 file changed, 42 insertions(+), 40 deletions(-) diff --git a/bodhi-server/bodhi/server/templates/update.html b/bodhi-server/bodhi/server/templates/update.html index 2e963ed87e..beb4e471ad 100644 --- a/bodhi-server/bodhi/server/templates/update.html +++ b/bodhi-server/bodhi/server/templates/update.html @@ -782,8 +782,10 @@ // These are the required taskotron tests var requirements = ${update.requirements_json | h}; - // These are the known waivers of failed testcases. + // These are the known waivers of failed testcases (key - waiver id, value - waiver) var waivers = {}; + // These are waived test failures (key - subject, value - hash: key - testcase##scenario, value - waiver id) + var waived = {}; // show the Greenwave decision var greenwave_api_url = '${request.registry.settings["greenwave_api_url"]}'; var missing_tests = {}; @@ -798,7 +800,7 @@ var greenwave_errors = 0; // handle Greenwave decision - var handle_unsatisfied_requirements = function(data){ + var handle_unsatisfied_requirements = function(data) { $.each(data['unsatisfied_requirements'], function(i, requirement) { unsatisfied_reqs_counter++; if (requirement.type == 'test-result-missing') { @@ -876,26 +878,22 @@ ' '; html += '' + summary; html += ''; - if (Object.keys(waivers).length != 0) { + if (Object.keys(waived).length != 0) { var seensubjects = []; // Create urls to waiverdb with waivers for this update - $.each(Object.keys(waivers), function(i, testcase) { - $.each(waivers[testcase], function(j, waiver) { - if (!seensubjects.includes(waiver.subject_identifier)) { - seensubjects.push(waiver.subject_identifier); - var url = '${request.registry.settings["waiverdb_api_url"]}'; - url += '/waivers' - url += '/?product_version=' + waiver.product_version; - url += '&subject_type=' + waiver.subject_type; - url += '&subject_identifier=' + waiver.subject_identifier; - html += ''; - html += '' + - ''; - html += ''; - } - }); + $.each(Object.keys(waived), function(i, subject) { + if (!seensubjects.includes(subject)) { + seensubjects.push(subject); + var url = '${request.registry.settings["waiverdb_api_url"]}'; + url += '/waivers' + url += '?subject_identifier=' + subject; + html += ''; + html += '' + + ''; + html += ''; + } }); } $('#test_status_badge').html(html); @@ -915,8 +913,7 @@ data: JSON.stringify(request), success: function(data) { $.each(data.waivers, function(i, waiver) { - if (!(waiver.testcase in waivers)) { waivers[waiver.testcase] = []; } - waivers[waiver.testcase].push(waiver); + if (!(waiver.id in waivers)) { waivers[waiver.id] = waiver; } }); handle_unsatisfied_requirements(data); $.each(data['satisfied_requirements'], function(i, requirement) { @@ -924,6 +921,14 @@ if ($.inArray(requirement.testcase, requirements) == -1) { requirements.push(requirement.testcase); } + // stash data about waived failures + if (requirement.type == "test-result-failed-waived") { + if (!(requirement.subject_identifier in waived)) { + waived[requirement.subject_identifier] = {}; + } + var key = [requirement.testcase, requirement.scenario].join('##'); + waived[requirement.subject_identifier][key] = requirement.waiver_id; + } }); test_reqs_counter += data['unsatisfied_requirements'].length; test_reqs_counter += data['satisfied_requirements'].length; @@ -937,21 +942,24 @@ }); }; - var make_row = function(outcome, testcase, note, arch, time, url, flavor) { + var make_row = function(subject, scenario, outcome, testcase, note, arch, time, url) { var icon = ''+ ''; + var flavor; + if (scenario.includes("fedora.updates-")) { + // this gets a correct flavor from an openQA scenario, e.g. + // 'kde' from 'fedora.updates-kde.x86_64.64bit' + flavor = scenario.split(".")[1].slice(8); + } + var required = ''; - if (testcase in waivers) { - var reason = 'Waived by '; - var comments = []; - for (let w of waivers[testcase]) { - waiver_details = "@" + w.username + ": '" + w.comment.replace("'", "'") + "'" - comments.push(waiver_details); - } - reason += comments.join(", ") + var key = [testcase, scenario].join('##'); + if (subject in waived && key in waived[subject]) { + var w = waivers[waived[subject][key]]; + var reason = 'Waived by @' + w.username + ": '" + w.comment.replace("'", "'") + "'"; required = '' + @@ -1090,22 +1098,16 @@ scenario = result.data.scenario[0]; } - var flavor; - if (scenario.includes("fedora.updates-")) { - // this gets a correct flavor from an openQA scenario, e.g. - // 'kde' from 'fedora.updates-kde.x86_64.64bit' - flavor = scenario.split(".")[1].slice(8); - } - table.append(make_row( + result.data.item[0], + scenario, result.outcome, result.testcase.name, result.note, // note: this is a single-item array result.data.arch, result.submit_time, - result.ref_url, - flavor + result.ref_url )); }); });