[PATCH v2 4/4] test: Don't unmount not (yet) mounted system
Heinrich Schuchardt
xypron.glpk at gmx.de
Thu May 13 13:32:01 CEST 2021
On 2/11/21 3:40 PM, Andy Shevchenko wrote:
> When test suite tries to create a file for a new filesystem test case and fails,
> the clean up of the exception tries to unmount the image, that has not yet been
> mounted. When it happens, the fuse_mounted global variable is set to False and
> inconveniently the test case tries to use sudo, so without this change the
> admin of the machine gets an (annoying) email:
>
> Subject: *** SECURITY information for example.com ***
>
> example.com : Feb 5 19:43:47 : ... COMMAND=/bin/umount .../build-sandbox/persistent-data/mnt
>
> and second run of the test cases on uncleaned build folder will ask for sudo
> which is not what expected.
>
> Besides that there is a double unmount calls during successfully run test case.
>
> All of these due to over engineered Python try-except clause and people didn't
> get it properly at all. The rule of thumb is that don't use more keywords than
> try-except in the exception handling code. Nevertheless, here we adjust code
> to be less intrusive to the initial logic behind that complex and unclear
> constructions in the test case, although it adds a lot of lines of the code,
> i.e. splits one exception handler to three, so on each step we know what
> cleanup shall perform.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko at linux.intel.com>
Dear Andy,
with this merged patch the following tests are not executed anymore locally:
test/py/tests/test_fs/test_basic.py
test/py/tests/test_fs/test_ext.py
SKIPPED [13] test/py/tests/test_fs/conftest.py:350: Setup failed for
filesystem: ext4. Command 'guestmount -a
build-sandbox/persistent-data/3GB.ext4.img -m /dev/sda
build-sandbox/persistent-data/mnt' returned non-zero exit status 1.
Please, revert the patch or fix it.
Best regards
Heinrich
> Reviewed-by: Simon Glass <sjg at chromium.org>
> ---
> v2: new patch
> test/py/tests/test_fs/conftest.py | 78 ++++++++++++++++++++++---------
> 1 file changed, 56 insertions(+), 22 deletions(-)
>
> diff --git a/test/py/tests/test_fs/conftest.py b/test/py/tests/test_fs/conftest.py
> index ec70e8c4ef3f..50af9efcf768 100644
> --- a/test/py/tests/test_fs/conftest.py
> +++ b/test/py/tests/test_fs/conftest.py
> @@ -270,9 +270,20 @@ def fs_obj_basic(request, u_boot_config):
>
> # 3GiB volume
> fs_img = mk_fs(u_boot_config, fs_type, 0xc0000000, '3GB')
> + except CalledProcessError as err:
> + pytest.skip('Creating failed for filesystem: ' + fs_type + '. {}'.format(err))
> + return
>
> - # Mount the image so we can populate it.
> + try:
> 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)
> +
> + try:
> + # Mount the image so we can populate it.
> mount_fs(fs_type, fs_img, mount_dir)
>
> # Create a subdirectory.
> @@ -335,18 +346,15 @@ def fs_obj_basic(request, u_boot_config):
> % big_file, shell=True).decode()
> md5val.append(out.split()[0])
>
> - umount_fs(mount_dir)
> except CalledProcessError as err:
> - pytest.skip('Setup failed for filesystem: ' + fs_type + \
> - '. {}'.format(err))
> + pytest.skip('Setup failed for filesystem: ' + fs_type + '. {}'.format(err))
> return
> else:
> yield [fs_ubtype, fs_img, md5val]
> finally:
> umount_fs(mount_dir)
> call('rmdir %s' % mount_dir, shell=True)
> - if fs_img:
> - call('rm -f %s' % fs_img, shell=True)
> + call('rm -f %s' % fs_img, shell=True)
>
> #
> # Fixture for extended fs test
> @@ -378,9 +386,20 @@ def fs_obj_ext(request, u_boot_config):
>
> # 128MiB volume
> fs_img = mk_fs(u_boot_config, fs_type, 0x8000000, '128MB')
> + except CalledProcessError as err:
> + pytest.skip('Creating failed for filesystem: ' + fs_type + '. {}'.format(err))
> + return
>
> - # Mount the image so we can populate it.
> + try:
> 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)
> +
> + try:
> + # Mount the image so we can populate it.
> mount_fs(fs_type, fs_img, mount_dir)
>
> # Create a test directory
> @@ -422,7 +441,6 @@ def fs_obj_ext(request, u_boot_config):
> md5val.append(out.split()[0])
>
> check_call('rm %s' % tmp_file, shell=True)
> - umount_fs(mount_dir)
> except CalledProcessError:
> pytest.skip('Setup failed for filesystem: ' + fs_type)
> return
> @@ -431,8 +449,7 @@ def fs_obj_ext(request, u_boot_config):
> finally:
> umount_fs(mount_dir)
> call('rmdir %s' % mount_dir, shell=True)
> - if fs_img:
> - call('rm -f %s' % fs_img, shell=True)
> + call('rm -f %s' % fs_img, shell=True)
>
> #
> # Fixture for mkdir test
> @@ -460,11 +477,10 @@ def fs_obj_mkdir(request, u_boot_config):
> fs_img = mk_fs(u_boot_config, fs_type, 0x8000000, '128MB')
> except:
> pytest.skip('Setup failed for filesystem: ' + fs_type)
> + return
> else:
> yield [fs_ubtype, fs_img]
> - finally:
> - if fs_img:
> - call('rm -f %s' % fs_img, shell=True)
> + call('rm -f %s' % fs_img, shell=True)
>
> #
> # Fixture for unlink test
> @@ -493,9 +509,20 @@ def fs_obj_unlink(request, u_boot_config):
>
> # 128MiB volume
> fs_img = mk_fs(u_boot_config, fs_type, 0x8000000, '128MB')
> + except CalledProcessError as err:
> + pytest.skip('Creating failed for filesystem: ' + fs_type + '. {}'.format(err))
> + return
>
> - # Mount the image so we can populate it.
> + try:
> 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)
> +
> + try:
> + # Mount the image so we can populate it.
> mount_fs(fs_type, fs_img, mount_dir)
>
> # Test Case 1 & 3
> @@ -519,7 +546,6 @@ def fs_obj_unlink(request, u_boot_config):
> check_call('dd if=/dev/urandom of=%s/dir5/file1 bs=1K count=1'
> % mount_dir, shell=True)
>
> - umount_fs(mount_dir)
> except CalledProcessError:
> pytest.skip('Setup failed for filesystem: ' + fs_type)
> return
> @@ -528,8 +554,7 @@ def fs_obj_unlink(request, u_boot_config):
> finally:
> umount_fs(mount_dir)
> call('rmdir %s' % mount_dir, shell=True)
> - if fs_img:
> - call('rm -f %s' % fs_img, shell=True)
> + call('rm -f %s' % fs_img, shell=True)
>
> #
> # Fixture for symlink fs test
> @@ -559,11 +584,22 @@ def fs_obj_symlink(request, u_boot_config):
>
> try:
>
> - # 3GiB volume
> + # 1GiB volume
> fs_img = mk_fs(u_boot_config, fs_type, 0x40000000, '1GB')
> + except CalledProcessError as err:
> + pytest.skip('Creating failed for filesystem: ' + fs_type + '. {}'.format(err))
> + return
>
> - # Mount the image so we can populate it.
> + try:
> 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)
> +
> + try:
> + # Mount the image so we can populate it.
> mount_fs(fs_type, fs_img, mount_dir)
>
> # Create a subdirectory.
> @@ -587,7 +623,6 @@ def fs_obj_symlink(request, u_boot_config):
> % medium_file, shell=True).decode()
> md5val.extend([out.split()[0]])
>
> - umount_fs(mount_dir)
> except CalledProcessError:
> pytest.skip('Setup failed for filesystem: ' + fs_type)
> return
> @@ -596,5 +631,4 @@ def fs_obj_symlink(request, u_boot_config):
> finally:
> umount_fs(mount_dir)
> call('rmdir %s' % mount_dir, shell=True)
> - if fs_img:
> - call('rm -f %s' % fs_img, shell=True)
> + call('rm -f %s' % fs_img, shell=True)
>
More information about the U-Boot
mailing list