Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix error message for non-existing VM passed to a VMProperty #571

Merged
merged 1 commit into from
Dec 16, 2023

Conversation

HW42
Copy link
Contributor

@HW42 HW42 commented Dec 11, 2023

Trying to set a VMProperty (netvm, template, ...) to a non-existing VM previously raised a QubesVMNotFoundError. While this is not wrong qvm-prefs can't distinguish this from other cases (for example see 1) so instead raise QubesPropertyValueError with a matching error message.

Fixes QubesOS/qubes-issues#6966

Trying to set a VMProperty (netvm, template, ...) to a non-existing VM
previously raised a QubesVMNotFoundError. While this is not wrong
qvm-prefs can't distinguish this from other cases (for example see [1])
so instead raise QubesPropertyValueError with a matching error message.

Fixes QubesOS/qubes-issues#6966

[1]: https://github.com/QubesOS/qubes-core-admin-client/blob/e7892560a3a64b924c20dd81bb7f718b5b1c49de/qubesadmin/base.py#L382
Copy link

codecov bot commented Dec 11, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (2c18f2c) 68.42% compared to head (83b1959) 68.40%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #571      +/-   ##
==========================================
- Coverage   68.42%   68.40%   -0.02%     
==========================================
  Files          56       56              
  Lines       11163    11163              
==========================================
- Hits         7638     7636       -2     
- Misses       3525     3527       +2     
Flag Coverage Δ
unittests 68.40% <100.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@HW42
Copy link
Contributor Author

HW42 commented Dec 11, 2023

If I understand the failing codeconv report correctly the complaints is that now the 2 lines in the initializer of QubesVMNotFoundError are no longer called when running the tests. For the changed code this is expected. Adding a tests for another, unrelated, path where QubesVMNotFoundError is instantiated seems out of scope for this PR.

@marmarek marmarek merged commit fca4b99 into QubesOS:main Dec 16, 2023
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bad error message when setting the template of a qube to a qube that doesn’t exist
2 participants