Change in vdsm[master]: tests: Add make_volume to FakeEnv
gerrit-hooks has posted comments on this change. Change subject: tests: Add make_volume to FakeEnv .. Patch Set 5: * Update tracker: IGNORE, no Bug-Url found * Set MODIFIED::IGNORE, no Bug-Url found. -- To view, visit https://gerrit.ovirt.org/60417 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ieba0617c10e10005e706d9f2a7d4800f077e1c2b Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: tests: Add make_volume to FakeEnv
Nir Soffer has submitted this change and it was merged. Change subject: tests: Add make_volume to FakeEnv .. tests: Add make_volume to FakeEnv The storagetestlib module has fake_block_env and fake_file_env for creating fake environments and make_block_volume and make_file_volume for creating fake volumes within those fake environments. In order to simplify permutated tests that test both environment types, add a make_volume member to the FakeEnv object that can be called with the same parameters in the block and file cases. Change-Id: Ieba0617c10e10005e706d9f2a7d4800f077e1c2b Signed-off-by: Adam Litke Reviewed-on: https://gerrit.ovirt.org/60417 Continuous-Integration: Jenkins CI Reviewed-by: Nir Soffer --- M tests/storagetestlib.py M tests/storagetestlibTests.py 2 files changed, 26 insertions(+), 6 deletions(-) Approvals: Nir Soffer: Looks good to me, approved Adam Litke: Verified Jenkins CI: Passed CI tests -- To view, visit https://gerrit.ovirt.org/60417 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ieba0617c10e10005e706d9f2a7d4800f077e1c2b Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: tests: Add make_volume to FakeEnv
Nir Soffer has posted comments on this change. Change subject: tests: Add make_volume to FakeEnv .. Patch Set 4: Code-Review+2 -- To view, visit https://gerrit.ovirt.org/60417 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ieba0617c10e10005e706d9f2a7d4800f077e1c2b Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: tests: Add make_volume to FakeEnv
Adam Litke has posted comments on this change. Change subject: tests: Add make_volume to FakeEnv .. Patch Set 4: Verified+1 -- To view, visit https://gerrit.ovirt.org/60417 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ieba0617c10e10005e706d9f2a7d4800f077e1c2b Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: tests: Add make_volume to FakeEnv
gerrit-hooks has posted comments on this change. Change subject: tests: Add make_volume to FakeEnv .. Patch Set 4: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/60417 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ieba0617c10e10005e706d9f2a7d4800f077e1c2b Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: tests: Add make_volume to FakeEnv
Nir Soffer has posted comments on this change. Change subject: tests: Add make_volume to FakeEnv .. Patch Set 3: Code-Review-1 Adam, can you fix the failing test so we can merge this and the next patch? -- To view, visit https://gerrit.ovirt.org/60417 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ieba0617c10e10005e706d9f2a7d4800f077e1c2b Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: tests: Add make_volume to FakeEnv
Adam Litke has posted comments on this change. Change subject: tests: Add make_volume to FakeEnv .. Patch Set 3: (1 comment) https://gerrit.ovirt.org/#/c/60417/3/tests/storagetestlib.py File tests/storagetestlib.py: Line 52: desc) Line 53: Line 54: Line 55: class FakeBlockEnv(object): Line 56: def __init__(self, tmpdir, lvm, sd_manifest, sdcache): > Lets keep the same order of the common arguments. Done Line 57: self.tmpdir = tmpdir Line 58: self.lvm = lvm Line 59: self.sd_manifest = sd_manifest Line 60: self.sdcache = sdcache -- To view, visit https://gerrit.ovirt.org/60417 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ieba0617c10e10005e706d9f2a7d4800f077e1c2b Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: tests: Add make_volume to FakeEnv
Nir Soffer has posted comments on this change. Change subject: tests: Add make_volume to FakeEnv .. Patch Set 3: We have faling test: 20:56:03 == 20:56:03 ERROR: test_no_fakelvm (storagetestlibTests.FakeFileEnvTests) 20:56:03 -- 20:56:03 Traceback (most recent call last): 20:56:03 File "/home/jenkins/workspace/vdsm_master_check-patch-el7-x86_64/vdsm/tests/storagetestlibTests.py", line 44, in test_no_fakelvm 20:56:03 self.assertIsNone(env.lvm) 20:56:03 AttributeError: 'FakeFileEnv' object has no attribute 'lvm' Looks like we need to change the test to test for attribute existence instead of None, or just remove this test. -- To view, visit https://gerrit.ovirt.org/60417 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ieba0617c10e10005e706d9f2a7d4800f077e1c2b Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: tests: Add make_volume to FakeEnv
Nir Soffer has posted comments on this change. Change subject: tests: Add make_volume to FakeEnv .. Patch Set 3: (1 comment) https://gerrit.ovirt.org/#/c/60417/3/tests/storagetestlib.py File tests/storagetestlib.py: Line 52: desc) Line 53: Line 54: Line 55: class FakeBlockEnv(object): Line 56: def __init__(self, tmpdir, lvm, sd_manifest, sdcache): Lets keep the same order of the common arguments. Line 57: self.tmpdir = tmpdir Line 58: self.lvm = lvm Line 59: self.sd_manifest = sd_manifest Line 60: self.sdcache = sdcache -- To view, visit https://gerrit.ovirt.org/60417 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ieba0617c10e10005e706d9f2a7d4800f077e1c2b Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: tests: Add make_volume to FakeEnv
gerrit-hooks has posted comments on this change. Change subject: tests: Add make_volume to FakeEnv .. Patch Set 3: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/60417 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ieba0617c10e10005e706d9f2a7d4800f077e1c2b Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: tests: Add make_volume to FakeEnv
Adam Litke has posted comments on this change. Change subject: tests: Add make_volume to FakeEnv .. Patch Set 1: (1 comment) https://gerrit.ovirt.org/#/c/60417/1/tests/storagetestlib.py File tests/storagetestlib.py: Line 43: def __init__(self, tmpdir, sd_manifest, sdcache, make_volume): Line 44: self.tmpdir = tmpdir Line 45: self.sd_manifest = sd_manifest Line 46: self.sdcache = sdcache Line 47: self.make_volume = make_volume > It looks like we need a FakeFileEnv and FakeBlockEnv, so we can use more st Done Line 48: self.lvm = None Line 49: Line 50: Line 51: @contextmanager -- To view, visit https://gerrit.ovirt.org/60417 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ieba0617c10e10005e706d9f2a7d4800f077e1c2b Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: tests: Add make_volume to FakeEnv
gerrit-hooks has posted comments on this change. Change subject: tests: Add make_volume to FakeEnv .. Patch Set 2: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/60417 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ieba0617c10e10005e706d9f2a7d4800f077e1c2b Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: tests: Add make_volume to FakeEnv
Nir Soffer has posted comments on this change. Change subject: tests: Add make_volume to FakeEnv .. Patch Set 1: (1 comment) https://gerrit.ovirt.org/#/c/60417/1/tests/storagetestlib.py File tests/storagetestlib.py: Line 43: def __init__(self, tmpdir, sd_manifest, sdcache, make_volume): Line 44: self.tmpdir = tmpdir Line 45: self.sd_manifest = sd_manifest Line 46: self.sdcache = sdcache Line 47: self.make_volume = make_volume It looks like we need a FakeFileEnv and FakeBlockEnv, so we can use more straightforward code. For example: class FakeFileEnv(object): def __init__(self, tmpdir, sd_manifest, fake_sdc): ... def make_volume(self, *args): return make_file_volume(self.sd_manifest, *args) class FakeBlockEnv(object): def __init__(self, tmpdir, sd_manifest, fake_sdc, lvm): ... def make_volume(self, *args): return make_block_volume(self, self.lvm, self.sd_manifest, *args) Doing the same with partial outside the class is possible by much more complicated and make the code harder to understand. This also solve the lvm issue for file env, we don't need to use default value, since this never relevant to fake file env. I'm using *args because I'm too lazy to lookup the signature, in the real code we should not hide the signature like this. Line 48: self.lvm = None Line 49: Line 50: Line 51: @contextmanager -- To view, visit https://gerrit.ovirt.org/60417 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ieba0617c10e10005e706d9f2a7d4800f077e1c2b Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: tests: Add make_volume to FakeEnv
gerrit-hooks has posted comments on this change. Change subject: tests: Add make_volume to FakeEnv .. Patch Set 1: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/60417 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ieba0617c10e10005e706d9f2a7d4800f077e1c2b Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: tests: Add make_volume to FakeEnv
Adam Litke has uploaded a new change for review. Change subject: tests: Add make_volume to FakeEnv .. tests: Add make_volume to FakeEnv The storagetestlib module has fake_block_env and fake_file_env for creating fake environments and make_block_volume and make_file_volume for creating fake volumes within those fake environments. In order to simplify permutated tests that test both environment types, add a make_volume member to the FakeEnv object that can be called with the same parameters in the block and file cases. Change-Id: Ieba0617c10e10005e706d9f2a7d4800f077e1c2b Signed-off-by: Adam Litke --- M tests/storagetestlib.py 1 file changed, 7 insertions(+), 3 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/17/60417/1 diff --git a/tests/storagetestlib.py b/tests/storagetestlib.py index 021eade..1c23f08 100644 --- a/tests/storagetestlib.py +++ b/tests/storagetestlib.py @@ -19,6 +19,7 @@ import os import uuid from contextlib import contextmanager +from functools import partial from testlib import make_file, namedTemporaryDir from storagefakelib import FakeLVM @@ -39,10 +40,11 @@ class FakeEnv(object): -def __init__(self, tmpdir, sd_manifest, sdcache): +def __init__(self, tmpdir, sd_manifest, sdcache, make_volume): self.tmpdir = tmpdir self.sd_manifest = sd_manifest self.sdcache = sdcache +self.make_volume = make_volume self.lvm = None @@ -57,7 +59,8 @@ [hsm, 'sdCache', fake_sdc], ]): fake_sdc.domains[sd_manifest.sdUUID] = FakeSD(sd_manifest) -yield FakeEnv(tmpdir, sd_manifest, fake_sdc) +make_volume = partial(make_file_volume, sd_manifest) +yield FakeEnv(tmpdir, sd_manifest, fake_sdc, make_volume) @contextmanager @@ -75,7 +78,8 @@ ]): sd_manifest = make_blocksd_manifest(tmpdir, lvm) fake_sdc.domains[sd_manifest.sdUUID] = FakeSD(sd_manifest) -env = FakeEnv(tmpdir, sd_manifest, fake_sdc) +make_volume = partial(make_block_volume, lvm, sd_manifest) +env = FakeEnv(tmpdir, sd_manifest, fake_sdc, make_volume) env.lvm = lvm yield env -- To view, visit https://gerrit.ovirt.org/60417 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ieba0617c10e10005e706d9f2a7d4800f077e1c2b Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
