Change in vdsm[master]: test: Replacing MonkeyPatch with mock.patch example
edwardh at redhat.com
edwardh at redhat.com
Sun Apr 10 06:42:41 UTC 2016
Edward Haas has posted comments on this change.
Change subject: test: Replacing MonkeyPatch with mock.patch example
......................................................................
Patch Set 3:
(3 comments)
https://gerrit.ovirt.org/#/c/55603/3/tests/network/netinfo_test.py
File tests/network/netinfo_test.py:
Line 114
Line 115
Line 116
Line 117
Line 118
> you can separate it to another patch with explanation why it is not needed
Ok guys, splitting this one.
Line 102:
Line 103: for passed, operstate, expected in values:
Line 104: with mock.patch('vdsm.netinfo.nics.io.open',
Line 105: lambda x: io.BytesIO(str(passed))), \
Line 106: mock.patch('vdsm.netinfo.nics.operstate',
> Using standard modules instead of our own is good idea in general, but:
I tried to prettify this in a following patch.
I don't think anyone raised the requirement to get rid of the existing monkey patches or to avoid py.test ones.
I do think that adding a dependency on a specific test runner is not a good choice if there are other options available, for the same reasons we want to move to py.test: In the future, a better runner may pop up.
mock provides a full mock tooling that covers monkey patches and provides much more capabilities (that I needed and used in the previous patch).
Line 107: lambda x: operstate):
Line 108: self.assertEqual(nics.speed('fake_nic'), expected)
Line 109:
Line 110: @mock.patch('vdsm.netinfo.cache.netinfo.networks',
Line 115: # it should.
Line 116: get()
Line 117:
Line 118: @mock.patch('vdsm.netinfo.cache.getLinks', lambda: [])
Line 119: @mock.patch('vdsm.netinfo.cache.netinfo.networks', lambda: {})
> I don't like this magic, importing stuff behind by back.
Well, I did not like the fact that I need to import into the test something that the test itself is not using.
Explicitly importing in order to mock its members is noise IMO.
But I guess this is a taste thing and probably influenced by our experience in the past.
Line 120: def testGetEmpty(self):
Line 121: result = {}
Line 122: result.update(get())
Line 123: self.assertEqual(result['networks'], {})
--
To view, visit https://gerrit.ovirt.org/55603
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I34ef99c00e7e2e4bbf13a52ec8471815e81d2a9e
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas <edwardh at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Edward Haas <edwardh at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Petr Horáček <phoracek at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list