Adam Litke has posted comments on this change.

Change subject: storage: Add basic BlockVolumeArtifacts
......................................................................


Patch Set 4:

(3 comments)

https://gerrit.ovirt.org/#/c/55987/4/tests/storage_volume_artifacts_test.py
File tests/storage_volume_artifacts_test.py:

Line 119:                 dict(img_id=self.img_id, vol_id=self.vol_id))
Line 120:             params = BASE_COW_PARAMS + (parent,)
Line 121: 
Line 122:             self.assertRaises(se.VolumeAlreadyExists,
Line 123:                               artifacts.create, *params)
> Can you separate the new common tests into another patch (probably before t
Done
Line 124: 
Line 125:     def test_new_image_create_and_commit(self):
Line 126:         with self.fake_env() as env:
Line 127:             artifacts = env.sd_manifest.get_volume_artifacts(


Line 149:                 self.img_id, self.vol_id)
Line 150:             artifacts.create(*BASE_RAW_PARAMS)
Line 151:             self.assertEqual({}, env.sd_manifest.getAllVolumes())
Line 152:             artifacts.commit()
Line 153:             self.assertIn(self.vol_id, 
env.sd_manifest.getAllVolumes())
> Can you explain this change?
It's a confusing diff artifact since the functions being moved are similar.  I 
am just moving tests around.
Line 154: 
Line 155: 
Line 156: class FileVolumeArtifactsTests(VolumeArtifactsTestsMixin, 
VdsmTestCase):
Line 157: 


Line 241:             artifacts = env.sd_manifest.get_volume_artifacts(
Line 242:                 self.img_id, self.vol_id)
Line 243:             artifacts.create(*BASE_RAW_PARAMS)
Line 244:             artifacts.commit()
Line 245:             self.assertRaises(OSError, artifacts.commit)
> These can also move to the patch modifying existing tests, before this patc
Done
Line 246: 
Line 247:     def validate_new_image_path(self, artifacts, has_md=False,
Line 248:                                 has_lease=False, has_volume=False):
Line 249:         path = artifacts.artifacts_dir


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4120c557fc89d82cc6bc854a9fdc8935e53bc93d
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <ali...@redhat.com>
Gerrit-Reviewer: Adam Litke <ali...@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