Nir Soffer has uploaded a new change for review.
Change subject: tests: Refine TerminationTests names ......................................................................
tests: Refine TerminationTests names
- Al the test are testing termination, there is no point in repeating the class name in the test names. - Use the tests name to describe the situation tested; terminating a running process, or kill failure. - Rename fake_kill to fail; this name can be used later for other failure tests.
Change-Id: Id085d7bbc65fe985454627e0958eb03d0e413b4d Signed-off-by: Nir Soffer nsoffer@redhat.com --- M tests/utilsTests.py 1 file changed, 4 insertions(+), 4 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/26/65326/1
diff --git a/tests/utilsTests.py b/tests/utilsTests.py index e3a184c..eabd459 100644 --- a/tests/utilsTests.py +++ b/tests/utilsTests.py @@ -93,19 +93,19 @@ self.kill_proc() self.proc.wait()
- def test_terminating(self): + def test_process_running(self): with utils.terminating(self.proc): self.assertIsNone(self.proc.poll()) self.assertEqual(self.proc.returncode, -signal.SIGKILL)
- def test_terminating_with_kill_exception(self): + def test_kill_failure(self): class FakeKillError(Exception): pass
- def fake_kill(): + def fail(): raise FakeKillError("fake kill exception")
- self.proc.kill = fake_kill + self.proc.kill = fail with self.assertRaises(utils.TerminatingFailure) as e: with utils.terminating(self.proc): self.assertIsNone(self.proc.poll())
gerrit-hooks has posted comments on this change.
Change subject: tests: Refine TerminationTests names ......................................................................
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: Refine TerminationTests names ......................................................................
Patch Set 1: Verified+1
Verified by the tests.
gerrit-hooks has posted comments on this change.
Change subject: tests: Refine TerminationTests names ......................................................................
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: Refine TerminationTests names ......................................................................
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: Refine TerminationTests names ......................................................................
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: Refine TerminationTests names ......................................................................
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: Refine TerminationTests names ......................................................................
Patch Set 4: Code-Review+1
gerrit-hooks has posted comments on this change.
Change subject: tests: Refine TerminationTests names ......................................................................
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'])
Piotr Kliczewski has posted comments on this change.
Change subject: tests: Refine TerminationTests names ......................................................................
Patch Set 5:
(3 comments)
https://gerrit.ovirt.org/#/c/65326/5//COMMIT_MSG Commit Message:
PS5, Line 9: Al For all the tests that test termination there is no need to repeat...
PS5, Line 11: tests name test names
PS5, Line 13: for other : failure tests what do you mean? how do you want to do it? (my questions based on the code in this patch)
Nir Soffer has posted comments on this change.
Change subject: tests: Refine TerminationTests names ......................................................................
Patch Set 5:
(3 comments)
https://gerrit.ovirt.org/#/c/65326/5//COMMIT_MSG Commit Message:
Line 5: CommitDate: 2016-10-13 03:07:01 +0300 Line 6: Line 7: tests: Refine TerminationTests names Line 8: Line 9: - Al the test are testing termination, there is no point in repeating
For all the tests that test termination there is no need to repeat...
Done Line 10: the class name in the test names. Line 11: - Use the tests name to describe the situation tested; terminating a Line 12: running process, or kill failure. Line 13: - Rename fake_kill to fail; this name can be used later for other
Line 7: tests: Refine TerminationTests names Line 8: Line 9: - Al the test are testing termination, there is no point in repeating Line 10: the class name in the test names. Line 11: - Use the tests name to describe the situation tested; terminating a
test names
Done Line 12: running process, or kill failure. Line 13: - Rename fake_kill to fail; this name can be used later for other Line 14: failure tests. Line 15:
Line 10: the class name in the test names. Line 11: - Use the tests name to describe the situation tested; terminating a Line 12: running process, or kill failure. Line 13: - Rename fake_kill to fail; this name can be used later for other Line 14: failure tests.
what do you mean? how do you want to do it? (my questions based on the code
I was planning to move this to a module function, and reuse the same function all the tests.
When I added the next tests, I discovered that we cannot reuse it since it should raise different exception and different text, so the best way is to have a nested function in the test.
Hopefully more clear in the next version. Line 15: Line 16: Change-Id: Id085d7bbc65fe985454627e0958eb03d0e413b4d
gerrit-hooks has posted comments on this change.
Change subject: tests: Refine TerminationTests names ......................................................................
Patch Set 6:
* 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'])
Nir Soffer has posted comments on this change.
Change subject: tests: Refine TerminationTests names ......................................................................
Patch Set 6: Verified+1
Piotr Kliczewski has posted comments on this change.
Change subject: tests: Refine TerminationTests names ......................................................................
Patch Set 6: Code-Review+1
Francesco Romani has posted comments on this change.
Change subject: tests: Refine TerminationTests names ......................................................................
Patch Set 6: Code-Review+1
Yaniv Bronhaim has posted comments on this change.
Change subject: tests: Refine TerminationTests names ......................................................................
Patch Set 6:
(4 comments)
https://gerrit.ovirt.org/#/c/65326/6//COMMIT_MSG Commit Message:
Line 5: CommitDate: 2016-10-13 14:32:59 +0300 Line 6: Line 7: tests: Refine TerminationTests names Line 8: Line 9: - Al the test are testing termination, there is no need to repeat s/Al/All Line 10: the class name in the test names. Line 11: - Use the tests names to describe the situation tested; terminating a Line 12: running process, or kill failure. Line 13: - Rename fake_kill to fail, describing what this function is doing.
Line 7: tests: Refine TerminationTests names Line 8: Line 9: - Al the test are testing termination, there is no need to repeat Line 10: the class name in the test names. Line 11: - Use the tests names to describe the situation tested; terminating a s/names/name Line 12: running process, or kill failure. Line 13: - Rename fake_kill to fail, describing what this function is doing. Line 14: Line 15: Change-Id: Id085d7bbc65fe985454627e0958eb03d0e413b4d
Line 9: - Al the test are testing termination, there is no need to repeat Line 10: the class name in the test names. Line 11: - Use the tests names to describe the situation tested; terminating a Line 12: running process, or kill failure. Line 13: - Rename fake_kill to fail, describing what this function is doing. Rameing ... describes "better" what this function does Line 14: Line 15: Change-Id: Id085d7bbc65fe985454627e0958eb03d0e413b4d
https://gerrit.ovirt.org/#/c/65326/6/tests/utilsTests.py File tests/utilsTests.py:
Line 101 Line 102 Line 103 Line 104 Line 105 you asked for this name iirc
vdsm-patches@lists.fedorahosted.org