Dan Kenigsberg has posted comments on this change. Change subject: ipwrapperTests: prevent infinite stuck ......................................................................
Patch Set 2: Code-Review-1 (3 comments) http://gerrit.ovirt.org/#/c/35901/2/tests/ipwrapperTests.py File tests/ipwrapperTests.py: Line 158: tcTests._checkDependencies() Line 159: mon = Monitor() Line 160: mon.start() Line 161: Line 162: def _timeout(mon): Why do you need to pass mon as an arg? It is know and bounded in this context. Line 163: mon.stop() Line 164: raise MonitorError("Test timeout") Line 165: timer = Timer(3, _timeout, args=[mon]) Line 166: timer.start() Line 160: mon.start() Line 161: Line 162: def _timeout(mon): Line 163: mon.stop() Line 164: raise MonitorError("Test timeout") Who needs this raise? Does anything handle it? A newline after function definition is advisable. Line 165: timer = Timer(3, _timeout, args=[mon]) Line 166: timer.start() Line 167: Line 168: iterator = iter(mon) Line 180: with self.assertRaises(StopIteration): Line 181: while True: Line 182: iterator.next() Line 183: Line 184: timer.cancel() hmmm, for politeness, this should happen in a "finally" clause that is begun after timer.start() Line 185: Line 186: Line 187: class TestLinks(TestCaseBase): Line 188: _bridge = tcTests._Bridge() -- To view, visit http://gerrit.ovirt.org/35901 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie71eb5b33cabf5402f00815aa603bbc384159d56 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček <phora...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
