Yaniv Bronhaim has posted comments on this change.

Change subject: test: Replacing MonkeyPatch with mock.patch example
......................................................................


Patch Set 3:

(2 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
> The conversion from mockeypatch to mock includes mockeypatch to None.
you can separate it to another patch with explanation why it is not needed any 
more in the commit msg. more appropriate


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',
> The original code was better. Having a way to monkey patch multiple modules
we do prefer to use the standard mock. i don't see any issue with this 
approach, its not uglier not prettier than using MonkeyPatch and it doesn't 
matter. the idea here is to use well documented and standard python mock library
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',


-- 
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 <edwa...@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
Gerrit-Reviewer: Edward Haas <edwa...@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com>
Gerrit-Reviewer: Petr Horáček <phora...@redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczew...@gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com>
Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org>
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to