On Tue, Jul 28, 2020 at 09:13:41PM +0200, Peter Krempa wrote: > 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 <phrd...@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.
You are missing the point that the presence of the file is used later in the script: if ret != 0 or not os.path.exists(access_file): sys.exit(ret) the whole point is that only if the file exists the script will do the actual check and compare the file-access-***.txt file. If the file doesn't exist the script will fail with: Traceback (most recent call last): File "/home/phrdina/work/libvirt/scripts/check-file-access.py", line 52, in <module> with open(access_file, "r") as fh: FileNotFoundError: [Errno 2] No such file or directory: 'file-access-IIyVhsUpnErLEkUm.txt' If we don't mind running the whole script for test cases where the access check doesn't make sense the example above can be changed to this: fd, filename = tempfile.mkstemp(dir=abs_builddir, prefix='file-access-', suffix='.txt') os.close(fd) ... if ret != 0: sys.exit(ret) but at this point it doesn't make any difference and we can just go with the first example where we remove the file as well. Pavel
signature.asc
Description: PGP signature