[PATCH v2 4/4] test: Don't unmount not (yet) mounted system

[Andy Shevchenko]

On Tuesday, March 30, 2021, Tom Rini <trini at konsulko.com> wrote:

> On Tue, Mar 30, 2021 at 10:41:15PM +0300, Andy Shevchenko wrote:
> > On Thu, Feb 18, 2021 at 09:52:12PM -0700, Simon Glass wrote:
> > > On Thu, 18 Feb 2021 at 03:56, Andy Shevchenko <
> andy.shevchenko at gmail.com> wrote:
> > > > On Thu, Feb 18, 2021 at 6:46 AM Simon Glass <sjg at chromium.org>
> wrote:
> > > > > On Thu, 11 Feb 2021 at 07:40, Andy Shevchenko
> > > > > <andriy.shevchenko at linux.intel.com> 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>
> > > > > > ---
> > > > > > 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 <sjg at chromium.org>
> >
> > Thanks!
> >
> > Tom, I don't see this being applied. Can we actually get it in?
>
> Does this apply cleanly to master?  I thought only 1/4 did so I applied
> that.


Patches 2&3 are against Simon’s tree, patches 1&4 are against main branch


>
> --
> Tom
>


-- 
With Best Regards,
Andy Shevchenko