Introduce LocalVM to extract BaseVM being smaller class#637
Conversation
|
@marmarek can you run openQA on it please? |
|
Sure, I can, but I think you may want to first take care of unit tests, this one looks like a real issue: |
fepitre
force-pushed
the
localvm
branch
3 times, most recently
from
0e2e8d9 to
9b8407f
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #637 +/- ##
==========================================
+ Coverage 70.14% 70.24% +0.09%
==========================================
Files 59 59
Lines 12726 12739 +13
==========================================
+ Hits 8927 8948 +21
+ Misses 3799 3791 -8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Fixed. |
OpenQA test summaryComplete test suite and dependencies: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2025032603-4.3&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=2025031804-4.3&flavor=update
Failed tests22 failures
Fixed failuresCompared to: https://openqa.qubes-os.org/tests/132953#dependencies 14 fixed
Unstable testsDetailsPerformance TestsPerformance degradation:13 performance degradations
Remaining performance tests:59 tests
|
It prepares the work for RemoteVM.
| `domain-init` and `domain-load` events. | ||
| """ | ||
|
|
||
| # pylint: disable=no-member |
There was a problem hiding this comment.
Why? it is defined in __init__
| import qubesdb # pylint: disable=import-error | ||
|
|
||
| try: | ||
| # pylint: disable=no-member |
| class. | ||
| """ | ||
| # cleanup old watch connection first, if any | ||
| # pylint: disable=no-member |
There was a problem hiding this comment.
I don't remember, I double checked, now it's good locally. I removed the three disable.
| ) | ||
| ) | ||
| if not issubclass(vmclass, BaseVM): | ||
| if not issubclass(vmclass, LocalVM): |
There was a problem hiding this comment.
While default to LocalVM makes sense, should it really forbid using RemoteVM as some property value?
|
|
||
| def __lt__(self, other: object): | ||
| if not isinstance(other, BaseVM): | ||
| if not isinstance(other, LocalVM): |
There was a problem hiding this comment.
This should remain BaseVM, otherwise sorting VM objects will not work with RemoteVM
|
PipelineRetry |
3 participants
It prepares the work for RemoteVM.