Commit 1ba21bb06b08 ("test: Don't unmount not (yet) mounted system") fixes an issue in the filesystem tests where the test setup may fail to mount an image and still attempt to unmount it. However, the commit unintentionally breaks the test setups in two ways.

The newly created unmounted filesystem images are being immediately deleted due to some cleanup steps being misplaced into finally blocks, which makes them always run instead of only on failures. The mount calls always fail since the images never exist, causing the tests to be always skipped. This patch moves these cleanup calls into the except blocks to fix this and makes the tests run again.

There are also unmount calls misplaced into finally blocks, making them run after the tests instead of before the tests. These unmount calls make the filesystem image file consistent with the changes made to it as part of the test setup, and this misplacement is making a number of tests fail unexpectedly.

The unmount calls must be run before the tests use the image, meaning before the yield call and not in the finally block. They must also be run as a cleanup step when the filesystem setup fails, so they can't be placed as the final call in the try blocks since they would be skipped on such failures. For these reasons, this patch places the unmount calls both in the except blocks and the else blocks of the final setup step. This makes the unexpectedly failing tests to succeed again.

Furthermore, this isolates the mount calls to their own try-except statement to avoid reintroducing the original issue of unmounting a not-mounted image while fixing the unmount misplacement.

After these fixes, running "make tests" with guestmount available results in two test failures not related to the mentioned commit. If the guestmount executables are unavailable, the mounts fallback to using sudo and result in no failures.

Fixes: 1ba21bb06b08 ("test: Don't unmount not (yet) mounted system") Signed-off-by: Alper Nebi Yasak alpernebiyasak@gmail.com --- There is more discussion at Heinrich's revert-patch [1] of the mentioned commit. This patch is an alternative to the revert.

Andy: I don't think unmounting the failed-to-mount image was the cause of the sudo call you described in that commit, so I believe this would end up running sudo on your system. Wanted to warn in advance, but looks like your issue needs a different solution.

[1] https://patchwork.ozlabs.org/project/uboot/patch/20210513114035.177293-1-xyp...

test/py/tests/test_fs/conftest.py | 48 +++++++++++++++++++++++-------- 1 file changed, 36 insertions(+), 12 deletions(-)

diff --git a/test/py/tests/test_fs/conftest.py b/test/py/tests/test_fs/conftest.py index 50af9efcf768..410a675b9714 100644 --- a/test/py/tests/test_fs/conftest.py +++ b/test/py/tests/test_fs/conftest.py @@ -278,14 +278,19 @@ def fs_obj_basic(request, u_boot_config): check_call('mkdir -p %s' % mount_dir, shell=True) except CalledProcessError as err: pytest.skip('Preparing mount folder failed for filesystem: ' + fs_type + '. {}'.format(err)) - return - finally: call('rm -f %s' % fs_img, shell=True) + return

try: # Mount the image so we can populate it. mount_fs(fs_type, fs_img, mount_dir) + except CalledProcessError as err: + pytest.skip('Mounting to folder failed for filesystem: ' + fs_type + '. {}'.format(err)) + call('rmdir %s' % mount_dir, shell=True) + call('rm -f %s' % fs_img, shell=True) + return

+ try: # Create a subdirectory. check_call('mkdir %s/SUBDIR' % mount_dir, shell=True)

@@ -348,11 +353,12 @@ def fs_obj_basic(request, u_boot_config):

except CalledProcessError as err: pytest.skip('Setup failed for filesystem: ' + fs_type + '. {}'.format(err)) + umount_fs(mount_dir) return else: + umount_fs(mount_dir) yield [fs_ubtype, fs_img, md5val] finally: - umount_fs(mount_dir) call('rmdir %s' % mount_dir, shell=True) call('rm -f %s' % fs_img, shell=True)

@@ -394,14 +400,19 @@ def fs_obj_ext(request, u_boot_config): check_call('mkdir -p %s' % mount_dir, shell=True) except CalledProcessError as err: pytest.skip('Preparing mount folder failed for filesystem: ' + fs_type + '. {}'.format(err)) - return - finally: call('rm -f %s' % fs_img, shell=True) + return

try: # Mount the image so we can populate it. mount_fs(fs_type, fs_img, mount_dir) + except CalledProcessError as err: + pytest.skip('Mounting to folder failed for filesystem: ' + fs_type + '. {}'.format(err)) + call('rmdir %s' % mount_dir, shell=True) + call('rm -f %s' % fs_img, shell=True) + return

+ try: # Create a test directory check_call('mkdir %s/dir1' % mount_dir, shell=True)

@@ -443,11 +454,12 @@ def fs_obj_ext(request, u_boot_config): check_call('rm %s' % tmp_file, shell=True) except CalledProcessError: pytest.skip('Setup failed for filesystem: ' + fs_type) + umount_fs(mount_dir) return else: + umount_fs(mount_dir) yield [fs_ubtype, fs_img, md5val] finally: - umount_fs(mount_dir) call('rmdir %s' % mount_dir, shell=True) call('rm -f %s' % fs_img, shell=True)

@@ -517,14 +529,19 @@ def fs_obj_unlink(request, u_boot_config): check_call('mkdir -p %s' % mount_dir, shell=True) except CalledProcessError as err: pytest.skip('Preparing mount folder failed for filesystem: ' + fs_type + '. {}'.format(err)) - return - finally: call('rm -f %s' % fs_img, shell=True) + return

try: # Mount the image so we can populate it. mount_fs(fs_type, fs_img, mount_dir) + except CalledProcessError as err: + pytest.skip('Mounting to folder failed for filesystem: ' + fs_type + '. {}'.format(err)) + call('rmdir %s' % mount_dir, shell=True) + call('rm -f %s' % fs_img, shell=True) + return

+ try: # Test Case 1 & 3 check_call('mkdir %s/dir1' % mount_dir, shell=True) check_call('dd if=/dev/urandom of=%s/dir1/file1 bs=1K count=1' @@ -548,11 +565,12 @@ def fs_obj_unlink(request, u_boot_config):

except CalledProcessError: pytest.skip('Setup failed for filesystem: ' + fs_type) + umount_fs(mount_dir) return else: + umount_fs(mount_dir) yield [fs_ubtype, fs_img] finally: - umount_fs(mount_dir) call('rmdir %s' % mount_dir, shell=True) call('rm -f %s' % fs_img, shell=True)

@@ -594,14 +612,19 @@ def fs_obj_symlink(request, u_boot_config): check_call('mkdir -p %s' % mount_dir, shell=True) except CalledProcessError as err: pytest.skip('Preparing mount folder failed for filesystem: ' + fs_type + '. {}'.format(err)) - return - finally: call('rm -f %s' % fs_img, shell=True) + return

try: # Mount the image so we can populate it. mount_fs(fs_type, fs_img, mount_dir) + except CalledProcessError as err: + pytest.skip('Mounting to folder failed for filesystem: ' + fs_type + '. {}'.format(err)) + call('rmdir %s' % mount_dir, shell=True) + call('rm -f %s' % fs_img, shell=True) + return

+ try: # Create a subdirectory. check_call('mkdir %s/SUBDIR' % mount_dir, shell=True)

@@ -625,10 +648,11 @@ def fs_obj_symlink(request, u_boot_config):

except CalledProcessError: pytest.skip('Setup failed for filesystem: ' + fs_type) + umount_fs(mount_dir) return else: + umount_fs(mount_dir) yield [fs_ubtype, fs_img, md5val] finally: - umount_fs(mount_dir) call('rmdir %s' % mount_dir, shell=True) call('rm -f %s' % fs_img, shell=True)