LTP Whitelist: Add keep_fail attribute to whitelist JSON #17986
Conversation
A non-zero keep_fail attribute in a whitelist entry will prevent matching failures from being whitelisted. Only a fail info box will be created with the message provided by the whitelist entry to give test reviewers reliable info about the failure, in particular a bugref and symptoms so that different failures don't get mixed together. This will be used for failures which need to be closely tracked and cannot be whitelisted.
There was a problem hiding this comment.
LGTM. Works as expected https://openqa.suse.de/tests/12538408#step/memcg_regression/12
| $testmod->{result} = 'softfail'; | ||
| $testmod->record_soft_failure_result(join("\n", $msg, ($entry->{message} // ()))); | ||
| if ($entry->{keep_fail}) { | ||
| $testmod->record_resultfile('Known', $entry->{message}, result => 'fail'); |
There was a problem hiding this comment.
@Martchus @mdoucha I was trying to find, why Cannot remove memory cgroup in subtest 3. bsc#1214890 in keep_fail message in https://openqa.suse.de/tests/12538408#step/memcg_regression/12 does not perform URL transformation bugref_to_href (i.e. bsc#1214890 is a plain text instead of clickable URL), which soft failures do (https://openqa.suse.de/tests/12497280#step/ustat01/12).
Soft failures use record_soft_failure_result, but that is quite similar to record_resultfile.
What am I missing?
There was a problem hiding this comment.
That's strange, indeed. I'll have a look.
The following works:
script/openqa eval 'say(app->build_controller->rendered_refs_no_shortening("Cannot remove memory cgroup in subtest 3. bsc#1214890"))'
Cannot remove memory cgroup in subtest 3. <a href="https://bugzilla.suse.com/show_bug.cgi?id=1214890">bsc#1214890</a>
So the rendering function threats both inputs equally. I guess the problem with the failure step result is that it is rendered at the time the details tab is loaded (in contrast to the softfailure which is rendered server-side via an additional AJAX request that is only done when clicking on the step).
There was a problem hiding this comment.
So the problem is this JavaScript code which doesn't handle rendering bugrefs: https://github.com/os-autoinst/openQA/blob/f46075b5f42985c47976249f44935c80fa780bca/assets/javascripts/render.js#L187
The problem with just implementing it is that we'd have to duplicate/rewrite existing server-side Perl code in JavaScript which wouldn't be ideal. I guess ideally all that frontend logic would be done entirely in JavaScript.
There was a problem hiding this comment.
Thanks for finding the root cause. Hm, long time ago web developers tried to avoid text handling in javascript. But openQA web UI is simply unusable without javascript (job detail shows nothing), thus we can ignore bug reference being broken without javascript. But somebody would have to implement it.
There was a problem hiding this comment.
It looks like this function is actually quite simple - only the regex it uses is a bit involved. So duplicating the function in JavaScript wouldn't be that hard and the regex could simply be made available to the JavaScript like other backend variables. I'll give it a try :-)
There was a problem hiding this comment.
I tried the following: https://github.com/Martchus/openQA/pull/new/render-bugrefs-in-js-as-well
But it doesn't work because the JavaScript regex support cannot cope with the Perl regex we're using in the backend.
There was a problem hiding this comment.
It might be easier to just remove the horrible data-text hack and render the bug actions template into every result box detail on the server side, including softfails.
There was a problem hiding this comment.
Thank you both. I'm not familiar with the code, but I'd also prefer Martin's suggestion.
There was a problem hiding this comment.
Mh… it is not that horrible but I was also thinking about simply rendering the HTML server-side.
There was a problem hiding this comment.
This would improve the rendering: os-autoinst/openQA#5334
Getting rid of the hack would also be a possibility but it would mean a slight waiting time when loading a step which can be annoying when going though a lot of them so I refrained from that for now.
Does not work so easily because JavaScript's regex support does not accept the advanced Perl regex we use in the backend. See os-autoinst/os-autoinst-distri-opensuse#17986 (review) for further context.
A non-zero
keep_failattribute in a whitelist entry will prevent matching failures from being whitelisted. Only a fail info box will be created with the message provided by the whitelist entry to give test reviewers reliable info about the failure, in particular a bugref and symptoms so that different failures don't get mixed together. This will be used for failures which need to be closely tracked and cannot be whitelisted.