Nir Soffer has posted comments on this change. Change subject: Simplify tests for qemuimg to allow additional capabilities checks ......................................................................
Patch Set 3: (4 comments) Nice! http://gerrit.ovirt.org/#/c/36366/3/tests/qemuimgTests.py File tests/qemuimgTests.py: Line 248: def supported(self, command, result): Line 249: def check(arg): Line 250: self.assertEqual(command, arg) Line 251: return result Line 252: return check This is duplicated from the class above. I would extract a class that both tests inherit from, implementing this helper. Line 253: Line 254: Line 255: class CompatTests(TestCaseBase): Line 256: Line 253: Line 254: Line 255: class CompatTests(TestCaseBase): Line 256: Line 257: def test_qcow2_compat_check_command(self, **kw): This should be two different tests (test_create_command, test_convert_command), since the input and the output are different. Line 258: def create(cmd, **kw): Line 259: expected = [QEMU_IMG, 'create', '-f', 'qcow2', '-o', '?', Line 260: '/dev/null'] Line 261: self.assertEqual(cmd, expected) Line 272: Line 273: with FakeCmd(utils, 'execCmd', convert): Line 274: qemuimg._supports_qcow2_compat('convert') Line 275: Line 276: def test_qcow2_compat_supported(self, **kw): This can be also different tests, reusing self.check_supported defined as a method. Line 277: def check_supported(cmd, **kw): Line 278: return 0, 'Supported options:\ncompat ...\n', '' Line 279: Line 280: with FakeCmd(utils, 'execCmd', check_supported): Line 282: Line 283: with FakeCmd(utils, 'execCmd', check_supported): Line 284: self.assertTrue(qemuimg._supports_qcow2_compat('convert')) Line 285: Line 286: def test_qcow2_compat_unsupported(self, **kw): Same as above Line 287: def check_supported(cmd, **kw): Line 288: return 0, 'Supported options:\nsize ...\n', '' Line 289: Line 290: with FakeCmd(utils, 'execCmd', check_supported): -- To view, visit http://gerrit.ovirt.org/36366 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I386772bf2a25a880b5ad387f284679eed81c5615 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amador Pahim <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
