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 <[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
