Adam Litke has posted comments on this change. Change subject: StorageDomainManifest: introduce class heirarchy ......................................................................
Patch Set 12: (6 comments) https://gerrit.ovirt.org/#/c/41993/12/tests/manifest_tests.py File tests/manifest_tests.py: Line 32: class FileManifestTests(VdsmTestCase): Line 33: MDSIZE = 524288 Line 34: Line 35: def test_getreaddelay(self): Line 36: with namedTemporaryDir() as path: > I would call path tmpdir to avoid confusion. Done Line 37: manifest = self.make_manifest(path) Line 38: self.assertIsInstance(manifest.getReadDelay(), float) Line 39: Line 40: def make_manifest(self, path): Line 49: manifest.bind(dict()) Line 50: return manifest Line 51: Line 52: Line 53: class BlockManifestTests(VdsmTestCase): > lets add empty line after the class for consistency. Done Line 54: def test_getreaddelay(self): Line 55: with namedTemporaryDir() as path: Line 56: manifest = self.make_manifest(path) Line 57: vg = manifest.sdUUID Line 53: class BlockManifestTests(VdsmTestCase): Line 54: def test_getreaddelay(self): Line 55: with namedTemporaryDir() as path: Line 56: manifest = self.make_manifest(path) Line 57: vg = manifest.sdUUID > better call this vg_name, to avoid confusion later with vg namedtuple retur Done Line 58: lvm = FakeLVM(path) Line 59: make_metafile(lvm.lvPath(vg, 'metadata')) Line 60: Line 61: with MonkeyPatchScope([(blockSD, 'lvm', lvm)]): Line 64: def make_manifest(self, path): Line 65: sduuid = str(uuid.uuid4()) Line 66: manifest = blockSD.BlockStorageDomainManifest(sduuid) Line 67: Line 68: # Just use a dict for the metadata > I think the code explain itself, lets remove the comments. Done Line 69: manifest.bind(dict()) Line 70: return manifest Line 71: Line 72: Line 69: manifest.bind(dict()) Line 70: return manifest Line 71: Line 72: Line 73: def make_metafile(metafile): > Lets rename it to make_fake_metafile and remove the Done Line 74: os.makedirs(os.path.dirname(metafile)) Line 75: with open(metafile, "w") as f: https://gerrit.ovirt.org/#/c/41993/12/tests/storagefakelib.py File tests/storagefakelib.py: Line 1: # Copyright 2012 Red Hat, Inc. > 2015 Done Line 2: # Line 3: # This program is free software; you can redistribute it and/or modify Line 4: # it under the terms of the GNU General Public License as published by Line 5: # the Free Software Foundation; either version 2 of the License, or -- To view, visit https://gerrit.ovirt.org/41993 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia275424a8d423722efb229a4d8545f035fe5fa51 Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke <[email protected]> Gerrit-Reviewer: Adam Litke <[email protected]> Gerrit-Reviewer: Ala Hino <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Freddy Rolland <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
