Edward Haas has posted comments on this change.

Change subject: net: introduce acquire module
......................................................................


Patch Set 16:

(2 comments)

https://gerrit.ovirt.org/#/c/60974/16/tests/network/ifacquire_test.py
File tests/network/ifacquire_test.py:

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):
> Mock themselves or mock_ifacquire_opens context manager?
The mocks, there will be no need for the helper context manager.
I'm not clear what is not working exactly and why you had to use a helper to 
create the two mocks.
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:


Line 133: 
Line 134:     @mock.patch.object(ifacquire.ifcfg, 'ifdown', return_value=None)
Line 135:     @mock.patch.object(ifacquire.os.path, 'isfile', return_value=True)
Line 136:     def test_rollback_acquired_ifcfg_nic(self, mock_isfile, 
mock_ifdown):
Line 137:         with mock_ifacquire_opens() as (open_handle, 
atomic_write_handle):
> How?
Just because there is no helper fun in between like in the previous case.
Line 138:             open_handle.readlines.return_value = NIC_IFCFG
Line 139:             atomic_write_handle.readlines.return_value = NIC_IFCFG
Line 140: 
Line 141:             with self.assertRaises(TestException):


-- 
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 <phora...@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
Gerrit-Reviewer: Edward Haas <edwa...@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček <phora...@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/admin/lists/vdsm-patches@lists.fedorahosted.org

Reply via email to