action #123724
closedauto_review not working despite ticket in openQA auto review project size:M
Added by tjyrinki_suse almost 2 years ago. Updated over 1 year ago.
0%
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
Updated by okurz almost 2 years ago
- Tags set to reactive work
- Project changed from openQA Project (public) to 115
- Priority changed from Normal to High
- Target version set to Ready
Updated by livdywan almost 2 years 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
Updated by mkittler almost 2 years 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?
Updated by mkittler almost 2 years 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")
Updated by mkittler almost 2 years 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.
Updated by mkittler almost 2 years 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.
Updated by mkittler almost 2 years 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.
Updated by mkittler almost 2 years 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).
Updated by okurz almost 2 years 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
Updated by okurz almost 2 years 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.
- 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.
- Declare combination of bugrefs+labels as undefined behaviour -> Ensure that our documentation and webUI help states that
- 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
- 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
- We assume that we call hook scripts only if no carry over has happened
- 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.
- If a label is carried-over then evaluate it, e.g. force result accordingly
- Extend automatic take-over message to clarify that no hook scripts are called
- 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
Updated by okurz almost 2 years ago
- Project changed from 115 to openQA Project (public)
- Category set to Regressions/Crashes
Updated by mkittler almost 2 years ago
- Status changed from Blocked to In Progress
Updated by mkittler almost 2 years 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).
Updated by okurz almost 2 years ago
- Due date set to 2023-03-24
Updated by mkittler over 1 year ago
PR for 4 (and 4.1): https://github.com/os-autoinst/openQA/pull/5035
Updated by mkittler over 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.
Updated by mkittler over 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.
Updated by okurz about 1 year 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