Re: [PATCH 3/4] qemu-iotests: let "check" spawn an arbitrary test command
23.03.2021 20:22, Paolo Bonzini wrote: On 23/03/21 18:11, Vladimir Sementsov-Ogievskiy wrote: If you have positional arguments that must begin with - and don’t look like negative numbers, you can insert the pseudo-argument '--' which tells parse_args() that everything after that is a positional argument: So, as I understand argparse supports '--' feature out of the box. So, we can keep '*' as is, and it would parse all remaining positional arguments which are either tests or the command, and '--' will be automatically dropped. So, we only need to check existing of '--' in original sys.argv to chose our behavior. There is still a difference with REMAINDER: ./check aa -- bb => REMAINDER: error because ./-- is not a test => look for '--': invoke "aa -- bb" So I think REMAINDER provides the best behavior overall. Ok, with '*' you need to check that exactly sys.argv[-len(agrs.tests)-1] == '--', which gives the same behavior as REMAINDER. I'm OK with REMAINDER too, still with we probably also need some comment to not embarrass next person who go into documentation and not find it. -- Best regards, Vladimir
Re: [PATCH 3/4] qemu-iotests: let "check" spawn an arbitrary test command
23.03.2021 20:00, Paolo Bonzini wrote:
On 23/03/21 17:43, Vladimir Sementsov-Ogievskiy wrote:
Interesting that REMAINDER documentation disappeared from latest (3.9) python
documentation https://docs.python.org/3.9/library/argparse.html , but exists
here https://docs.python.org/3.8/library/argparse.html (and no mark of
deprecation)
Whoa. https://bugs.python.org/issue17050 says:
---
Since this feature is buggy, and there isn't an easy fix, we should probably
remove any mention of it from the docs. We can still leave it as an
undocumented legacy feature.
There is precedent for leaving `nargs` constants undocumented.
`argparse.PARSER` ('+...') is used by the subparser mechanism, but is not
documented. https://bugs.python.org/issue16988
---
The problematic case appears to be when you have more than one positional
argument, which is exactly the case with the 3.8 documented use of REMAINDER.
Hence the decision to drop the documentation.
However, "check" works fine because the REMAINDER argument is the only
positional argument:
$ ./check 001 -d
Test "tests/-d" is not found
Another possibility is to pre-process sys.argv like this:
if '--' in sys.argv:
cmd = True
args = sys.argv[0:sys.argv.index('--')]
posargs = sys.argv[len(args)+1:]
else:
cmd = False
args = list(x for x in sys.argv if x.startswith('-'))
posargs = list(x for x in sys.argv if not x.startswith('-'))
But getting the help message right etc. would be messy.
Paolo
Hmm:
If you have positional arguments that must begin with - and don’t look like
negative numbers, you can insert the pseudo-argument '--' which tells
parse_args() that everything after that is a positional argument:
So, as I understand argparse supports '--' feature out of the box. So, we can
keep '*' as is, and it would parse all remaining positional arguments which are
either tests or the command, and '--' will be automatically dropped. So, we
only need to check existing of '--' in original sys.argv to chose our behavior.
--
Best regards,
Vladimir
Re: [PATCH 3/4] qemu-iotests: let "check" spawn an arbitrary test command
On 23/03/21 17:43, Vladimir Sementsov-Ogievskiy wrote:
Interesting that REMAINDER documentation disappeared from latest (3.9)
python documentation https://docs.python.org/3.9/library/argparse.html ,
but exists here https://docs.python.org/3.8/library/argparse.html (and
no mark of deprecation)
Whoa. https://bugs.python.org/issue17050 says:
---
Since this feature is buggy, and there isn't an easy fix, we should
probably remove any mention of it from the docs. We can still leave it
as an undocumented legacy feature.
There is precedent for leaving `nargs` constants undocumented.
`argparse.PARSER` ('+...') is used by the subparser mechanism, but is
not documented. https://bugs.python.org/issue16988
---
The problematic case appears to be when you have more than one
positional argument, which is exactly the case with the 3.8 documented
use of REMAINDER. Hence the decision to drop the documentation.
However, "check" works fine because the REMAINDER argument is the only
positional argument:
$ ./check 001 -d
Test "tests/-d" is not found
Another possibility is to pre-process sys.argv like this:
if '--' in sys.argv:
cmd = True
args = sys.argv[0:sys.argv.index('--')]
posargs = sys.argv[len(args)+1:]
else:
cmd = False
args = list(x for x in sys.argv if x.startswith('-'))
posargs = list(x for x in sys.argv if not x.startswith('-'))
But getting the help message right etc. would be messy.
Paolo
Re: [PATCH 3/4] qemu-iotests: let "check" spawn an arbitrary test command
On 23/03/21 18:11, Vladimir Sementsov-Ogievskiy wrote: If you have positional arguments that must begin with - and don’t look like negative numbers, you can insert the pseudo-argument '--' which tells parse_args() that everything after that is a positional argument: So, as I understand argparse supports '--' feature out of the box. So, we can keep '*' as is, and it would parse all remaining positional arguments which are either tests or the command, and '--' will be automatically dropped. So, we only need to check existing of '--' in original sys.argv to chose our behavior. There is still a difference with REMAINDER: ./check aa -- bb => REMAINDER: error because ./-- is not a test => look for '--': invoke "aa -- bb" So I think REMAINDER provides the best behavior overall. Paolo
Re: [PATCH 3/4] qemu-iotests: let "check" spawn an arbitrary test command
23.03.2021 16:06, Paolo Bonzini wrote:
Right now there is no easy way for "check" to print a reproducer command.
Because such a reproducer command line would be huge, we can instead teach
check to start a command of our choice. This can be for example a Python
unit test with arguments to only run a specific subtest.
Signed-off-by: Paolo Bonzini
---
tests/qemu-iotests/check | 16 ++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index d1c87ceaf1..aab25dac6a 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -19,6 +19,7 @@
import os
import sys
import argparse
+import shutil
from findtests import TestFinder
from testenv import TestEnv
from testrunner import TestRunner
@@ -100,8 +101,8 @@ def make_argparser() -> argparse.ArgumentParser:
'one to TEST (not inclusive). This may be used to '
'rerun failed ./check command, starting from the '
'middle of the process.')
-g_sel.add_argument('tests', metavar='TEST_FILES', nargs='*',
- help='tests to run')
+g_sel.add_argument('tests', nargs=argparse.REMAINDER,
Interesting that REMAINDER documentation disappeared from latest (3.9) python
documentation https://docs.python.org/3.9/library/argparse.html , but exists
here https://docs.python.org/3.8/library/argparse.html (and no mark of
deprecation)
+ help='tests to run, or "--" followed by a command')
return p
@@ -114,6 +115,17 @@ if __name__ == '__main__':
imgopts=args.imgopts, misalign=args.misalign,
debug=args.debug, valgrind=args.valgrind)
+if args.tests[0] == '--':
+del args.tests[0]
+cmd = args.tests
+env.print_env()
+exec_path = shutil.which(cmd[0])
+if exec_path is None:
+sys.exit('command not found: ' + cmd[0])
+cmd[0] = exec_path
+full_env = env.prepare_subprocess(cmd)
+os.execve(cmd[0], cmd, full_env)
+
testfinder = TestFinder(test_dir=env.source_iotests)
groups = args.groups.split(',') if args.groups else None
I hope at some moment we'll run testcases with syntax like
./check -qcow2 test_name.ClassName.method_name
But a possibility to run any command under check script defined environment is
a good thing anyway:
Reviewed-by: Vladimir Sementsov-Ogievskiy
--
Best regards,
Vladimir
