Nir Soffer has uploaded a new change for review.
Change subject: tests: Add test for terminating a terminated process ......................................................................
tests: Add test for terminating a terminated process
In this case we should find that the process has terminated and waited, and do not invoke kill().
Change-Id: Ic0f06c28857664cc49beb938f33ac3c9d07ca3b6 Signed-off-by: Nir Soffer nsoffer@redhat.com --- M tests/utilsTests.py 1 file changed, 12 insertions(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/28/65328/1
diff --git a/tests/utilsTests.py b/tests/utilsTests.py index 27a84e7..8c7ae61 100644 --- a/tests/utilsTests.py +++ b/tests/utilsTests.py @@ -111,6 +111,18 @@ pass self.assertEqual(self.proc.returncode, -signal.SIGTERM)
+ def test_process_terminated(self): + self.proc.terminate() + self.proc.wait() + + def fail(): + raise RuntimeError("Attempt to kill a terminated process") + + self.proc.kill = fail + with utils.terminating(self.proc): + pass + self.assertEqual(self.proc.returncode, -signal.SIGTERM) + def test_kill_failure(self): class FakeKillError(Exception): pass
gerrit-hooks has posted comments on this change.
Change subject: tests: Add test for terminating a terminated process ......................................................................
Patch Set 1:
* Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0'])
Nir Soffer has posted comments on this change.
Change subject: tests: Add test for terminating a terminated process ......................................................................
Patch Set 1: Verified+1
Verified by the tests.
gerrit-hooks has posted comments on this change.
Change subject: tests: Add test for terminating a terminated process ......................................................................
Patch Set 2:
* Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0'])
gerrit-hooks has posted comments on this change.
Change subject: tests: Add test for terminating a terminated process ......................................................................
Patch Set 3:
* Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0'])
gerrit-hooks has posted comments on this change.
Change subject: tests: Add test for terminating a terminated process ......................................................................
Patch Set 4:
* Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0'])
Nir Soffer has posted comments on this change.
Change subject: tests: Add test for terminating a terminated process ......................................................................
Patch Set 4: Continuous-Integration+1
Verfied on travis: https://travis-ci.org/nirs/vdsm/builds/166549485
Francesco Romani has posted comments on this change.
Change subject: tests: Add test for terminating a terminated process ......................................................................
Patch Set 4: Code-Review+1
gerrit-hooks has posted comments on this change.
Change subject: tests: Add test for terminating a terminated process ......................................................................
Patch Set 5:
* Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0'])
gerrit-hooks has posted comments on this change.
Change subject: tests: Add test for terminating a terminated process ......................................................................
Patch Set 6:
* Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0'])
Piotr Kliczewski has posted comments on this change.
Change subject: tests: Add test for terminating a terminated process ......................................................................
Patch Set 6: Code-Review-1
(1 comment)
https://gerrit.ovirt.org/#/c/65328/6/tests/utilsTests.py File tests/utilsTests.py:
PS6, Line 117: def fail(): : raise RuntimeError("Attempt to kill a terminated process") I see that copy and paste are our friends :)
Nir Soffer has posted comments on this change.
Change subject: tests: Add test for terminating a terminated process ......................................................................
Patch Set 6:
(1 comment)
https://gerrit.ovirt.org/#/c/65328/6/tests/utilsTests.py File tests/utilsTests.py:
Line 114: self.proc.terminate() Line 115: self.proc.wait() Line 116: Line 117: def fail(): Line 118: raise RuntimeError("Attempt to kill a terminated process")
I see that copy and paste are our friends :)
Did you notice that each exception has different text?
Please suggest how to improve this while keeping the code clear. Line 119: Line 120: self.proc.kill = fail Line 121: with utils.terminating(self.proc): Line 122: pass
gerrit-hooks has posted comments on this change.
Change subject: tests: Add test for terminating a terminated process ......................................................................
Patch Set 7:
* Update Tracker::IGNORE, not relevant for branch: master * Check Bug-Url::IGNORE, not relevant for branch: master * Check Public Bug::WARN, no public bug url found * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0'])
vdsm-patches@lists.fedorahosted.org