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(a)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.
Pavel