Nir Soffer has posted comments on this change. Change subject: tests: Add tests for waiting on a zombie process ......................................................................
Patch Set 6: (1 comment) https://gerrit.ovirt.org/#/c/65327/6/tests/utilsTests.py File tests/utilsTests.py: Line 102: self.proc.terminate() Line 103: wait_for_zombie(self.proc, 1) Line 104: Line 105: def fail(): Line 106: raise RuntimeError("Attempt to kill a zombie process") > In patch #65328 we use RuntimeError as well. We mentioned in commit message Yes, but with different error text - this should make debugging the tests easier in case of errors. We can do something like this: def fail(exception, msg): raise exception(msg) And then n each test we can do: self.proc.kill = partial(fail, RuntimeError, "Attempt to kill a zombie process") But I find this much harder to understand compared to: def fail(): raise RuntimeError("Attempt to kill a zombie process") self.proc.kill = fail The best would be to use lambda: self.proc.kill = lambda: raise RuntimeError("Attempt to kill a zombie process") But lambda does not support this syntax. Line 107: Line 108: self.proc.kill = fail Line 109: with utils.terminating(self.proc): Line 110: pass -- To view, visit https://gerrit.ovirt.org/65327 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I238d77213153b1fa98bb3f52b33a6fd78bb57bca Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczew...@gmail.com> Gerrit-Reviewer: Yaniv Bronhaim <ybron...@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
