Change in vdsm[master]: storagetests: add test for mount.isMounted
ishaby at redhat.com
ishaby at redhat.com
Mon Mar 28 07:55:27 UTC 2016
Idan Shaby has posted comments on this change.
Change subject: storagetests: add test for mount.isMounted
......................................................................
Patch Set 1:
(2 comments)
https://gerrit.ovirt.org/#/c/55180/1/tests/mountTests.py
File tests/mountTests.py:
Line 303: class TestRemoteSdIsMounted(TestCaseBase):
Line 304:
Line 305: @permutations([
Line 306: # [<fs_spec>, <mount_line>]
Line 307: ["server:/path",
> The spec is not relevant to mount.isMounted, it accept a local file created
Generally you are right - in a normal world this test should check mount.isMounted and the input should be the fs_file.
However, when checking if a file is mounted, the most relevant part of the scenario here is the conversion of fs_spec to fs_file, and it happens before the call to mount.isMounted.
The problem is that normalization of fs_spec is not performed before the conversion to fs_file and so there are cases that the file is not found in /proc/mounts because of it.
If I do what you suggest:
1. We will miss testing this part of the problem - the normalization.
2. The test will compare the given fs_file to the other given fs_file. So it can be reduced to strings comparison, what makes it stupid and useless.
It's true that mount.isMounted calls mount.getMountFromTarget, which calls normpath on the target (the fs_file), but it's redundant since normalizing the fs_file doesn't do anything.
Thus, I think that the input should remain of the form [<fs_spec>, <mount_line>], so that we can check the whole flow, including the normalization part of it.
Line 308: "server:/path /rhev/data-center/mnt/server:_path nfs4 defaults 0 0"],
Line 309: ])
Line 310: def test_is_mounted(self, fs_spec, mount_line):
Line 311: with fake_mounts([mount_line]):
Line 310: def test_is_mounted(self, fs_spec, mount_line):
Line 311: with fake_mounts([mount_line]):
Line 312: self.assertTrue(mount.isMounted(self.generate_fs_file(fs_spec)))
Line 313:
Line 314: def generate_fs_file(self, fs_spec):
> Not needed, as the input to this test should be a fs_file as sent by the co
Please see the comment above.
Line 315: mnt_path = fs_spec.replace('_', '__').replace('/', '_')
--
To view, visit https://gerrit.ovirt.org/55180
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I5f2c1c9f9fa03a079e8d94bd91f8e7b4fd43c0ee
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Idan Shaby <ishaby at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland at redhat.com>
Gerrit-Reviewer: Idan Shaby <ishaby at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Tal Nisan <tnisan at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list