Mark Wu has posted comments on this change.
Change subject: tests: Adding functional tests for networking
......................................................................
Patch Set 12: (5 inline comments)
....................................................
File tests/functional/networkTests.py
Line 70: opts={'bridged':
bridged})
Line 71: self.assertEqual(status, neterrors.ERR_BAD_BONDING, msg)
Line 72:
Line 73: def testFailWithInvalidBridgeName(self):
Line 74: invalid_bridge_names = ('a' * 16, 'a b', 'a\tb', 'a.b', 'a:b')
I am fine to start with the test of parameters validation, but most of them
have done in the unit test. So I think we needn't cover more test cases done in
the unit test.
Line 75: for bridge_name in invalid_bridge_names:
Line 76: status, msg = self.vdsm_net.addNetwork(bridge_name)
Line 77: self.assertEqual(status, neterrors.ERR_BAD_BRIDGE, msg)
Line 78:
....................................................
File tests/functional/utils.py
Line 42: )
Line 43:
Line 44:
Line 45: @contextmanager
Line 46: def dummyIf():
it's better allow creating multiple dummies on one call. You add add a prameter
numDummies for it. It's useful for the test of bonding.
Line 47: """
Line 48: Manages a dummy interface resource, which is created with
Line 49: a pseudo-random name (e.g. dummy_85). A fixed number of
Line 50: attempts are done to create the required dummy interface.
Line 54: dummy_name = None
Line 55: attempts = 0
Line 56: try:
Line 57: while rc != SUCCESS and attempts < 100:
Line 58: dummy_name = 'dummy_%s' % randrange(0, 100)
Can I suggest to use 'dummy%s'? it comply with nic naming convention. And it
also make the fake nic pattern defined in config.py.in can cover the dummy
interfaces created by loading dummy module with the given argument numdummies
Line 59: ip_add_dummy = [ip.cmd, 'link', 'add', dummy_name,
Line 60: 'type', 'dummy']
Line 61: rc, out, err = utils.execCmd(ip_add_dummy, sudo=True)
Line 62: attempts += 1
Line 77: def cleanupNet(func):
Line 78: """
Line 79: Restored a previously persisted network config
Line 80: in case of a test failure, traceback is kept.
Line 81: """
Actually, I don't understand why it needs to restore network configuration.
Each test case is independently because we run test against dummies randomly
generated and the fake nics arel also removed after each test case. So why do
we restore it?
Line 82:
Line 83: @wraps(func)
Line 84: def wrapper(*args, **kwargs):
Line 85: try:
....................................................
File tests/testValidation.py
Line 104: def RequireDummyMod(f):
Line 105: @wraps(f)
Line 106: def wrapper(*args, **kwargs):
Line 107: cmd_modprobe = [modprobe.cmd, "dummy"]
Line 108: if os.path.exists('/sys/modules/dummy'):
it's confusing for me. If I understanding correct, shouldn't be
if not os.path.exists('/sys/module/dummy'):
At least, on fedora18, it's 'module', not 'modules'
Line 109: rc, out, err = utils.execCmd(cmd_modprobe, sudo=True)
Line 110: return f(*args, **kwargs)
Line 111: return wrapper
Line 112:
--
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: 12
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
