Dan Kenigsberg has posted comments on this change. Change subject: adding statistics unit tests ......................................................................
Patch Set 1: (4 comments) http://gerrit.ovirt.org/#/c/35703/1/tests/functional/networkTests.py File tests/functional/networkTests.py: Line 52: import caps Line 53: from network import errors Line 54: from network import tc Line 55: Line 56: from subprocess import Popen, PIPE imports from the stdlib should show up up above Line 57: import time Line 58: Line 59: Line 60: NETWORK_NAME = 'test-network' Line 383: hasLoopback = False Line 384: status, msg, hostStats = self.vdsm_net.getVdsStats() Line 385: self.assertEqual(status, SUCCESS, msg) Line 386: self.assertIn('network', hostStats) Line 387: if any("lo" in s for s in hostStats['network']): this is complex enough to merit its own test. testSetupNetworksAddBondWithManyVlans is not really related, and is too complex by its own rights. In a separate test, you should simple raise SkipTest if the host misses "lo". Line 388: hasLoopback = True Line 389: prevLoopRx = self.loopRx Line 390: #running some traffic to the interface Line 391: Popen(PING_COMMAND + " -c 1 "+LOOPBACK_IP,shell=True, stdout=PIPE, stderr=PIPE).wait() Line 387: if any("lo" in s for s in hostStats['network']): Line 388: hasLoopback = True Line 389: prevLoopRx = self.loopRx Line 390: #running some traffic to the interface Line 391: Popen(PING_COMMAND + " -c 1 "+LOOPBACK_IP,shell=True, stdout=PIPE, stderr=PIPE).wait() please use execCmd, and don't overstep the 80 char line length. Line 392: # give the stats a chance to update Line 393: time.sleep(2) Line 394: status, msg, hostStats = self.vdsm_net.getVdsStats() Line 395: self.assertIn('tx', hostStats) Line 396: self.assertIn('rx', hostStats) Line 397: if hasLoopback: Line 398: #gather the rx stats and check it's incrementing Line 399: self.loopRx = hostStats['network']['lo']['rx']; Line 400: self.assertLess(prevLoopRx,self.loopRx) come to this of this, loopback device is problematic, since other processes are likely to increment its stats unrelated to this test. It's safer to call veth.create() and measure if the reported stats are pricesly the expected ones. Line 401: self.prevLoopRx = self.loopRx Line 402: for tag in range(VLAN_COUNT): Line 403: self.assertIn( Line 404: BONDING_NAME + '.' + str(tag), hostStats['network']) -- To view, visit http://gerrit.ovirt.org/35703 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9bfd09b33a2bf6ca6800fe65b02e104a2ddf4a48 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amir Shladovsky <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
