Nir Soffer has posted comments on this change.

Change subject: tests: refactor manifest_tests fake env setup
......................................................................


Patch Set 1:

(3 comments)

https://gerrit.ovirt.org/#/c/50273/1/tests/manifest_tests.py
File tests/manifest_tests.py:

Line 53:             manifest = make_blocksd(tmpdir, lvm, metadata=metadata)
Line 54:             yield FakeEnv(manifest, lvm)
Line 55: 
Line 56: 
Line 57: class ManifestTestsMixin(object):
Do we really need this? How many tests are common?
Line 58:     def test_getmetaparam(self):
Line 59:         with self._fake_env() as env:
Line 60:             env.manifest._metadata[sd.DMDK_SDUUID] = 
env.manifest.sdUUID
Line 61:             self.assertEquals(env.manifest.sdUUID,


Line 62:                               
env.manifest.getMetaParam(sd.DMDK_SDUUID))
Line 63: 
Line 64: 
Line 65: class FileManifestTests(VdsmTestCase, ManifestTestsMixin):
Line 66:     _fake_env = fake_file_env
For tests we don't need to use private attributes, nobody is accessing these 
attributes. The test framework will only call test_methods anyway.
Line 67: 
Line 68:     def test_getreaddelay(self):
Line 69:         with self._fake_env() as env:
Line 70:             self.assertIsInstance(env.manifest.getReadDelay(), float)


Line 103: class BlockManifestTests(VdsmTestCase, ManifestTestsMixin):
Line 104:     _fake_env = fake_block_env
Line 105: 
Line 106:     def test_getreaddelay(self):
Line 107:         with self._fake_env() as env:
Nice, but will be even nicer without self._fake_env:

    with fake_block_env() as env:
        ...

And then you can also create both file and block manifest in the same test:

    with fake_block_env() as block_env, fake_file_env() as file_env:
        ...
Line 108:             vg_name = env.manifest.sdUUID
Line 109:             make_file(env.lvm.lvPath(vg_name, 'metadata'))
Line 110:             self.assertIsInstance(env.manifest.getReadDelay(), float)
Line 111: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I74b82ffaa34d6df68e1531735cc776e4de23f95c
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <ali...@redhat.com>
Gerrit-Reviewer: Adam Litke <ali...@redhat.com>
Gerrit-Reviewer: Ala Hino <ah...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Freddy Rolland <froll...@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com>
Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org>
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to