Adam Litke has posted comments on this change. Change subject: storage: Add basic BlockVolumeArtifacts ......................................................................
Patch Set 18: (8 comments) https://gerrit.ovirt.org/#/c/55987/18/tests/storage_volume_artifacts_test.py File tests/storage_volume_artifacts_test.py: Line 331: # If we fail to create the LV then storage is clean and we can retry Line 332: with self.fake_env() as env: Line 333: artifacts = env.sd_manifest.get_volume_artifacts( Line 334: self.img_id, self.vol_id) Line 335: artifacts._create_lv_artifact = self.failure > We don't want to know about private methods in the tests. We should mock ou Yes, changed and it looks much nicer. Line 336: self.assertRaises(ExpectedFailure, artifacts.create, Line 337: *BASE_RAW_PARAMS) Line 338: self.validate_invisibility(env, artifacts, is_garbage=False) Line 339: Line 341: artifacts = env.sd_manifest.get_volume_artifacts( Line 342: self.img_id, self.vol_id) Line 343: artifacts.create(*BASE_RAW_PARAMS) Line 344: Line 345: def test_create_fail_acquiring_meta_slot(self): > We need to mock the domain method for acquiring metadata slot. Done Line 346: # If we fail to acquire the meta_slot we have just a garbage LV Line 347: with self.fake_env() as env: Line 348: artifacts = env.sd_manifest.get_volume_artifacts( Line 349: self.img_id, self.vol_id) Line 352: *BASE_RAW_PARAMS) Line 353: self.validate_invisibility(env, artifacts, is_garbage=True) Line 354: self.validate_domain_has_garbage(env.sd_manifest) Line 355: Line 356: def test_create_fail_setting_meta_slot(self): > Name too similar to previous test - maybe test_create_fail_to_set_mdtag changed to test_create_fail_setting_metadata_lvtag Line 357: # If we fail to set the meta_slot in the LV tags that slot remains Line 358: # available for allocation (even without garbage collection) Line 359: Line 360: with self.fake_env() as env: Line 354: self.validate_domain_has_garbage(env.sd_manifest) Line 355: Line 356: def test_create_fail_setting_meta_slot(self): Line 357: # If we fail to set the meta_slot in the LV tags that slot remains Line 358: # available for allocation (even without garbage collection) > Nice! Yeah, pretty slick. Line 359: Line 360: with self.fake_env() as env: Line 361: def patch_and_fail(f): Line 362: @wraps(f) Line 364: with MonkeyPatchScope([ Line 365: [env.lvm, 'changeLVTags', self.failure] Line 366: ]): Line 367: f(*args, **kwargs) Line 368: return wrapper > Too complicated. Fixed with MonkeyPatch Line 369: Line 370: slot_before = self.get_next_free_slot(env) Line 371: artifacts = env.sd_manifest.get_volume_artifacts( Line 372: self.img_id, self.vol_id) Line 370: slot_before = self.get_next_free_slot(env) Line 371: artifacts = env.sd_manifest.get_volume_artifacts( Line 372: self.img_id, self.vol_id) Line 373: artifacts._acquire_metadata_slot = \ Line 374: patch_and_fail(artifacts._acquire_metadata_slot) > We can do: Done Line 375: self.assertRaises(ExpectedFailure, artifacts.create, Line 376: *BASE_RAW_PARAMS) Line 377: self.assertEqual(slot_before, self.get_next_free_slot(env)) Line 378: self.validate_invisibility(env, artifacts, is_garbage=True) Line 384: with self.fake_env() as env: Line 385: slot_before = self.get_next_free_slot(env) Line 386: artifacts = env.sd_manifest.get_volume_artifacts( Line 387: self.img_id, self.vol_id) Line 388: artifacts._create_metadata = self.failure > same Done Line 389: self.assertRaises(ExpectedFailure, artifacts.create, Line 390: *BASE_RAW_PARAMS) Line 391: self.validate_invisibility(env, artifacts, is_garbage=True) Line 392: self.validate_domain_has_garbage(env.sd_manifest) Line 396: # We leave behind a garbage LV and metadata area Line 397: with self.fake_env() as env: Line 398: artifacts = env.sd_manifest.get_volume_artifacts( Line 399: self.img_id, self.vol_id) Line 400: artifacts._create_lease = self.failure > Same - you should mock the domain clusterlock object. I just mocked BlockVolumeMetadata.newVolumeLease Line 401: self.assertRaises(ExpectedFailure, artifacts.create, Line 402: *BASE_RAW_PARAMS) Line 403: self.validate_invisibility(env, artifacts, is_garbage=True) Line 404: self.validate_domain_has_garbage(env.sd_manifest) -- 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: 18 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: 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