Edward Haas has posted comments on this change.
Change subject: net test: Replacing MonkeyPatch with mock.patch for netinfo_test
......................................................................
Patch Set 2:
(6 comments)
https://gerrit.ovirt.org/#/c/61269/2/tests/network/netinfo_test.py
File tests/network/netinfo_test.py:
PS2, Line 111: lambda: {'fake': {'bridged': True}}
i think we can use return_value here. Not sure if we want to use
shorter 'l
Using return_value suggests that we need a generated mock object, but
we do not need it here, we just provide a stub.
PS2, Line 118: vdsm.network.netinfo.cache
won't be better to import cache with others modules from netinfo
and then u
Yes, but it is not a must and importing the whole module seemed
overdoing.
I can do it if you think it is worth it.
PS2, Line 118: @mock.patch('vdsm.network.netinfo.cache.getLinks')
: @mock.patch('vdsm.network.netinfo.cache.libvirt.networks')
if we do not specify return_value, mocked function returns the same
and has
If it is not specified, it returns None, which is fine in this case.
PS2, Line 149: partial(_fakeTypeDetection, ipwrapper.Link)
we can use lambda requiring one parameter, _fakeTypeDetections is
just:
Ok, will clean it up in a following patch.
PS2, Line 159: """Creates a test fixture so that nics() reports:
: physical nics: em, me, me0, me1, hid0 and hideous
: dummies: fake and fake0
: bonds: jbond (over me0 and me1)"""
if it is not a part of method docstring, we should write it as inline
comme
Done
PS2, Line 164: ipwrapper.Link(address='f0:de:f1:da:aa:e7', index=2,
: linkType=ipwrapper.LinkType.NIC, mtu=1500,
: name='em', qdisc='pfifo_fast',
state='up'),
: ipwrapper.Link(address='ff:de:f1:da:aa:e7', index=3,
: linkType=ipwrapper.LinkType.NIC, mtu=1500,
: name='me', qdisc='pfifo_fast',
state='up'),
: ipwrapper.Link(address='ff:de:fa:da:aa:e7', index=4,
: linkType=ipwrapper.LinkType.NIC, mtu=1500,
: name='hid0', qdisc='pfifo_fast',
state='up'),
: ipwrapper.Link(address='ff:de:11:da:aa:e7', index=5,
: linkType=ipwrapper.LinkType.NIC, mtu=1500,
: name='hideous',
qdisc='pfifo_fast', state='up'),
: ipwrapper.Link(address='66:de:f1:da:aa:e7', index=6,
: linkType=ipwrapper.LinkType.NIC, mtu=1500,
: name='me0', qdisc='pfifo_fast',
state='up',
: master='jbond'),
: ipwrapper.Link(address='66:de:f1:da:aa:e7', index=7,
: linkType=ipwrapper.LinkType.NIC, mtu=1500,
: name='me1', qdisc='pfifo_fast',
state='up',
: master='jbond'),
: ipwrapper.Link(address='ff:aa:f1:da:aa:e7', index=34,
: linkType=ipwrapper.LinkType.DUMMY, mtu=1500,
: name='fake0',
qdisc='pfifo_fast', state='up'),
: ipwrapper.Link(address='ff:aa:f1:da:bb:e7', index=35,
: linkType=ipwrapper.LinkType.DUMMY, mtu=1500,
: name='fake', qdisc='pfifo_fast',
state='up'),
: ipwrapper.Link(address='66:de:f1:da:aa:e7', index=419,
: linkType=ipwrapper.LinkType.BOND, mtu=1500,
: name='jbond',
qdisc='pfifo_fast', state='up')
: ]
can we define it outside the method in a constant?
Done
--
To view, visit
https://gerrit.ovirt.org/61269
To unsubscribe, visit
https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia4e14162bb2773c50e551f4409c4c37cffe9f2a3
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas <edwardh(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes