Change in vdsm[master]: vm: drive: allow error policy fine tuning
nsoffer at redhat.com
nsoffer at redhat.com
Tue Feb 24 20:21:56 UTC 2015
Nir Soffer has posted comments on this change.
Change subject: vm: drive: allow error policy fine tuning
......................................................................
Patch Set 4:
(7 comments)
https://gerrit.ovirt.org/#/c/38009/4/tests/vmStorageTests.py
File tests/vmStorageTests.py:
Line 265:
Line 266: @expandPermutations
Line 267: class ErrorPolicyTests(XMLTestCase):
Line 268:
Line 269: _xml_template = """
We don't have to use _private names in a test class. Nobody is using this object so xml_template is safe enough.
Since this is a constant, it should be XML_TEMPLATE
Line 270: <disk device="disk" snapshot="no" type="file">
Line 271: <source file="/path/to/volume"/>
Line 272: <target bus="virtio" dev="vda"/>
Line 273: <serial>54-a672-23e5b495a9ea</serial>
Line 273: <serial>54-a672-23e5b495a9ea</serial>
Line 274: <driver cache="none" error_policy="%s"
Line 275: io="threads" name="qemu" type="raw"/>
Line 276: </disk>
Line 277: """
Using a template is fragile - if other unrelated code will change (e.g. one formatting the <source>), all these tests will fail for no good reason.
I think we should generated xml and find the relevant element (driver) and compare only that element. Some tests in vmTests are doing this, using find() which could be a testlib helper.
Line 278:
Line 279: @permutations([['stop', 'stop'], ['report', 'report'],
Line 280: ['ignore', 'ignore'], ['enospace', 'enospace']])
Line 281: @MonkeyPatch(utils, 'isBlockDevice', lambda path: False)
Line 278:
Line 279: @permutations([['stop', 'stop'], ['report', 'report'],
Line 280: ['ignore', 'ignore'], ['enospace', 'enospace']])
Line 281: @MonkeyPatch(utils, 'isBlockDevice', lambda path: False)
Line 282: def test_set_supported_policy(self, policy, expected):
test_set_supported -> test_supported?
Line 283: conf = drive_config(errorPolicy=policy)
Line 284: drive = Drive({}, self.log, **conf)
Line 285: self.assertXMLEqual(drive.getXML().toxml(),
Line 286: self._xml_template % expected)
Line 286: self._xml_template % expected)
Line 287:
Line 288: @permutations([[''], [None], [1], ['foobar']])
Line 289: @MonkeyPatch(utils, 'isBlockDevice', lambda path: False)
Line 290: def test_set_unsupported_policy(self, policy):
test_set_unsupported -> test_unsupported?
Line 291: conf = drive_config(errorPolicy=policy)
Line 292: drive = Drive({}, self.log, **conf)
Line 293: self.assertRaises(ValueError, drive.getXML)
Line 294:
Line 293: self.assertRaises(ValueError, drive.getXML)
Line 294:
Line 295: @MonkeyPatch(utils, 'isBlockDevice', lambda path: False)
Line 296: def test_propagate_errors(self):
Line 297: # better explicit than implicit
Unneeded comment.
Line 298: conf = drive_config(propagateErrors='on')
Line 299: drive = Drive({}, self.log, **conf)
Line 300: self.assertXMLEqual(drive.getXML().toxml(),
Line 301: self._xml_template % 'enospace')
Line 294:
Line 295: @MonkeyPatch(utils, 'isBlockDevice', lambda path: False)
Line 296: def test_propagate_errors(self):
Line 297: # better explicit than implicit
Line 298: conf = drive_config(propagateErrors='on')
How errorPolicy and propagateErrors='on' mix? do we need tests for conflicting options?
Line 299: drive = Drive({}, self.log, **conf)
Line 300: self.assertXMLEqual(drive.getXML().toxml(),
Line 301: self._xml_template % 'enospace')
Line 302:
Line 300: self.assertXMLEqual(drive.getXML().toxml(),
Line 301: self._xml_template % 'enospace')
Line 302:
Line 303: @MonkeyPatch(utils, 'isBlockDevice', lambda path: False)
Line 304: def test_not_propagate_errors(self):
not -> do_not?
Line 305: conf = drive_config(propagateErrors='off')
Line 306: drive = Drive({}, self.log, **conf)
Line 307: self.assertXMLEqual(drive.getXML().toxml(),
Line 308: self._xml_template % 'stop')
--
To view, visit https://gerrit.ovirt.org/38009
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1b1234ab15fc87ad75dfeae197d058eec23accbc
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list