public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: u-boot@lists.denx.de
Subject: [PATCH v2 4/4] test: Don't unmount not (yet) mounted system
Date: Thu, 11 Feb 2021 16:40:12 +0200	[thread overview]
Message-ID: <20210211144012.55676-4-andriy.shevchenko@linux.intel.com> (raw)
In-Reply-To: <20210211144012.55676-1-andriy.shevchenko@linux.intel.com>

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@linux.intel.com>
---
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)
-- 
2.30.0

  parent reply	other threads:[~2021-02-11 14:40 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-11 14:40 [PATCH v2 1/4] test: Include /sbin to the PATH when creating ext4 disk image Andy Shevchenko
2021-02-11 14:40 ` [PATCH v2 2/4] test: Allow simple glob pattern in the test name Andy Shevchenko
2021-02-18  4:45   ` Simon Glass
2021-04-29 16:03     ` Simon Glass
2021-02-11 14:40 ` [PATCH v2 3/4] test: Use positive conditional in test_matches() Andy Shevchenko
2021-02-18  4:45   ` Simon Glass
2021-04-29 16:03     ` Simon Glass
2021-02-11 14:40 ` Andy Shevchenko [this message]
2021-02-18  4:45   ` [PATCH v2 4/4] test: Don't unmount not (yet) mounted system Simon Glass
2021-02-18 10:55     ` Andy Shevchenko
2021-02-19  4:52       ` Simon Glass
2021-03-30 19:41         ` Andy Shevchenko
2021-03-30 19:53           ` Tom Rini
2021-03-30 21:50             ` Andy Shevchenko
2021-03-31 13:47   ` Tom Rini
2021-05-13 11:32   ` Heinrich Schuchardt
2021-05-17  6:36     ` Andy Shevchenko
2021-02-11 14:46 ` [PATCH v2 1/4] test: Include /sbin to the PATH when creating ext4 disk image Andy Shevchenko
2021-02-18  4:45 ` Simon Glass
2021-03-15 13:59 ` Andy Shevchenko
2021-03-18 16:56 ` Tom Rini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210211144012.55676-4-andriy.shevchenko@linux.intel.com \
    --to=andriy.shevchenko@linux.intel.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox