Nir Soffer has posted comments on this change.

Change subject: terminating tests - verify process death
......................................................................


Patch Set 5:

(3 comments)

https://gerrit.ovirt.org/#/c/52362/5/tests/utilsTests.py
File tests/utilsTests.py:

Line 123:             try:
Line 124:                 with utils.terminating(proc):
Line 125:                     self.assertTrue(os.path.exists(proc_path))
Line 126:             except FakeKillError:
Line 127:                 exception_raise = True
> this fails :) terminating doesn't raise the kill exception (or poll excepti
How it does not raise? we are using try-finally, which run stuff on finally, 
and letting the exception bubble up to the caller.

 >>> try:
 ...     raise RuntimeError("foo")
 ... finally:
 ...     print "cleaning up"
 ... 
 cleaning up
 Traceback (most recent call last):
   File "<stdin>", line 2, in <module>
 RuntimeError: foo
Line 128: 
Line 129:             if not exception_raise:
Line 130:                 self.fail("Exception was not raised")
Line 131:             self.assertTrue(proc.pid in self.reaped)


Line 126:             except FakeKillError:
Line 127:                 exception_raise = True
Line 128: 
Line 129:             if not exception_raise:
Line 130:                 self.fail("Exception was not raised")
Why you are avoiding self.assertRaises(FakeKillError)?
Line 131:             self.assertTrue(proc.pid in self.reaped)
Line 132:             real_kill()
Line 133:             self.assertFalse(wait_for_removal(proc_path, timeout=0.1))
Line 134: 


Line 129:             if not exception_raise:
Line 130:                 self.fail("Exception was not raised")
Line 131:             self.assertTrue(proc.pid in self.reaped)
Line 132:             real_kill()
Line 133:             self.assertFalse(wait_for_removal(proc_path, timeout=0.1))
We don't need this wait, there is no chance that kill will fail on sleep.
Line 134: 
Line 135:     def test_terminating_with_infected_kill(self):
Line 136:         with MonkeyPatchScope([(zombiereaper,
Line 137:                                 'autoReapPID',


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib1600a298b3faaffe3cc72b4eb62201370bf03c7
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybron...@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
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to