Petr Horáček has posted comments on this change. Change subject: net: native ovs [5]: rollback trigger ......................................................................
Patch Set 6: (5 comments) https://gerrit.ovirt.org/#/c/55312/6/tests/network/ovs_test.py File tests/network/ovs_test.py: PS6, Line 162: netconfpersistence > I guess RunningConfig alone will be nicer. Done PS6, Line 168: fake_config = netconfpersistence.RunningConfig() : fake_config.networks = {} : fake_config.save() > Please extract to a helper method Done Line 171: Line 172: with ovs_switch.rollback_trigger(in_rollback=False) as running_config: Line 173: running_config.setNetwork('net1', {'nic': 'eth1'}) Line 174: Line 175: self.assertEquals(running_config.networks['net1']['nic'], 'eth1') > Why this intermediate test is needed if the end result is the assert from t it tests if it is also save()d in the system. Line 176: self.assertEquals( Line 177: netconfpersistence.RunningConfig().networks['net1']['nic'], 'eth1') Line 178: Line 179: def test_failed_setup(self): PS6, Line 189: raise Exception('This is not a test.') > ? https://upload.wikimedia.org/wikipedia/en/b/b9/MagrittePipe.jpg we need to trigger rollback somehow. PS6, Line 194: else: : raise > Why is this needed? AFAIK assert raises doesn't allow us to assert contents of an exception, so we need to do it ourselves. if it failed with another error than RollbackIncomplete, test failed. -- To view, visit https://gerrit.ovirt.org/55312 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5bceae0aada964c47aacfcfdbe93ca19eb8f3d15 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Edward Haas <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Ondřej Svoboda <[email protected]> Gerrit-Reviewer: Petr Horáček <[email protected]> Gerrit-Reviewer: gerrit-hooks <[email protected]> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
