Change in vdsm[master]: virt: recovery: tests: exercise more recovery code

mzamazal at redhat.com mzamazal at redhat.com
Fri Feb 19 10:06:23 UTC 2016


Milan Zamazal has posted comments on this change.

Change subject: virt: recovery: tests: exercise more recovery code
......................................................................


Patch Set 5: Code-Review-1

(2 comments)

Not liking code duplication, otherwise fine for me.

https://gerrit.ovirt.org/#/c/53690/5/tests/vmRecoveryTests.py
File tests/vmRecoveryTests.py:

Line 73:             self.assertTrue(res)
Line 74: 
Line 75:             self.assertVmStatus(testvm, fakecif.vmRequests[testvm.id][0])
Line 76: 
Line 77:     def test_load_with_create_exception(self):
This is similar to the previous method. We are relaxed with code duplication in tests, I'm not sure whether it's for clarity of the tests or for less noble reasons. I'd personally merge the methods to a common method receiving create_fail as an argument, but I can live with them as they are now if there is a reason to keep them this way.
Line 78: 
Line 79:         with self.setup_env() as (testvm, tmpdir):
Line 80:             stored = recovery.File(testvm.id)
Line 81:             stored.save(testvm)


Line 92:             self.assertFalse(res)
Line 93:             self.assertEqual(fakecif.vmContainer, {})
Line 94:             self.assertEqual(fakecif.vmRequests, {})
Line 95: 
Line 96:     def test_load_with_create_error(self):
This almost the same as the previous method. Except for one line and some permutation of lines (looks unnecessary to me). So these methods should be definitely merged some way.
Line 97: 
Line 98:         with self.setup_env() as (testvm, tmpdir):
Line 99:             stored = recovery.File(testvm.id)
Line 100:             stored.save(testvm)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ieaf829c5f306a576c2bf5b8776bb5c0784ca3ca1
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: Milan Zamazal <mzamazal at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list