Change in vdsm[master]: tests: Test storagetestlib FakeEnv functions

nsoffer at redhat.com nsoffer at redhat.com
Sat Apr 9 18:42:34 UTC 2016


Nir Soffer has posted comments on this change.

Change subject: tests: Test storagetestlib FakeEnv functions
......................................................................


Patch Set 1: Code-Review-1

(9 comments)

Nice!

https://gerrit.ovirt.org/#/c/55889/1/tests/storagetestlibTests.py
File tests/storagetestlibTests.py:

Line 65:         with fake_file_env() as env:
Line 66:             set_domain_metaparams(env.sd_manifest,
Line 67:                                   {sd.DMDK_DESCRIPTION: 'foo'})
Line 68:             self.assertEqual('foo',
Line 69:                              env.sd_manifest.getMetaParam(sd.DMDK_DESCRIPTION))
I'm not sure what do you test here - is this related to the fake env?
Line 70: 
Line 71:             # Test that metadata is persisted to our temporary storage area
Line 72:             domain_dir = env.sd_manifest.domaindir
Line 73:             manifest = fileSD.FileStorageDomainManifest(domain_dir)


Line 70: 
Line 71:             # Test that metadata is persisted to our temporary storage area
Line 72:             domain_dir = env.sd_manifest.domaindir
Line 73:             manifest = fileSD.FileStorageDomainManifest(domain_dir)
Line 74:             self.assertEqual('foo', manifest.getMetaParam(sd.DMDK_DESCRIPTION))
Do you want to test that our fake domain looks like a real domain?
Line 75: 
Line 76:     def test_volume_metadata_io(self):
Line 77:         with fake_file_env() as env:
Line 78:             size = 1 * MB


Line 78:             size = 1 * MB
Line 79:             img_id, vol_id = make_file_volume(env.sd_manifest.domaindir, size)
Line 80:             vol = env.sd_manifest.produceVolume(img_id, vol_id)
Line 81:             vol.setDescription('foo')
Line 82:             self.assertEqual('foo', vol.getDescription())
Why do we need this assert? we are not testing the storage domain manifest here.
Line 83: 
Line 84:             # Test that metadata is persisted to our temporary storage area
Line 85:             vol = env.sd_manifest.produceVolume(img_id, vol_id)
Line 86:             self.assertEqual('foo', vol.getDescription())


Line 82:             self.assertEqual('foo', vol.getDescription())
Line 83: 
Line 84:             # Test that metadata is persisted to our temporary storage area
Line 85:             vol = env.sd_manifest.produceVolume(img_id, vol_id)
Line 86:             self.assertEqual('foo', vol.getDescription())
I would be nice to use a constant for 'foo', same for the previous test.
Line 87: 
Line 88: 
Line 89: class FakeBlockEnvTests(VdsmTestCase):
Line 90: 


Line 98:             md_path = env.lvm.lvPath(vg_name, sd.METADATA)
Line 99:             self.assertTrue(os.path.exists(md_path))
Line 100: 
Line 101:             # Check that the domain's special LVs can be found via LVM
Line 102:             special_lvs = sd.SPECIAL_VOLUME_SIZES_MIB.keys() + [sd.METADATA]
Unneeded
Line 103:             for lv in special_lvs:
Line 104:                 self.assertEqual(lv, env.lvm.getLV(vg_name, lv).name)
Line 105: 
Line 106:             images_dir = os.path.join(env.sd_manifest.domaindir, vg_name,


Line 99:             self.assertTrue(os.path.exists(md_path))
Line 100: 
Line 101:             # Check that the domain's special LVs can be found via LVM
Line 102:             special_lvs = sd.SPECIAL_VOLUME_SIZES_MIB.keys() + [sd.METADATA]
Line 103:             for lv in special_lvs:
for lv in blockSD.SPECIAL_LVS:
Line 104:                 self.assertEqual(lv, env.lvm.getLV(vg_name, lv).name)
Line 105: 
Line 106:             images_dir = os.path.join(env.sd_manifest.domaindir, vg_name,
Line 107:                                       sd.DOMAIN_IMAGES)


Line 118:             self.assertEqual(vol_id, env.lvm.getLV(sd_id, vol_id).name)
Line 119:             vol = env.sd_manifest.produceVolume(img_id, vol_id)
Line 120:             self.assertEqual(size / volume.BLOCK_SIZE, vol.getSize())
Line 121:             vol.setDescription('foo')
Line 122:             self.assertEqual('foo', vol.getDescription())
This test the volume class, we don't need to test it here.
Line 123: 
Line 124:             # Test that metadata is persisted to our temporary storage area
Line 125:             vol = env.sd_manifest.produceVolume(img_id, vol_id)
Line 126:             self.assertEqual('foo', vol.getDescription())


Line 122:             self.assertEqual('foo', vol.getDescription())
Line 123: 
Line 124:             # Test that metadata is persisted to our temporary storage area
Line 125:             vol = env.sd_manifest.produceVolume(img_id, vol_id)
Line 126:             self.assertEqual('foo', vol.getDescription())
Use constant for 'foo'
Line 127: 
Line 128:     def test_domain_metadata_io(self):
Line 129:         with fake_block_env() as env:
Line 130:             set_domain_metaparams(env.sd_manifest,


Line 129:         with fake_block_env() as env:
Line 130:             set_domain_metaparams(env.sd_manifest,
Line 131:                                   {sd.DMDK_DESCRIPTION: 'foo'})
Line 132:             self.assertEqual('foo',
Line 133:                              env.sd_manifest.getMetaParam(sd.DMDK_DESCRIPTION))
I think we can remote this
Line 134: 
Line 135:             # Test that metadata is persisted to our temporary storage area
Line 136:             sd_id = env.sd_manifest.sdUUID
Line 137:             manifest = blockSD.BlockStorageDomainManifest(sd_id)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I65ab8c6419dfa1181efff2a533c00589ba5b8263
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list