Edward Haas has posted comments on this change. Change subject: test: Use mock module for testing ......................................................................
Patch Set 7: (2 comments) https://gerrit.ovirt.org/#/c/55342/7/tests/network/netswitch_test.py File tests/network/netswitch_test.py: Line 23: from nose.plugins.attrib import attr Line 24: Line 25: from vdsm.network import netswitch Line 26: Line 27: from testlib import mock > why do indirect import? It won't be nice to repeat this noisy logic on each import. We usually use compat module to hide this noise, but for tests it seems more appropriate to locate it in testlib. Line 28: from testlib import VdsmTestCase Line 29: Line 30: Line 31: @attr(type='unit') https://gerrit.ovirt.org/#/c/55342/7/tests/testlib.py File tests/testlib.py: Line 38: try: Line 39: from unittest import mock Line 40: except ImportError: # py2 Line 41: import mock Line 42: mock > why do you import mock here if it's not needed? This is a proxy, see compat for this pattern. Line 43: Line 44: from nose import config Line 45: from nose import core Line 46: from nose import result -- To view, visit https://gerrit.ovirt.org/55342 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1c0af7baab7c35a2617bd60a62a0b1534e5f8894 Gerrit-PatchSet: 7 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: Ondřej Svoboda <[email protected]> Gerrit-Reviewer: Petr Horáček <[email protected]> Gerrit-Reviewer: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Sandro Bonazzola <[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
