Antoni Segura Puimedon has posted comments on this change.
Change subject: tests: setupNetworks add one or more vlans.
......................................................................
Patch Set 1: I would prefer that you didn't submit this
(4 inline comments)
Thanks for the tests Giuseppe!
....................................................
File tests/functional/networkTests.py
Line 347: @RequireDummyMod
Line 348: @ValidateRunningAsRoot
Line 349: def testSetupNetworksAddManyVlans(self, bridged):
Line 350: VLAN_COUNT = 5
Line 351: NET_VLANS = [(NETWORK_NAME + str(id), str(id))
since id is a python builtin, could we use 'index' here to prevent shadowing?
Line 352: for id in xrange(VLAN_COUNT)]
Line 353:
Line 354: with dummyIf(1) as nics:
Line 355: nic = nics[0]
Line 348: @ValidateRunningAsRoot
Line 349: def testSetupNetworksAddManyVlans(self, bridged):
Line 350: VLAN_COUNT = 5
Line 351: NET_VLANS = [(NETWORK_NAME + str(id), str(id))
Line 352: for id in xrange(VLAN_COUNT)]
xrange is not necessary, range should be preferred for forwards compatibility
(and also because list comprehensions are already special cased and don't need
the boost xrange used to give).
Line 353:
Line 354: with dummyIf(1) as nics:
Line 355: nic = nics[0]
Line 356: networks = dict((vlan_net,
Line 351: NET_VLANS = [(NETWORK_NAME + str(id), str(id))
Line 352: for id in xrange(VLAN_COUNT)]
Line 353:
Line 354: with dummyIf(1) as nics:
Line 355: nic = nics[0]
That's a minor thing, but I much prefer to use unpacking since it detects wrong
sizing of the list.
nic, = nics
Line 356: networks = dict((vlan_net,
Line 357: {'vlan': str(tag), 'nic': nic,
Line 358: 'bridged': bridged})
Line 359: for vlan_net, tag in NET_VLANS)
Line 368: self.assertTrue(self.vdsm_net.vlanExists(nic +
'.' + tag))
Line 369:
Line 370: with self.vdsm_net.pinger():
Line 371: for vlan_net, tag in NET_VLANS:
Line 372: status, msg = self.vdsm_net.setupNetworks(
I would probably change this into a single setupNetworks that removes all the
nets, just like the one in line 362 adds all the nets.
Line 373: {vlan_net: {'remove': True}}, {}, {})
Line 374: self.assertEqual(status, SUCCESS, msg)
Line 375:
self.assertFalse(self.vdsm_net.networkExists(vlan_net,
Line 376:
bridged))
--
To view, visit http://gerrit.ovirt.org/17432
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I62fa6de1faa7d4e886c45361a001b0009fd92502
Gerrit-PatchSet: 1
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: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
