Nir Soffer has posted comments on this change. Change subject: gluster: Allow gluster mount with additional servers ......................................................................
Patch Set 5: (5 comments) https://gerrit.ovirt.org/#/c/40665/5/tests/glusterStorageServerTests.py File tests/glusterStorageServerTests.py: Line 17: class FakeMount(object): Line 18: Line 19: def __init__(self, spec, localPath): Line 20: self._remotePath = spec Line 21: self._localPath = localPath Use public names (self.remotePath etc.), we want to peek into these values Line 22: self.fs_file = "" Line 23: Line 24: def isMounted(self): Line 25: return False Line 21: self._localPath = localPath Line 22: self.fs_file = "" Line 23: Line 24: def isMounted(self): Line 25: return False Make it return self.mounted - so we can test the behavior when remounting. Line 26: Line 27: def mount(self, options, vfsType): Line 28: self._options = options Line 29: self._vfsType = vfsType Line 28: self._options = options Line 29: self._vfsType = vfsType Line 30: Line 31: def getRecord(self): Line 32: return self Simple and nice! Line 33: Line 34: Line 35: class GlusterFSConnectionTests(VdsmTestCase): Line 36: Line 77: gluster = GlusterFSConnection(spec="192.168.122.1:/music", Line 78: mountClass=FakeMount) Line 79: try: Line 80: gluster.connect() Line 81: self.assertEquals(gluster._mount._options, self.MOUNT_OPTIONS) You don't need to connect to check the mount options - please see my comment about this in version 4. I think what we need is: - Simple test that requires no mocking, checking that we create the correct mount options - no need to call connect. - Test that gluster connection create the correct mountpoint directory - Test that mount is created with the correct __init__ arguments - Test that mount.mount is called with the correct arguments Line 82: finally: Line 79: try: Line 80: gluster.connect() Line 81: self.assertEquals(gluster._mount._options, self.MOUNT_OPTIONS) Line 82: finally: Line 83: fileUtils.cleanupdir(gluster._getLocalPath()) When we use a fake, we do *not* want to create directories in the real location. We need to create a temporary directory (see testlib.namedTemporaryDir), which will cleanup automatically without wiring explicit try-finally block. You want something like: with testlib.namedTemporaryDir() as tmpdir: with monkeyPatchScope([(GlusterfsConnection, "localPathBase", tmpdir)]): gluster.connect() # Check that connect did the right thing -- To view, visit https://gerrit.ovirt.org/40665 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2478a5edc1fc9d24eb96d64a32a98a2467ce2989 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Sandro Bonazzola <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
