Nir Soffer has posted comments on this change. Change subject: gluster: Don't fail connect server when getting volume info ......................................................................
Patch Set 9: (3 comments) https://gerrit.ovirt.org/#/c/53785/9/tests/storageServerTests.py File tests/storageServerTests.py: Line 274: gluster.validate() Line 275: Line 276: @MonkeyPatch(storageServer, 'supervdsm', FakeSupervdsm()) Line 277: @permutations([[''], ['backup-volfile-servers=server1:server2']]) Line 278: def test_volinfo_if_gluster_server_not_accessible(self, should be test_options_... Line 279: userMountOptions): Line 280: Line 281: def glusterVolumeInfo(volumeName=None, remoteServer=None): Line 282: raise ge.GlusterCmdExecFailedException() Line 280: Line 281: def glusterVolumeInfo(volumeName=None, remoteServer=None): Line 282: raise ge.GlusterCmdExecFailedException() Line 283: Line 284: storageServer.supervdsm.glusterVolumeInfo = glusterVolumeInfo This block (define glusterVolumeInfo that fail, set the fake) is repeated twice. It would be nicer to create a class for testing this scenario - failing to query gluster server, and then you can put this code in the setUp for all tests. Line 285: Line 286: gluster = GlusterFSConnection(spec="192.168.122.1:/music", Line 287: options=userMountOptions) Line 288: self.assertEquals(gluster.volinfo, {}) Line 284: storageServer.supervdsm.glusterVolumeInfo = glusterVolumeInfo Line 285: Line 286: gluster = GlusterFSConnection(spec="192.168.122.1:/music", Line 287: options=userMountOptions) Line 288: self.assertEquals(gluster.volinfo, {}) We don't need this test, we test only options in all the other tests. Lets not test the implementation details if we cannot avoid it. -- To view, visit https://gerrit.ovirt.org/53785 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e1835b7de4c5c4c5c4616d3c36f15c1f91a01dc Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino <[email protected]> Gerrit-Reviewer: Ala Hino <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: gerrit-hooks <[email protected]> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
