Improved device name collision recognition in autoinstallation #583
Conversation
| @@ -353,7 +353,7 @@ def keep_install_network_value(value) | |||
| before(:each) do | |||
| allow(Yast::LanItems) | |||
| .to receive(:Items) | |||
| .and_return(0 => { "ifcfg" => "eth0" }) | |||
| .and_return(0 => { "ifcfg" => "eth0", "udev" => { "net" => ["ATTR{address}==\"24:be:05:ce:1e:91\"", "KERNEL==\"eth*\"", "NAME=\"eth0\""] } }) | |||
There was a problem hiding this comment.
Is this supposed to be a test for this PR?
What about a new test case that fails without the fix and succeeds with the fix?
What about summarizing the 62 Bugzilla comments so far so that a reviewer gets an idea of what the bug even is?
There was a problem hiding this comment.
this is necessary fix to keep the testsuite working after the patch. I'll add testsuite for new methods.
mchf
changed the title
mchf
force-pushed
the
bnc1056109-udevs
branch
2 times, most recently
from
4c685fe
to
3c70075
Compare
mchf
force-pushed
the
bnc1056109-udevs
branch
from
3c70075
to
97837e1
Compare
mchf
changed the title
mchf
force-pushed
the
bnc1056109-udevs
branch
from
d2fc55f
to
4ae8540
Compare
mchf
force-pushed
the
bnc1056109-udevs
branch
from
4ae8540
to
929a410
Compare
There was a problem hiding this comment.
Please write a test for the reported bug. I think Comment 31 is a good start.
test/network_autoyast_test.rb
Outdated
| .and_return( | ||
| 0 => { | ||
| "ifcfg" => "eth0", | ||
| "renamed_to" => !renamed_to.nil? ? renamed_to : nil, |
There was a problem hiding this comment.
"renamed_to" => renamed_to, does work :)
| @@ -280,20 +281,34 @@ def assign_udevs_to_devs(udev_rules) | |||
| end | |||
| next if !matching_item | |||
|
|
|||
| name_from = LanItems.GetDeviceName(item) | |||
| name_from = item_name(item) | |||
There was a problem hiding this comment.
OK, I think I can even see that this is the bug fix: realizing that name_from may also have been renamed. Right?
So we should be able to write a test case that mimics the bug report, fails without this fix and succeeds with it.
There was a problem hiding this comment.
yes. I'd bet that I have an AY integration test for it ... but cannot find it. I'll do something with that.
There was a problem hiding this comment.
I have good news:
Failure/Error: expect(names.sort).to eql ["eth0", "eth1", "eth2"]
expected: ["eth0", "eth1", "eth2"]
got: ["eth0", "eth0", "eth2"]
This is the new test when run with the old code. So yes, it really tests the bug (and it passes with the new code.
Thanks!
|
thanks for the review |
bsc#1056109
https://trello.com/c/A0ujYsUt
Previously on this topic: #533
This PR solves issues when ay udev rules leads to a collision in device renaming. For example when AY profile defines rule eth0 -> eth1 (-> means "rename to") and eth1 exists in the system.