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

[Andy Shevchenko]

On Mon, May 17, 2021 at 8:57 PM Andy Shevchenko
<andy.shevchenko at gmail.com> wrote:
>
> On Mon, May 17, 2021 at 8:08 PM Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> > On 17.05.21 16:06, Andy Shevchenko wrote:
>
> ...
>
> > Let's look at the code without your patch:
> >
> > We have multiple functions ending with:
> >
> >     umount_fs(mount_dir)
> > except CalledProcessError as 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)
> >
> >
> > If no exception occurs, unmount_fs() is called twice.
> >
> > Both the unmount_fs() before except and the return statement should be
> > removed.

No, AFAIU the return must be there, otherwise you will go after that
without properly prepared FS.

> Then umount_fs() will be called only in the finally branch.

Btw, that's what my patch does, but at better granularity.

> > I hope this change to the original code is enough to solve your problem.
>
> Probably, but as I mentioned in the commit message that Pythonic
> exceptions are completely over engineered that no-one can get them. I
> think you missed the fact that the exception can happen at any time
> when FS is mounted and when it's not mounted yet. I suggest you think
> more of this code... (Yet it admits that I may have made mistake
> myself, see above)
>
> > I will look into creating a patch.



-- 
With Best Regards,
Andy Shevchenko