Change in vdsm[master]: virt: add device setup and teardown
nsoffer at redhat.com
nsoffer at redhat.com
Thu Apr 28 23:12:06 UTC 2016
Nir Soffer has posted comments on this change.
Change subject: virt: add device setup and teardown
......................................................................
Patch Set 12:
(1 comment)
https://gerrit.ovirt.org/#/c/55135/12/tests/vmfakelib.py
File tests/vmfakelib.py:
Line 419: def __init__(self, device, fail_setup=None, fail_teardown=None):
Line 420: self.fail_setup = fail_setup
Line 421: self.fail_teardown = fail_teardown
Line 422: self.device = device
Line 423: self.is_setup = False
> I was thinking about it. For current tests, it's afaik not needed. Future w
True, but since we don't use it now, I think it is better to remove it. The purpose of this class is to test setup or teardown flows, which never include setting up a device twice - this should raise.
We can use more strict way to record the state.
CREATED = "created"
SETUP = "setup"
TEARDOWN = "teardown"
class Device():
def __init__(self):
self.state = CREATED
def setup(self):
assert self.state is CREATED
self.state = SETUP
def teardown(self):
assert self.state is SETUP
self.state = TEARDOWN
Using this we can delete about half of the assertions in the tests. Instead of testing:
assert device.was_setup
assert device.was_teardown
We can check:
assert device.state is TEARDOWN
The only way the test will succeed is if the setup and teardown were called exactly once in the correct order.
Also the test failure would be much nicer:
"setup" != "teardown"
Instead of:
True is not False
What do you think?
Line 424: self.was_setup = False
Line 425: self.was_teardown = False
Line 426:
Line 427: @recorded
--
To view, visit https://gerrit.ovirt.org/55135
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3f99b855de43cff693b99b6873a835b7ad56db1b
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik 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: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list