Skip to content

Commit

Permalink
Fix display of waived failures: consider subject and scenario (#5397)
Browse files Browse the repository at this point in the history 
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 <[email protected]>
(cherry picked from commit 8ae22c4)
  • Loading branch information
@AdamWill @mattiaverga
AdamWill authored and mattiaverga committed Oct 3, 2023
1 parent a7272bb commit befee26
Showing 1 changed file with 42 additions and 40 deletions.
82 changes: 42 additions & 40 deletions bodhi-server/bodhi/server/templates/update.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -782,8 +782,10 @@ <h3 class="modal-title" id="waiveModalLabel">Trigger Tests</h3>

// 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 = {};
Expand All @@ -798,7 +800,7 @@ <h3 class="modal-title" id="waiveModalLabel">Trigger Tests</h3>
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') {
Expand Down Expand Up @@ -876,26 +878,22 @@ <h3 class="modal-title" id="waiveModalLabel">Trigger Tests</h3>
'</span></span> ';
html += '<a class="gating-summary notblue" href="#">' + summary;
html += '</a>';
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 += '<a class="notblue" target="_blank" href=' + url + '>';
html += '<span data-toggle="tooltip" data-placement="top" ' +
'title="Some test failures or missing tests were waived." ' +
'class="fa fa-asterisk">' +
'</span>';
html += '</a>';
}
});
$.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 += '<a class="notblue" target="_blank" href=' + url + '>';
html += '<span data-toggle="tooltip" data-placement="top" ' +
'title="Some test failures or missing tests were waived." ' +
'class="fa fa-asterisk">' +
'</span>';
html += '</a>';
}
});
}
$('#test_status_badge').html(html);
Expand All @@ -915,15 +913,22 @@ <h3 class="modal-title" id="waiveModalLabel">Trigger Tests</h3>
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) {
// the user may have already specified this in the required taskotron tests
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;
Expand All @@ -937,21 +942,24 @@ <h3 class="modal-title" id="waiveModalLabel">Trigger Tests</h3>
});
};

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 = '<span data-bs-toggle="tooltip" data-placement="top" ' +
'title="' + outcome + '" ' +
'class="fa fa-' + icons[outcome] + ' text-' + classes[outcome] + '">'+
'</span>';

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("'", "&#39;") + "'"
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("'", "&#39;") + "'";
required = '<span style="white-space:pre-line;" data-bs-toggle="tooltip" data-placement="top" ' +
'title="' + reason + '" ' +
'class="fa fa-thumbs-up">' +
Expand Down Expand Up @@ -1090,22 +1098,16 @@ <h3 class="modal-title" id="waiveModalLabel">Trigger Tests</h3>
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
));
});
});
Expand Down

0 comments on commit befee26

Please sign in to comment.