Dan Kenigsberg has posted comments on this change. Change subject: networkTests: Extend a test to also consider DHCPv6 ......................................................................
Patch Set 37: Code-Review-1 (4 comments) http://gerrit.ovirt.org/#/c/30532/37/tests/functional/dhcp.py File tests/functional/dhcp.py: Line 45: def __init__(self): Line 46: self.proc = None Line 47: Line 48: def start(self, interface, dhcpRangeFrom, dhcpRangeTo, dhcpv6, Line 49: dhcp6RangeFrom, dhcp6RangeTo, router=None): Passing dhcpv6 flag is a bit cumbersome. checking for dhcp6RangeFrom is None or dhcp6RangeTo is None would be cleaner. Line 50: # -p 0 don't act as a DNS server Line 51: # --dhcp-option=3,<router> advertise a specific gateway (or None) Line 52: # --dhcp-option=6 don't reply with any DNS servers Line 53: # -d don't daemonize and log to stderr Line 58: '--dhcp-option=3' + (',{0}'.format(router) if router else ''), Line 59: '--dhcp-option=6', Line 60: '-i', interface, '-I', 'lo', '-d', Line 61: '--bind-interfaces'] Line 62: # EL6's dnsmasq is not able to act as a DHCPv6 server in this level, we don't care about el6. if Dnsmasq was not asked to serve ipv6, it simply does not. So the comment can be dropped. Line 63: + (['--dhcp-range={0},{1},2m'.format(dhcp6RangeFrom, dhcp6RangeTo)] Line 64: if dhcpv6 else []), sync=False) Line 65: sleep(_START_CHECK_TIMEOUT) Line 66: if self.proc.returncode: http://gerrit.ovirt.org/#/c/30532/37/tests/functional/networkTests.py File tests/functional/networkTests.py: Line 422: def assertMtu(self, mtu, *elems): Line 423: for elem in elems: Line 424: self.assertEquals(int(mtu), int(self.vdsm_net.getMtu(elem))) Line 425: Line 426: def assertAddrInRange(self, addrs, addr_to_int=_ipv4_to_int, this funciton would have been more readable to me if it had a "family" arg instead of addr_to_int. Line 427: range_from=DHCP_RANGE_FROM, range_to=DHCP_RANGE_TO): Line 428: passes = (addr_to_int(range_from) <= addr_to_int(addr) <= Line 429: addr_to_int(range_to) for addr in addrs) Line 430: self.assertTrue(any(passes), 'no address {0} in expected range'.format( Line 427: range_from=DHCP_RANGE_FROM, range_to=DHCP_RANGE_TO): Line 428: passes = (addr_to_int(range_from) <= addr_to_int(addr) <= Line 429: addr_to_int(range_to) for addr in addrs) Line 430: self.assertTrue(any(passes), 'no address {0} in expected range'.format( Line 431: addrs)) why "any"? I'd expect all() addresses would be in range. Line 432: Line 433: def testLegacyBonds(self): Line 434: if not (caps.getos() in (caps.OSName.RHEVH, caps.OSName.RHEL) Line 435: and caps.osversion()['version'].startswith('6')): -- To view, visit http://gerrit.ovirt.org/30532 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic5d821edd54681a7a8c1013a90af61ae835baa39 Gerrit-PatchSet: 37 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ondřej Svoboda <osvob...@redhat.com> Gerrit-Reviewer: Antoni Segura Puimedon <asegu...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Ido Barkan <ibar...@redhat.com> Gerrit-Reviewer: Ondřej Svoboda <osvob...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
