Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LTP Whitelist: Add keep_fail attribute to whitelist JSON #17986

Merged
merged 1 commit into from
Oct 16, 2023
Merged

LTP Whitelist: Add keep_fail attribute to whitelist JSON #17986

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions lib/LTP/WhiteList.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,10 +126,16 @@ sub override_known_failures {
}
}

my $msg = "Failure in LTP:$suite:$testname is known, overriding to softfail";
bmwqemu::diag($msg);
$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');
Copy link
Contributor

@pevik pevik Oct 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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?

Copy link
Contributor

@Martchus Martchus Oct 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :-)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you both. I'm not familiar with the code, but I'd also prefer Martin's suggestion.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mh… it is not that horrible but I was also thinking about simply rendering the HTML server-side.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}
else {
my $msg = "Failure in LTP:$suite:$testname is known, overriding to softfail";
bmwqemu::diag($msg);
$testmod->{result} = 'softfail';
$testmod->record_soft_failure_result(join("\n", $msg, ($entry->{message} // ())));
}

return 1;
}

Expand Down