Dan Kenigsberg has posted comments on this change. Change subject: tests: Adding functional tests for networking ......................................................................
Patch Set 19: Fails; I would prefer that you didn't submit this (5 inline comments) .................................................... File tests/functional/Makefile.am Line 21: vdsmfunctestsdir = ${vdsmtestsdir}/functional Line 22: Line 23: dist_vdsmfunctests_PYTHON = \ Line 24: momTests.py \ Line 25: networkTests.py \ utils.py is missing here. Please verify your tests from the /usr/share/vdsm/tests directory, too. Line 26: sosPluginTests.py \ Line 27: xmlrpcTests.py \ Line 28: $(NULL) .................................................... File tests/functional/networkTests.py Line 36: vdsm.save_config() Line 37: Line 38: Line 39: @expandPermutations Line 40: class NetworkTestShould(TestCaseBase): "NetworkTestShould" is a surprising name. Classes should usually be named as a noun. Could you think of a more conventional name? Line 41: Line 42: def setUp(self): Line 43: self.vdsm_net = VdsProxy() Line 44: Line 67: status, msg = self.vdsm_net.addNetwork(NETWORK_NAME, Line 68: bond=bond_name, Line 69: nics=['whatever'], Line 70: opts={'bridged': bridged}) Line 71: self.assertEqual(status, neterrors.ERR_BAD_BONDING, msg) on el6 I get FAIL: testFailWithInvalidBondingName(bridged=False) (networkTests.NetworkTestShould) ---------------------------------------------------------------------- Traceback (most recent call last): File "/usr/share/vdsm/tests/testrunner.py", line 68, in wrapper return f(self, *args) File "/usr/share/vdsm/tests/functional/networkTests.py", line 71, in testFailWithInvalidBondingName self.assertEqual(status, neterrors.ERR_BAD_BONDING, msg) AssertionError: unknown nic: whatever you'd better include the "status" in the msg (here and elsewhere) to help debugging such occasions. Line 72: Line 73: def testFailWithInvalidBridgeName(self): Line 74: invalid_bridge_names = ('a' * 16, 'a b', 'a\tb', 'a.b', 'a:b') Line 75: for bridge_name in invalid_bridge_names: .................................................... File tests/functional/utils.py Line 47: Creates a dummy interface, in a fixed number of attempts (100). Line 48: The dummy interface created has a pseudo-random name (e.g. dummy_85 Line 49: in the format dummy_Number). Assumes root privileges. Line 50: """ Line 51: I find the randomness hard to digest. But if we have to do it, let's not re-invent the wheel: for i in random.sample(xrange(100), 100): name = dummy_name = 'dummy_%s' % i if not ip_add_dummy(): return name else: raise SkipTest Or even better, get a random string on module setup, and use it throughout your tests: DUMMY_PREFIX = random.sample(string.ascii_letters, 5) This would make it easier to find out which unittets run owns which dummy. Line 52: rc = -1 Line 53: attempts = 0 Line 54: dummy_name = None Line 55: .................................................... File tests/testValidation.py Line 107: ValidateRunningAsRoot decoration. Line 108: """ Line 109: @wraps(f) Line 110: def wrapper(*args, **kwargs): Line 111: cmd_modprobe = [modprobe.cmd, "dummy"] The assignment can safely go under the condition. Line 112: if not os.path.exists('/sys/module/dummy'): Line 113: rc, out, err = utils.execCmd(cmd_modprobe, sudo=True) Line 114: return f(*args, **kwargs) Line 115: return wrapper -- 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: 19 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Giuseppe Vallarelli <gvall...@redhat.com> Gerrit-Reviewer: Antoni Segura Puimedon <asegu...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Giuseppe Vallarelli <gvall...@redhat.com> Gerrit-Reviewer: Livnat Peer <lp...@redhat.com> Gerrit-Reviewer: Mark Wu <wu...@linux.vnet.ibm.com> Gerrit-Reviewer: Zhou Zheng Sheng <zhshz...@linux.vnet.ibm.com> Gerrit-Reviewer: oVirt Jenkins CI Server _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches