Prettify output and warning to not be deceived by summary#40
Conversation
| output_string = "{}\n" \ | ||
| "> [!WARNING]\n" \ | ||
| "> Failed tests may be incomplete if they failed to " \ | ||
| "upload, always check the complete test suite link.\n\n" \ |
There was a problem hiding this comment.
What is missing is listing upload failures - which this script does know about...
There was a problem hiding this comment.
Specifically https://github.com/QubesOS/openqa-tests-qubesos/blob/main/utils/lib/openqa_api.py#L229 and https://github.com/QubesOS/openqa-tests-qubesos/blob/main/utils/lib/openqa_api.py#L622-L629 - failure in "system_tests" job should be reported as upload failure if there are no detailed results in that job.
|
|
||
| if degradation: | ||
| s += (f" :small_red_triangle: ( previous " | ||
| green_circle = "🔻" |
There was a problem hiding this comment.
I'm pretty sure this is not green circle ;)
There was a problem hiding this comment.
ahhhhhhhhhhhhhhhhhhhhhhh
Sometimes Github doesn't render the emojis, unicode delegates rendering to the browser.
c4ddd54 to
54ba5a1
Compare
OpenQA test summaryComplete test suite and dependencies: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2025082411-4.3&flavor=pull-requests Test run included the following:
Upload failures
New failuresCompared to: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2025081011-4.3&flavor=update
Failed tests11 failures
Fixed failuresCompared to: https://openqa.qubes-os.org/tests/149225#dependencies 80 fixed
Performance TestsPerformance degradation:19 performance degradations
Remaining performance tests:155 tests
Would label it with: ['openqa-failed'] |
| failed_tests_details += ' * ' + str(fail) + '\n' | ||
| upload_failures.extend(results[k]) | ||
| if not upload_failures: | ||
| output_string += "No upload failures!\n" |
There was a problem hiding this comment.
I'd say simply skip the section if there are no upload failures.
| for test_group in json_data['job']['testresults']: | ||
| if test_group['result'] == 'passed': | ||
| continue | ||
| failure_per_group[test_group['name']] = [] |
There was a problem hiding this comment.
Do you actually reference failures from different group in a loop iteration? If not, maybe a simple list (not a dict of lists) would be enough? Same for delayed list below.
But also see the next comment.
There was a problem hiding this comment.
Not needed indeed.
| elif failure.name == "system_tests": | ||
| delayed_failure_per_group[failure.name].append(failure) | ||
|
|
||
| if not failure_per_group[test_group['name']]: |
There was a problem hiding this comment.
Does this work at all? If upload failed, you won't see those other groups at all...
There was a problem hiding this comment.
See the example here, where it worked: #40 (comment)
There was a problem hiding this comment.
Did it? I don't see audio failures listed for example (https://openqa.qubes-os.org/tests/150378)
There was a problem hiding this comment.
I guess this is another issue but unrelated to this PR?
See the original comment: QubesOS/qubes-core-admin#718 (comment)
I don't find the audio failures there also.
There was a problem hiding this comment.
Wait, I am checking the build numbers....
There was a problem hiding this comment.
The error occurs because of the f in upload_failures test...
OpenQA test summaryComplete test suite and dependencies: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2025082411-4.3&flavor=pull-requests Test run included the following:
Upload failures
New failuresCompared to: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2025081011-4.3&flavor=update
Failed tests13 failures
Fixed failuresCompared to: https://openqa.qubes-os.org/tests/149225#dependencies 80 fixed
Performance TestsPerformance degradation:19 performance degradations
Remaining performance tests:155 tests
Would label it with: ['openqa-failed'] |
|
The result appears to be in line with the |
diff /tmp/report.old /tmp/report
70d69
< * system_tests_backup
103d101
< * system_tests_basic_vm_qrexec_gui_ext4These were header without failures under them. |
marmarek
left a comment
marmarek
left a comment
There was a problem hiding this comment.
One minor thing remained and otherwise it's good.
| failure_per_group = {} | ||
| delayed_failure_per_group = {} |
There was a problem hiding this comment.
Those two are not needed anymore
marmarek
left a comment
marmarek
left a comment
There was a problem hiding this comment.
I'll merge it as is for now, as it improves several things, but there is a minor regression:
system_tests_pvgrub_salt_storage
system_tests: [Fail](https://openqa.qubes-os.org/tests/153694#step/system_tests/91) (unknown)
Tests qubes.tests.integ.storage failed (exit code 1), details repor...
system_tests: [Failed](https://openqa.qubes-os.org/tests/153694#step/system_tests/128) (test died)
# Test died: Some tests failed at qubesos/tests/system_tests.pm lin...
StorageReflinkOnBtrfs: [test_003_snapshot](https://openqa.qubes-os.org/tests/153694#step/StorageReflinkOnBtrfs/4) (error + timeout + cleanup)
qubes.exc.QubesVMShutdownTimeoutError: Domain shutdown timed out: '...
If detailed results are uploaded, the generic system_tests points should be skipped.
2 participants
Didn't test the warning message rendering.