[libvirt PATCH 312/351] meson: tests: add file access test setup

[Peter Krempa]

On Tue, Jul 28, 2020 at 18:48:18 +0200, Pavel Hrdina wrote:
> On Tue, Jul 28, 2020 at 03:39:41PM +0200, Pavel Hrdina wrote:
> > On Tue, Jul 28, 2020 at 03:19:25PM +0200, Pino Toscano wrote:
> > > On Tuesday, 28 July 2020 14:56:45 CEST Peter Krempa wrote:
> > > > On Thu, Jul 16, 2020 at 11:59:08 +0200, Pavel Hrdina wrote:
> > > > > We need to modify check-file-access.py to be usable as wrapper for
> > > > > libvirt tests. This way we can run the tests using this command:
> > > > > 
> > > > >     meson test --setup access
> > > > > 
> > > > > which will run all tests using check-file-access.py as a wrapper.
> > > > > 
> > > > > With autotools all file access are written into single file for all
> > > > > tests and compared once the whole test suite is done.
> > > > > 
> > > > > With Meson we will compare the file access after every single test
> > > > > because it is used as wrapper now. That requires writing the file
> > > > > access into separate files for every single test as they are executed
> > > > > in parallel.
> > > > > 
> > > > > Since the wrapper is used for all tests in Meson including tests outside
> > > > > of tests directory we have to check for presence of the output file.
> > > > > We should also cleanup after ourselves.
> > > > > 
> > > > > Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
> > > > > ---
> > > > >  Makefile.am                  |  3 ---
> > > > >  scripts/check-file-access.py | 24 +++++++++++++++++++-----
> > > > >  tests/Makefile.am            | 12 ------------
> > > > >  tests/meson.build            |  8 ++++++++
> > > > >  tests/virtestmock.c          |  2 +-
> > > > >  5 files changed, 28 insertions(+), 21 deletions(-)
> > > > > 
> > > > > diff --git a/Makefile.am b/Makefile.am
> > > > > index 363c5cf66fd..d05a0c1a85a 100644
> > > > > --- a/Makefile.am
> > > > > +++ b/Makefile.am
> > > > > @@ -37,9 +37,6 @@ srpm: clean
> > > > >  
> > > > >  check-local: all tests
> > > > >  
> > > > > -check-access: all
> > > > > -	@($(MAKE) $(AM_MAKEFLAGS) -C tests check-access)
> > > > > -
> > > > >  dist-hook: gen-AUTHORS
> > > > >  
> > > > >  .PHONY: gen-AUTHORS
> > > > > diff --git a/scripts/check-file-access.py b/scripts/check-file-access.py
> > > > > index aa120cafacf..f0e98f4b652 100755
> > > > > --- a/scripts/check-file-access.py
> > > > > +++ b/scripts/check-file-access.py
> > > > > @@ -21,15 +21,27 @@
> > > > >  #
> > > > >  #
> > > > >  
> > > > > +import os
> > > > > +import random
> > > > >  import re
> > > > > +import string
> > > > >  import sys
> > > > >  
> > > > > -if len(sys.argv) != 3:
> > > > > -    print("syntax: %s ACCESS-FILE PERMITTED-ACCESS-FILE")
> > > > > -    sys.exit(1)
> > > > > +abs_builddir = os.environ.get('abs_builddir', '')
> > > > > +abs_srcdir = os.environ.get('abs_srcdir', '')
> > > > >  
> > > > > -access_file = sys.argv[1]
> > > > > -permitted_file = sys.argv[2]
> > > > > +filename = ''.join(random.choice(string.ascii_letters) for _ in range(16))
> > > > 
> > > > Umm, python doesn't have a 'mkostemp' equivalent? This seems a bit
> > > > fishy.
> > > 
> > > Sure, it is the tempfile module:
> > > https://docs.python.org/3/library/tempfile.html
> > > 
> > >   filename = tempfile.mkstemp(dir=abs_builddir, prefix='file-access-', suffix='.txt')
> > 
> > I'll look into it. When porting it I just looked for any solution as
> > this can be changed any time after the series is pushed.
> 
> So I looked into it and the python script doesn't create the temporary
> file. The file is created by virtestmock.c if we run our test suite with
> file access check. The existence of the file is also used to check if
> there is something to be compared as not all of the tests needs to be
> check if they access some system files, for example all the check-* test
> cases.
> 
> The tempfile is a nice module but useless in this case where we care
> about only getting a temporary file name. In order to use the module
> we would have to do something like this:
> 
> 
> fd, filename = tempfile.mkstemp(dir=abs_builddir, prefix='file-access-', suffix='.txt')
> os.close(fd)
> os.unlink()
> 
> 
> which looks ugly. So I would rather keep it as it is.

Your algorithm is not robust enough so it could end up causing random
test failures. Unlikely but possible.

You can pass the filename to the test and let the mock append to the
existing file and then read it.