Project

General

Profile

Actions

action #123724

closed

auto_review not working despite ticket in openQA auto review project size:M

Added by tjyrinki_suse over 1 year ago. Updated about 1 year ago.

Status:
Resolved
Priority:
Normal
Assignee:
Category:
Regressions/Crashes
Target version:
Start date:
Due date:
2023-03-24
% Done:

0%

Estimated time:

Description

Observation

Ticket #122215 seems not functional still, the affected tests do not get the softfail forcing still automatically but need to be set manually daily.

The ticket has:
auto_review:"Test result is NOT same as baseline":force_result:softfailed
... at the end of the subject, and the ticket was moved to openQA auto review project.

The tests can be found at the 15-SP4 runs at https://openqa.suse.de/group_overview/429

Acceptance criteria

  • AC1: Result is changed to softfailed as specified in the title of the ticket

Suggestions

  • Verify that the regex valid/working e.g. auto_review:"Test result is NOT same as baseline":force_result:softfailed, maybe by putting the title in a unit test and seeing if that fails
  • Check that the ticket is filed correctly, i.e. using the force result tracker - this seems to be correct
  • Check the code in https://github.com/os-autoinst/scripts

Related issues 2 (1 open1 closed)

Related to openQA Project - action #124029: Bugrefs may be picked up even in clarifying comments and there's no clear way to tell if they will beResolvedmkittler

Actions
Related to openQA Project - action #135782: auto-review+force-result ticket does not seem to have an effect when issue tracker changed after the initial comment when carry-over is effectiveNew2023-09-14

Actions
Actions #2

Updated by okurz over 1 year ago

  • Tags set to reactive work
  • Project changed from openQA Project to QA
  • Priority changed from Normal to High
  • Target version set to Ready
Actions #3

Updated by livdywan over 1 year ago

  • Subject changed from auto_review not working despite ticket in openQA auto review project to auto_review not working despite ticket in openQA auto review project size:M
  • Description updated (diff)
  • Status changed from New to Workable
Actions #4

Updated by mkittler over 1 year ago

  • Assignee set to mkittler
Actions #5

Updated by mkittler over 1 year ago

When we've estimated the ticket I was under the impression there's be a link to relevant tests. However, the link is only about a job group. Do you really expect us to search for the affected tests like this? Having a concrete example job plus the log excerpt that is supposed to match the regex is important. How would it otherwise reproduce/test this?

Actions #6

Updated by mkittler over 1 year ago

I did some digging. So https://openqa.suse.de/tests/10461612 is an example job and the log excerpt that's supposed to match is:

[2023-02-10T03:55:28.693183+01:00] [debug] [pid:12522] <<< testapi::record_info(title="screen_manage", output="Test result is the same as baseline\n[9] screen_manage PASS", result="ok")
[2023-02-10T03:55:28.693602+01:00] [debug] [pid:12522] tests/security/cc/trustedprograms.pm:51 called audit_test::compare_run_log -> lib/audit_test.pm:150 called audit_test::_parse_results_with_diff_baseline -> lib/audit_test.pm:188 called testapi::record_info
[2023-02-10T03:55:28.693796+01:00] [debug] [pid:12522] <<< testapi::record_info(title="useradd", output="Test result is NOT same as baseline \nCurrent:  [10] useradd FAIL\nBaseline: [10] useradd PASS", result="fail")
[2023-02-10T03:55:28.694071+01:00] [debug] [pid:12522] tests/security/cc/trustedprograms.pm:51 called audit_test::compare_run_log -> lib/audit_test.pm:153 called testapi::record_info
[2023-02-10T03:55:28.694239+01:00] [debug] [pid:12522] <<< testapi::record_info(title="useradd_D", output="Test result is the same as baseline\n[11] useradd_D PASS", result="ok")
Actions #7

Updated by mkittler over 1 year ago

I've just ran echo 'http://openqa.suse.de/tests/10461612' | scheme=http host=10.160.0.207 ./openqa-label-known-issues and it created a comment as expected. (I've been using the IP because otherwise it times out, not sure why but that's likely a local issue.)

With echo 'http://openqa.suse.de/tests/10461612' | scheme=http host=10.160.0.207 enable_force_result=true ./openqa-label-known-issues it also created a force_result label so now the job is softfailed (before it was failed).

So the openqa-label-known-issues script is fine. It has likely just not been triggered for this job.

Actions #8

Updated by mkittler over 1 year ago

I looked at the code of openQA and the hook script and the hook script is really just triggered like in my previous test. However, there's one exception. When a bugref could be carried over, the hook script is not triggered at all. That means auto-review and its result override are not effective.

I've checked several jobs in the scenario https://openqa.suse.de/tests/latest?arch=x86_64&distri=sle&flavor=Server-DVD-Updates&machine=64bit&test=cc_audit-test-part2&version=15-SP4. I only found jobs where a bugref could be taken over (e.g. https://openqa.suse.de/tests/10462392 - EDIT: I removed that comment now, see my next comment.) or that were older than #122215 being in the right category (e.g. https://openqa.suse.de/tests/10214182). Maybe I missed some jobs, though.

Actions #9

Updated by mkittler over 1 year ago

I removed the comment https://openqa.suse.de/tests/10462392 referencing poo # 122215 which has been taken over from https://openqa.suse.de/tests/10214182 so the comment with the label will be taken over the next time. This will hopefully cause the next job to be softfailed as well.

Actions #10

Updated by mkittler over 1 year ago

  • Status changed from Workable to Feedback

The approach mentioned in my previous comment won't work. I had a look at the code and the carry over will not evaluate the force_result label (as this is implemented on API controller level and not when the database operation is done in general).

I could adjust openQA to handle such a label also when carrying over comments automatically. Is that wanted? I'm asking because this feature is not really what AC1 is asking for (as it has nothing to do anymore with auto review and ticket titles).

Actions #11

Updated by okurz over 1 year ago

  • Related to action #124029: Bugrefs may be picked up even in clarifying comments and there's no clear way to tell if they will be added
Actions #12

Updated by okurz over 1 year ago

  • Status changed from Feedback to Blocked
  • Priority changed from High to Normal

We (jbaier, tinita, mkittler, okurz) talked about the issue. openQA is designed that a bugref beats label. And also as far as we remember carry-over evaluation happens before invocation of hook scripts.

  1. openqa-label-known-issues when called with "force-result" seems to put actually two lines of comments, one line with a label and a second with a bugref. This turns the complete comment into a bugref but apparently the label is still evaluated as proven by https://openqa.suse.de/tests/10461612#comment-743022 by mkittler.
    1. Declare combination of bugrefs+labels as undefined behaviour -> Ensure that our documentation and webUI help states that
    2. Remove the bugref line in openqa-label-known-issues for the case of force_result -> that would be a revert of https://github.com/os-autoinst/scripts/pull/155 for #109857 and we need to think carefully
  2. https://openqa.suse.de/tests/10461612#comments shows all, a plain comment, a bugref and a comment which has a bugref and label and the
  3. We assume that we call hook scripts only if no carry over has happened
    1. If yes then we could consider extending openQA to always invoke the hook scripts regardless if a carry-over has happened. And then we parse openQA comments in openqa-label-known-issues to see if a carry-over has happened. As alternative forward the presence of relevant bugrefs to openqa-label-known-issues. All sounds like a badly performing approach.
    2. If a label is carried-over then evaluate it, e.g. force result accordingly
  4. Extend automatic take-over message to clarify that no hook scripts are called
    1. We could check for presence of hook scripts to only include the message when there are hook scripts

We decided that we should look into #124029 first

Actions #13

Updated by okurz over 1 year ago

  • Project changed from QA to openQA Project
  • Category set to Regressions/Crashes
Actions #14

Updated by mkittler about 1 year ago

  • Status changed from Blocked to In Progress
Actions #15

Updated by mkittler about 1 year ago

  • Status changed from In Progress to Feedback
  • 1.1: done
  • 1.2: https://github.com/os-autoinst/scripts/pull/220
  • 2: Now bugrefs within labels are rendered as such because #124029 (which was blocking this ticket) has been resolved. So we can merge the PR for 1.2.
  • 3: That assumption is correct. I suppose 3.2 wouldn't be too bad to look into. We'd just needed to move the label-evaluation from API-controller code to DB-schema code (so it is generally invoked when comments are added/updated).
  • 4: That would be a very easy change (including 4.1).
Actions #16

Updated by okurz about 1 year ago

  • Due date set to 2023-03-24
Actions #18

Updated by mkittler about 1 year ago

The PR has been merged.

Here's another one for 3.2: https://github.com/os-autoinst/openQA/pull/5039

Since 3.1 and 3.2 are alternative, this means all points from #123724#note-12 will be handled if that PR is merged.

Actions #19

Updated by mkittler about 1 year ago

  • Status changed from Feedback to Resolved

All PR have been merged so I'm considering this resolved as mentioned in the previous comment.

Actions #20

Updated by okurz 8 months ago

  • Related to action #135782: auto-review+force-result ticket does not seem to have an effect when issue tracker changed after the initial comment when carry-over is effective added
Actions

Also available in: Atom PDF