auto_review not working despite ticket in openQA auto review project size:M
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
- AC1: Result is changed to softfailed as specified in the title of the ticket
- 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
#2 Updated by okurz about 2 months ago
- Tags set to reactive work
- Project changed from openQA Project to QA
- Priority changed from Normal to High
- Target version set to Ready
#3 Updated by cdywan about 2 months 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
#4 Updated by mkittler about 1 month ago
- Assignee set to mkittler
#5 Updated by mkittler about 1 month 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?
#6 Updated by mkittler about 1 month 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 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:  useradd FAIL\nBaseline:  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 useradd_D PASS", result="ok")
#7 Updated by mkittler about 1 month 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.)
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).
openqa-label-known-issues script is fine. It has likely just not been triggered for this job.
#8 Updated by mkittler about 1 month 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.
#9 Updated by mkittler about 1 month 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.
#10 Updated by mkittler about 1 month 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).
#11 Updated by okurz about 1 month 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
#12 Updated by okurz about 1 month 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
#13 Updated by okurz about 1 month ago
- Project changed from QA to openQA Project
- Category set to Concrete Bugs
#15 Updated by mkittler 18 days 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).
#16 Updated by okurz 16 days ago
- Due date set to 2023-03-24
#17 Updated by mkittler 12 days ago
PR for 4 (and 4.1): https://github.com/os-autoinst/openQA/pull/5035
#18 Updated by mkittler 10 days 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.