Edward Haas has posted comments on this change.

Change subject: net: edit bond detached from bridge but still attached to a vlan
......................................................................


Patch Set 5:

(1 comment)

https://gerrit.ovirt.org/#/c/63723/5/tests/network/func_ifcfg_test.py
File tests/network/func_ifcfg_test.py:

PS5, Line 38:     def assertIfcfgParamSet(self, iface, param):
            :         self.assertTrue(_ifcfg_param_set(iface, param))
            : 
            :     def assertIfcfgParamNotSet(self, iface, param):
            :         self.assertFalse(_ifcfg_param_set(iface, param))
With the new functional tests, we tried to separate between the scenarios, 
setup mechanics and assertions logic.
I prefer to see in the func_* modules, only the scenarios.

How about this suggestion:
- Move these assertions to its own class and inherit from NetFuncTestCase. I 
guess it fits under netfunctestlib or its own module (I personally prefer it in 
its own module, as the existing one is using only the caps to check against).
- Move the scenarios under the bond test module and for 'legacy', add these 
additional assertions.

The ideal test IMO would have been to examine this through the end result and 
not through intermediate states (ifcfg file content). What I mean is, that it 
will be more accurate to perform the actual steps and check that the 
caps/kernel is wrong, without 'understanding' the low level details (ifcfg file 
content). If for example, after setup we can restart vdsmd or the network 
service, and check then, it will be better.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I460cb08cf436b932e7d9592557a03d7b6fc36a0f
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Horáček <phora...@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
Gerrit-Reviewer: Edward Haas <edwa...@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček <phora...@redhat.com>
Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org>
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Reply via email to