Install module package IDs on module enable
#2086
Conversation
Module packages should be explicitly specified when enabling modules, see rpm-software-management/libdnf#1656.
- Replace some occurrences of `base.enable` with `base._moduleContainer.enable(_, _, False)` to not count as a module state modification. - Don't test installing old versions of modules, this is not supported by DNF
|
I've updated the tests. They are passing now, but I've commented out |
| @@ -204,35 +205,36 @@ def test_install_profile_latest(self): | |||
|
|
|||
| def test_install_profile(self): | |||
| self.test_enable_name_stream() | |||
| self.module_base.install(["httpd:2.4:1/default"]) | |||
| self.module_base.install(["httpd:2.4:2/default"]) | |||
There was a problem hiding this comment.
May I ask you why have you changed the argument "httpd:2.4:1/default" to "httpd:2.4:2/default"? In case that both arguments works as expected what about to add both options to test scenarios?
There was a problem hiding this comment.
Leaving it as httpd:2.4:1/default does not work. This test calls test_enable_name_stream, which calls self.module_base.enable(["httpd:2.4"]), which adds a request to install oneof 'httpd:2.4:1::', 'httpd:2.4:2::'. So the solver sees two install oneofs:
job install oneof httpd:2.4:NoRequires-1.noarch@_all httpd:2.4:NoRequires-2.noarch@_all [forcebest,setevr,setarch]
job install oneof httpd:2.4:NoRequires-1.noarch@_all [forcebest,setevr,setarch]
and produces the following problems:
problem 70e5c809 info package httpd:2.4:NoRequires-2.noarch conflicts with module(httpd) provided by httpd:2.4:NoRequires-1.noarch
problem 70e5c809 solution 154300cc deljob install oneof httpd:2.4:NoRequires-1.noarch@_all [forcebest,setevr,setarch]
problem 70e5c809 solution 2d67ee55 deljob install oneof httpd:2.4:NoRequires-1.noarch@_all httpd:2.4:NoRequires-2.noarch@_all [forcebest,setevr,setarch]
There was a problem hiding this comment.
Oh I see.
I think we can remove self.test_enable_name_stream(). The line self.module_base.install(["httpd:2.4:2/default"]) also enable module httpd:2.4. The problem is that test does not state what starting condition is expected for the test.
I see following scenarios:
- install a profile from a module stream that is not yet enabled
- install a profile from a module stream that is already enabled (
using self.base._moduleContainer.enable("httpd", "2.4", False)) - install a profile from not the latest module item from a module stream that is not yet enabled
| @@ -106,7 +106,7 @@ def test_enable_invalid(self): | |||
| self.module_base.enable(["httpd:invalid"]) | |||
|
|
|||
| def test_enable_different_stream(self): | |||
| self.module_base.enable(["httpd:2.4"]) | |||
| self.base._moduleContainer.enable("httpd", "2.4", False) | |||
There was a problem hiding this comment.
This is not the problem of you PR, but the problem was here before. As you can see, tests are without any description, therefore it is difficult to guess what test is testing (WHY?), what test actually do (How?).
This test tests whether it is possible to change stream. The first part is setting of initial condition for the test - one stream is enabled. The second part is testing part. May I ask you to add some comments to the tests?
| "libnghttp2-1.21.1-1.x86_64", # expected behaviour, non-modular rpm pulled in | ||
| ] | ||
| self.assertInstalls(expected) | ||
|
|
||
| def test_install_two_profiles_different_versions(self): | ||
| """ |
There was a problem hiding this comment.
This test cannot work with the new implementation. In theory, it would be possible to add a workaround but the condition will be very complicated - think about algorithm that will join elements based on name, stream, and context (not version). Personally I don't feel comfortable to add another very complex logic to the code. I am glad, that we have this test, because it discovered additional change in the behavior with the new code.
Module packages should be explicitly specified when enabling modules.
This is the dnf companion to rpm-software-management/libdnf#1656.
For https://issues.redhat.com/browse/RHEL-16580.
Currently a draft since tests are failing.