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 <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Edward Haas <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Petr Horáček <[email protected]> Gerrit-Reviewer: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: gerrit-hooks <[email protected]> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
