Change in vdsm[master]: test: Use mock module for testing

edwardh at redhat.com edwardh at redhat.com
Thu Mar 31 06:17:47 UTC 2016


Edward Haas has posted comments on this change.

Change subject: test: Use mock module for testing
......................................................................


Patch Set 3:

(4 comments)

https://gerrit.ovirt.org/#/c/55342/3/tests/network/netswitch_test.py
File tests/network/netswitch_test.py:

Line 47:     @mock.patch('vdsm.network.netswitch.libvirtNets2vdsm')
Line 48:     @mock.patch('vdsm.network.netswitch.libvirt_nets')
Line 49:     @mock.patch('vdsm.network.netswitch.legacy_switch')
Line 50:     def test_setup(self, mock_switch, mock_libvirt_nets, mock_libvirtNets2vdsm,
Line 51:                    mock_netinfo_get, mock_CachingNetInfo, mock_connectivity):
> What is mock_switch and the other parameters?
These are the mocks created automatically for you.
I would not expect anything to clash, but if it does, please let me know.
I know there is even a py.test plugin that adds some minor functionality over mock, but it does not worth depending on py.test for it.

mock.patch can be used as a context manager as well, but I found it more convenient this way.
Line 52: 
Line 53:         mock_libvirt_nets.return_value = None
Line 54:         mock_libvirtNets2vdsm.return_value = None
Line 55:         mock_netinfo_get.return_value = None


Line 56:         mock_CachingNetInfo.return_value = None
Line 57: 
Line 58:         netswitch.setup(self.NET_TEST, self.BOND_TEST, self.OPT_TEST, False)
Line 59: 
Line 60:         mock_configurator = mock_switch.ConfiguratorClass().__enter__()
> Why do you need to call __enter__(), and where the call to __exit__()?
This is the object returned by the ConfiguratorClass contextmanager. The 'remove_networks' method is taking it as an argument in the production code, so we check what will be returned by the mock and use it to assert the arguments it got.
I think that there are better ways of doing this, but we need more experience playing with mock. For now it seems to me like good enough.
Line 61:         mock_switch.remove_networks.assert_called_with(self.NET_TEST,
Line 62:                                                        self.BOND_TEST,
Line 63:                                                        mock_configurator,
Line 64:                                                        None, None)


Line 60:         mock_configurator = mock_switch.ConfiguratorClass().__enter__()
Line 61:         mock_switch.remove_networks.assert_called_with(self.NET_TEST,
Line 62:                                                        self.BOND_TEST,
Line 63:                                                        mock_configurator,
Line 64:                                                        None, None)
> What if this was called several times? which call are you asserting - the f
It is called once in the production code.
If I'm not mistaken, this is the last one.
You can assert on the number of calls or even get the whole history record, there are many options available.
https://docs.python.org/3/library/unittest.mock.html#the-mock-class
Line 65:         mock_switch.bonds_setup.assert_called_with(self.BOND_TEST,
Line 66:                                                    mock_configurator,
Line 67:                                                    None,
Line 68:                                                    False)


Line 64:                                                        None, None)
Line 65:         mock_switch.bonds_setup.assert_called_with(self.BOND_TEST,
Line 66:                                                    mock_configurator,
Line 67:                                                    None,
Line 68:                                                    False)
> Can you check the order of the calls? does it matter?
You can check that for the same mock (or its children), but not between different ones (like different funcs).
That's also not recommended, it couples the test to the tested unit too much, internal optimization in the unit should not be visible to the test, so it won't break often.
Line 69:         mock_switch.add_missing_networks.assert_called_with(mock_configurator,
Line 70:                                                             self.NET_TEST,
Line 71:                                                             self.BOND_TEST,
Line 72:                                                             None)


-- 
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: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas <edwardh at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Edward Haas <edwardh at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Ondřej Svoboda <osvoboda at redhat.com>
Gerrit-Reviewer: Petr Horáček <phoracek at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list