Edward Haas has posted comments on this change. Change subject: net: introduce acquire module ......................................................................
Patch Set 16: Code-Review-1 (2 comments) https://gerrit.ovirt.org/#/c/60974/16/tests/network/ifacquire_test.py File tests/network/ifacquire_test.py: Line 69: @mock.patch.object(ifacquire.ifcfg, 'ifdown') Line 70: @mock.patch.object(ifacquire, 'open', create=True) Line 71: def test_do_not_acquire_owned_nic(self, mock_open, mock_ifdown, mock_kill, Line 72: mock_flush): Line 73: with ifacquire.Transaction() as a: I'm not clear of why it is split between prepare and turn_down. Couldn't these be part of the Transaction init? The disable_onboot should not be part of this transaction, it should be set when Engine instructs to persist the config. Line 74: a.prepare(ifaces=[NIC_NAME], netinfo_nets=NETINFO_NETS) Line 75: a.turn_down() Line 76: a.disable_onboot() Line 77: Line 112: def _test_acquire_ifcfg_nic(self, mock_isfile, mock_ifdown, Line 113: original_ifcfg, Line 114: ifcfg_after_turn_down, Line 115: ifcfg_after_disable_onboot): Line 116: with mock_ifacquire_opens() as (open_handle, atomic_write_handle): I do not understand why you are testing the atomic write. You just need to test ifacquire, assuming the atomic write works as expected (you should have tests for that already). So I would recommend mocking only utils.atomic_write, checking that it is called with. And use it as a decorator and drop the contextmanager.. Line 117: open_handle.readlines.return_value = original_ifcfg Line 118: atomic_write_handle.readlines.return_value = original_ifcfg Line 119: Line 120: with ifacquire.Transaction() as a: -- To view, visit https://gerrit.ovirt.org/60974 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I180cfd7d69c0ae0a24188bc3d909b9d3d7c12145 Gerrit-PatchSet: 16 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Edward Haas <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček <[email protected]> Gerrit-Reviewer: gerrit-hooks <[email protected]> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/admin/lists/[email protected]
