Rename disp template if only preloads are running#429
Rename disp template if only preloads are running#429ben-grande wants to merge 1 commit intoQubesOS:mainfrom
Conversation
ben-grande
force-pushed
the
preload-template-switch
branch
3 times, most recently
from
9adeb58 to
541827d
Compare
ben-grande
force-pushed
the
preload-template-switch
branch
from
541827d to
ce23ab7
Compare
|
At this point, failed tests only occurs because of missing depends mentioned on op. |
ben-grande
force-pushed
the
preload-template-switch
branch
from
ace3820 to
ce23ab7
Compare
| settings_window.delete_vm_button.click() | ||
|
|
||
| if vm.name == "fedora-36": | ||
| # Qubes with same names from mock_app may be on the system, such as |
There was a problem hiding this comment.
I'm not sure what this comment means. Same names to what?
There was a problem hiding this comment.
I kind of still don't get it; the system has just what is in the mock, it's quite expected and not "may be there or not"?
| dependencies = [ | ||
| (vm, prop) | ||
| for (vm, prop) in utils.vm_dependencies(self.qubes_app, vm) | ||
| if not vm or (vm and not manager_utils.is_preload(vm)) |
There was a problem hiding this comment.
is there any situation in which a preloaded VM should be another VM's dependency? It feels to me like they should just never be returned by the vm_dependencies function?
There was a problem hiding this comment.
By default, there is no such situation, but if the user chooses to make it, we need to show.
There was a problem hiding this comment.
Yes, I didn't test what happens if I make a preload a netvm of a qube, need to test...
There was a problem hiding this comment.
By default, there is no such situation, but if the user chooses to make it, we need to show.
but here and everywhere else this function is used you are deliberately excluding preloaded dispvms, so I think they should just be excluded by the function itself?
There was a problem hiding this comment.
Yes, I didn't test what happens if I make a preload a netvm of a qube, need to test...
I meant the opposite, cause being an audiovm or netvm is handled by QubesOS/qubes-core-admin#733. Preloaded disposables can theoretically be one other property, guivm of a qube... not too useful right now but there is nothing blocking it in qubes.ext.gui.
but here and everywhere else this function is used you are deliberately excluding preloaded dispvms, so I think they should just be excluded by the function itself?
You are right. I think the tests can stay, here is the only place where a qube can be renamed.
There was a problem hiding this comment.
Or not even tests, because it becomes irrelevant one qubesadmin is fixed.
There was a problem hiding this comment.
Ok, so this issue remains - I think the function vm_dependencies should do the filtering, so that here you wouldn't need a list comprehension (and just use it like in previous versions)
There was a problem hiding this comment.
I am closing this PR in favor of:
ben-grande
force-pushed
the
preload-template-switch
branch
from
ce23ab7 to
d01ef8b
Compare
OpenQA test summaryComplete test suite and dependencies: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2025101510-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=2025081011-4.3&flavor=update
Failed tests18 failures
Fixed failuresCompared to: https://openqa.qubes-os.org/tests/149225#dependencies 83 fixed
Unstable testsDetailsPerformance TestsPerformance degradation:14 performance degradations
Remaining performance tests:165 tests
|
|
PipelineRetryFailed |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #429 +/- ##
==========================================
+ Coverage 69.17% 69.28% +0.10%
==========================================
Files 17 17
Lines 3913 3930 +17
==========================================
+ Hits 2707 2723 +16
- Misses 1206 1207 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
4 participants
Fixes: QubesOS/qubes-issues#10227
For: QubesOS/qubes-issues#1512
Depends on: