QA » openQA Project

The web UI just renders whatever os-autoinst stores as the result (even if it is a hashref which is not so nice of course).

But value expected by the web UI is definitely softfailed. Not sure why only softfail or even a hash is written by os-autoinst.

Seems like when we get a hashref, it contains a nested result:

   {
      "title" : "Soft Failed",
      "text" : "grub_set_bootargs-7.txt",
      "result" : {
         "frametime" : [
            "82.62",
            "82.67"
         ],
         "screenshot" : "grub_set_bootargs-6.png",
         "result" : "unk"
      },
      "needles" : []
   },

(Excerpt from cat /var/lib/openqa/testresults/02084/02084411-sle-15-SP1-Installer-DVD-s390x-Build47.1-skip_registration@s390x-zVM-ctc/details-grub_set_bootargs.json | json_pp of job https://openqa.suse.de/tests/2084411#step/grub_set_bootargs/5.)

I assume that's a bug in os-autoinst.

Sorry if my bug report confused you because, yes, it is also obvious to me that this is an os-autoinst problem and that openQA makes the right assumptions. Nevertheless, this here is also the ticket reporting system for os-autoinst ;)

Not talking about CSS would have helped :)

I think the 'even if it is a hashref' point could still be improved on the openQA-side.

I'm also wondering what has changed in os-autoinst. Eg. git log -G'softfail' returns a bunch of commits but git log -G'softfailed' nothing.

Didn't we consolidate the names in openQA some time and actually rename from softfail to softfailed? Maybe it's not a regression but never worked differently

format_result helper is doing this mapping

format_result is not doing any mapping from the terms currently returned by os-autoinst to openQA terms. It already assumes softfailed like any other place in openQA (or I missed something when grepping). And besides, this helper is only used for the module result but not for the step result (which this issue is about).

That's what I've found out about the current behavior:

1. The test API function record_soft_failure always adds the last screenshot info as result where the result should actually just a string like 'ok' or 'fail'. (https://github.com/os-autoinst/os-autoinst/blob/master/basetest.pm#L459)
2. os-autoinst does not produce any details with the result 'softfail' or 'softfail*ed*'.
3. If an image is shown as softfailure in the web UI, that is because the web UI itself checks for the workaround flag being present and then sets the result to 'softfail*ed*' on its own. (https://github.com/os-autoinst/openQA/blob/master/lib/OpenQA/WebAPI/Plugin/Helpers.pm#L373)

How I would change it:

1. record_soft_failure doesn't attempt to add any kind of screenshot info. This test API function is only for recording text. Softfailure screenshots happen via 3.
2. os-autoinst sets the result of the details produced via record_soft_failure to 'softfail' which is consistent with 'fail' and with the last PR handled by the web UI.
3. no change

How I would change it:

1. record_soft_failure doesn't attempt to add any kind of screenshot info. This test API function is only for recording text. Softfailure screenshots happen via 3.
2. os-autoinst sets the result of the details produced via record_soft_failure to 'softfail' which is consistent with 'fail' and with the last PR handled by the web UI.
3. no change

Hm, maybe related: What I would like to be able is to override a "failed" status from a job with a call to record_soft_failure. The use case is the following: More and more "softfail" means for me "known issue" not how many other people understand it as "less severe than fail" because we should not or even can not decide about a severity within openQA fails. Reviewing test failures is annoying to many especially when mainly it is about "known issues" which we need to label. So it would be good if we as reviewers can save time and put an explicit record_soft_failure call in the post_fail_hook of jobs so that for known issues the job turns out as "softfail" instead of "fail" with a reference to the issue as shown by record_soft_failure. See #39719

PR for fixing the os-autoinst side as explained in my comment above: https://github.com/os-autoinst/os-autoinst/pull/1042

@okurz Ok. But I wouldn't change multiple things at a time and track this idea separately.