Petr Horáček has posted comments on this change.
Change subject: net test: Replacing MonkeyPatch with mock.patch for netinfo_test
......................................................................
Patch Set 2: Code-Review-1
(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 'lambda'
or be consistent with return_value.
PS2, Line 118: vdsm.network.netinfo.cache
won't be better to import cache with others modules from netinfo and then use
patch.object?
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 the same
side-effect as the original one.
PS2, Line 149: partial(_fakeTypeDetection, ipwrapper.Link)
we can use lambda requiring one parameter, _fakeTypeDetections is just:
def _fakeTypeDetection(cls, devname):
pass
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 comment (with # on
each line)
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?
--
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