Dan Kenigsberg has posted comments on this change.

Change subject: tests: netlink: update expected events
......................................................................


Patch Set 2: Code-Review-1

(3 comments)

https://gerrit.ovirt.org/#/c/40814/2/tests/netlinkTests.py
File tests/netlinkTests.py:

Line 92:                 iterator.next()
Line 93: 
Line 94:     @ValidateRunningAsRoot
Line 95:     def test_events_keys(self):
Line 96:         # TODO dependent on 
/proc/sys/net/ipv6/conf/default/disable_ipv6
stale comment
Line 97:         def _expected_events(nic, address, cidr):
Line 98:             with open('/proc/sys/net/ipv6/conf/default/disable_ipv6') 
as f:
Line 99:                 ipv6_enabled = f.read()
Line 100:             ipv6_enabled = int(ipv6_enabled[0])


Line 96:         # TODO dependent on 
/proc/sys/net/ipv6/conf/default/disable_ipv6
Line 97:         def _expected_events(nic, address, cidr):
Line 98:             with open('/proc/sys/net/ipv6/conf/default/disable_ipv6') 
as f:
Line 99:                 ipv6_enabled = f.read()
Line 100:             ipv6_enabled = int(ipv6_enabled[0])
I think it would be nicer to define an is_disabled_ipv6() function in 
lib/vdsm/sysctl.py
Line 101: 
Line 102:             if ipv6_enabled:
Line 103:                 return deque([
Line 104:                     {'event': 'new_link', 'name': nic},


Line 99:                 ipv6_enabled = f.read()
Line 100:             ipv6_enabled = int(ipv6_enabled[0])
Line 101: 
Line 102:             if ipv6_enabled:
Line 103:                 return deque([
this duplicates some of the expected data.

I think it would be nicer to have

events = [some events]
if ipv6:
  events.append(ipv6 event)
events.extend([more common events])


etc.
Line 104:                     {'event': 'new_link', 'name': nic},
Line 105:                     {'event': 'new_addr', 'address': address + '/' + 
cidr},
Line 106:                     {'event': 'new_link', 'name': nic},
Line 107:                     {'event': 'new_addr', 'family': 'inet6'},


-- 
To view, visit https://gerrit.ovirt.org/40814
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I81decdfc2f8dfe1040409f06ad5f7c0cf4f7dd17
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Horáček <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to