From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andy Shevchenko Date: Tue, 30 Mar 2021 22:41:15 +0300 Subject: [PATCH v2 4/4] test: Don't unmount not (yet) mounted system In-Reply-To: References: <20210211144012.55676-1-andriy.shevchenko@linux.intel.com> <20210211144012.55676-4-andriy.shevchenko@linux.intel.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Thu, Feb 18, 2021 at 09:52:12PM -0700, Simon Glass wrote: > On Thu, 18 Feb 2021 at 03:56, Andy Shevchenko wrote: > > On Thu, Feb 18, 2021 at 6:46 AM Simon Glass wrote: > > > On Thu, 11 Feb 2021 at 07:40, 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 > > > > --- > > > > v2: new patch > > > > test/py/tests/test_fs/conftest.py | 78 ++++++++++++++++++++++--------- > > > > 1 file changed, 56 insertions(+), 22 deletions(-) > > > > > > > > > > This looks OK to me, but there is a lot of duplication in the code, > > > isn't there? Perhaps another forray? > > > > Can we apply this fix as is and think about optimisations later, please? > > W/o this I'm really blocked from running tests against U-Boot. > > 'make qcheck' bypasses this. > > +Heinrich Schuchardt > > Reviewed-by: Simon Glass Thanks! Tom, I don't see this being applied. Can we actually get it in? -- With Best Regards, Andy Shevchenko