Giuseppe Vallarelli has posted comments on this change. Change subject: tests: Adding functional tests for networking ......................................................................
Patch Set 8: (6 inline comments) .................................................... File tests/functional/networkTests.py Line 26: Line 27: NETWORK_NAME = 'test-network' Line 28: VLAN_ID = '27' Line 29: BONDING_NAME = 'bond0' Line 30: None of those tests, needs to modprobe any dummy or either checking the existence of a dummy device. Perhaps it may be useful for testAddDeleteNetwork. Line 31: Line 32: @expandPermutations Line 33: class NetworkTestShould(TestCaseBase): Line 34: Line 34: Line 35: def setUp(self): Line 36: self.vdsm_net = VdsProxy() Line 37: Line 38: def testAddDeleteANetwork(self): I will add that. Line 39: status, msg = self.vdsm_net.addNetwork(NETWORK_NAME) Line 40: self.assertEqual(status, SUCCESS, msg) Line 41: self.assertTrue(self.vdsm_net.networkExists(NETWORK_NAME)) Line 42: Line 36: self.vdsm_net = VdsProxy() Line 37: Line 38: def testAddDeleteANetwork(self): Line 39: status, msg = self.vdsm_net.addNetwork(NETWORK_NAME) Line 40: self.assertEqual(status, SUCCESS, msg) I like the idea of using a context manager (it a resource handling case), I will explore that. Line 41: self.assertTrue(self.vdsm_net.networkExists(NETWORK_NAME)) Line 42: Line 43: status, msg = self.vdsm_net.delNetwork(NETWORK_NAME) Line 44: self.assertEqual(status, SUCCESS, msg) .................................................... File tests/functional/utils.py Line 16: # Line 17: # Refer to the README and COPYING files for full details of the license Line 18: # Line 19: from vdsm import netinfo Line 20: from vdsm import vdscli This module is useful for making tests more readable, without that I need to: 1. Specify empty strings '' for parameters which I don't care in a specific test. 2. Unpack the status code and message from the result 3. Ask a different module info about network. I try to avoid boilerplate when it is possible and cheap. When I write unit tests I try to follow the Arrange Act Assert rule [1]. I'm not attached to this implementation I can change it and bring the code inside the test module, if people are not enough convinced. [1] http://c2.com/cgi/wiki?ArrangeActAssert Line 21: Line 22: Line 23: SUCCESS = 0 Line 24: Line 16: # Line 17: # Refer to the README and COPYING files for full details of the license Line 18: # Line 19: from vdsm import netinfo Line 20: from vdsm import vdscli For now it's networking, cause I felt the need of creating an external module, but nothing prevents to add here, shared code of different functional tests, or move this code in a different module. Usually I strive to have only tests in test modules and utilities somewhere else. Line 21: Line 22: Line 23: SUCCESS = 0 Line 24: Line 24: Line 25: Line 26: class VdsProxy(object): Line 27: Line 28: def __init__(self): Ok. Line 29: self.vdscli = vdscli.connect(useSSL=False) Line 30: Line 31: def _get_net_args(self, vlan, bond, nics, opts): Line 32: if vlan is None: -- To view, visit http://gerrit.ovirt.org/14840 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic3be71db9dc0b92c443b87e22fe06f920055c4d3 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Giuseppe Vallarelli <[email protected]> Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Giuseppe Vallarelli <[email protected]> Gerrit-Reviewer: Livnat Peer <[email protected]> Gerrit-Reviewer: Mark Wu <[email protected]> Gerrit-Reviewer: Zhou Zheng Sheng <[email protected]> Gerrit-Reviewer: oVirt Jenkins CI Server _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
