Change in vdsm[master]: tests: Test storagetestlib FakeEnv functions
alitke at redhat.com
alitke at redhat.com
Mon Apr 11 14:47:42 UTC 2016
Adam Litke has posted comments on this change.
Change subject: tests: Test storagetestlib FakeEnv functions
......................................................................
Patch Set 1:
(9 comments)
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?
The idea is to test that setting the metadata in this instance of the domain manifest (basically the in-memory component of the persistent dict) worked. Next, we'll use a different object to ensure that this metadata change will be "connected" to the new domain manifest instance we are creating.
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?
Is this related to your question above or something different?
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
It's the same reason as for the domain test. I am verifying that the property change first propagated to the object, then I will recreate the volume and ensure the change has persisted. I can remove this check, but I think it helps to illustrate what is being tested (and will help to figure out where the error is should this test fail).
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.
Done
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
Done
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:
Done
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.
Done
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'
Done
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
Done
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: 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