Edward Haas has posted comments on this change. Change subject: test: Replacing MonkeyPatch with mock.patch example ......................................................................
Patch Set 2: (3 comments) https://gerrit.ovirt.org/#/c/55603/2/tests/network/netinfo_test.py File tests/network/netinfo_test.py: Line 87 Line 88 Line 89 Line 90 Line 91 > what about those mocks ^? It was converted to None :) These are mocks that are no longer needed. These are good examples of why a monkey patch is not a good choice for some mocking: At some point a mock was needed, after several code changes, the dependency that required the mock has been dropped (or changed).. However, the mock has not been taken out, as no error has hinted that it should. To avoid such cases, when a mock is used, we should check it was called/used. Line 114 Line 115 Line 116 Line 117 Line 118 > what about _detectType and _GetBondingOptions mocks? Same as the previous. Line 282 Line 283 Line 284 Line 285 Line 286 > how is it not needed anymore? Dan helped me figure this out, I had trouble understanding it myself (why it passes without the mocking). In testrunner.py we assign: constants.P_VDSM = "../vdsm/" for local runs. -- 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: 2 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
