Use new exception for handling absent policy#301
Conversation
marmarta
left a comment
marmarta
left a comment
There was a problem hiding this comment.
Test fails are due to dependency on changes in core. Otherwise looks good.
|
QubesOS/qubes-core-qrexec#223 (comment) I said here that I'd not inherit from |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #301 +/- ##
==========================================
- Coverage 92.99% 92.88% -0.11%
==========================================
Files 64 64
Lines 13312 13349 +37
==========================================
+ Hits 12379 12399 +20
- Misses 933 950 +17 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
openQArun PR_LABEL=openqa-pending TEST=system_tests_gui_tools Didn't trigger gitlab nor openqa pipeline? |
|
openQArun PR_LABEL=openqa-pending TEST=system_tests_gui_tools This repo probably isn't in the allowed list. |
| if admin_exc | ||
| else subprocess.CalledProcessError | ||
| ) as ex: | ||
| if getattr(ex, "returncode", None) == 126: |
There was a problem hiding this comment.
This is wrong - it makes real subprocess.CalledProcessError not handled anymore, for example when the call was denied by the policy (which is the exit code 126), but could be also other cases.
Same comment for the other functions.
There was a problem hiding this comment.
I started with separate except clauses but wanted to avoid duplication. I will return to that, but will have to find a way to deal in case the exception class is not present yet (outdated qrexec). You solution to reassign the exception to subprocess.CalledProcessError might be the one.
OpenQA test summaryComplete test suite and dependencies: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2026042817-devel&flavor=pull-requests Test run included the following:
New failures, excluding unstableCompared to: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2026032404-devel&flavor=update
Failed tests43 failures
Fixed failuresCompared to: https://openqa.qubes-os.org/tests/170766#dependencies 27 fixed
Unstable testsDetails
Performance TestsPerformance degradation:No issues Remaining performance tests:39 tests
|
|
Passed openqa. I'd like to run the full test suite, do you prefer the label is set back to openqa-pending to use other PRs also or keep it as openqa-group-3? |
4 participants
For: QubesOS/qubes-issues#10746
Requires: QubesOS/qubes-core-qrexec#223