[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v7 10/11] iotests: rewrite check into python
From: |
Kevin Wolf |
Subject: |
Re: [PATCH v7 10/11] iotests: rewrite check into python |
Date: |
Fri, 22 Jan 2021 17:08:04 +0100 |
Am 16.01.2021 um 14:44 hat Vladimir Sementsov-Ogievskiy geschrieben:
> Just use classes introduced in previous three commits. Behavior
> difference is described in these three commits.
>
> Drop group file, as it becomes unused.
>
> Drop common.env: now check is in python, and for tests we use same
> python interpreter that runs the check itself. Use build environment
> PYTHON in check-block instead, to keep "make check" use the same
> python.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> diff --git a/tests/check-block.sh b/tests/check-block.sh
> index fb4c1baae9..26eb1c0a9b 100755
> --- a/tests/check-block.sh
> +++ b/tests/check-block.sh
> @@ -69,7 +69,7 @@ export QEMU_CHECK_BLOCK_AUTO=1
>
> ret=0
> for fmt in $format_list ; do
> - ./check -makecheck -$fmt $group || ret=1
> + ${PYTHON} ./check -makecheck -$fmt $group || ret=1
> done
When I add an echo to print that command line, it seems that ${PYTHON}
is empty for me. Is this expected?
> exit $ret
> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
> index 952762d5ed..914321806a 100755
> --- a/tests/qemu-iotests/check
> +++ b/tests/qemu-iotests/check
[...]
> -
> -# Set qemu-io cache mode with $CACHEMODE we have
> -QEMU_IO_OPTIONS="$QEMU_IO_OPTIONS --cache $CACHEMODE"
> -# Set qemu-io aio mode with $AIOMODE we have
> -QEMU_IO_OPTIONS="$QEMU_IO_OPTIONS --aio $AIOMODE"
> -
> -QEMU_IO_OPTIONS_NO_FMT="$QEMU_IO_OPTIONS"
TestEnv sets the attribute, but forgets to include
QEMU_IO_OPTIONS_NO_FMT in env_variables, so I think it's not actually
exported.
> -if [ "$IMGOPTSSYNTAX" != "true" ]; then
> - QEMU_IO_OPTIONS="$QEMU_IO_OPTIONS -f $IMGFMT"
> -fi
> -
> -# Set default options for qemu-img create -o if they were not specified
> -if [ "$IMGFMT" == "qcow2" ] && ! (echo "$IMGOPTS" | grep "compat=" >
> /dev/null); then
> - IMGOPTS=$(_optstr_add "$IMGOPTS" "compat=1.1")
> -fi
You dropped this one, which makes sense because it's already the default
in 'qemu-img create'. Maybe the commit message could mention it as a
difference.
> -if [ "$IMGFMT" == "luks" ] && ! (echo "$IMGOPTS" | grep "iter-time=" >
> /dev/null); then
> - IMGOPTS=$(_optstr_add "$IMGOPTS" "iter-time=10")
> -fi
> -if [ "$IMGFMT" == "vmdk" ] && ! (echo "$IMGOPTS" | grep "zeroed_grain=" >
> /dev/null); then
> - IMGOPTS=$(_optstr_add "$IMGOPTS" "zeroed_grain=on")
> -fi
> -
> -if [ -z "$SAMPLE_IMG_DIR" ]; then
> - SAMPLE_IMG_DIR="$source_iotests/sample_images"
> -fi
> -
> -export TEST_DIR
> -export SOCK_DIR
> -export SAMPLE_IMG_DIR
> -
> -if [ -s $tmp.list ]
> -then
> - # found some valid test numbers ... this is good
> - :
> -else
> - if $have_test_arg
> - then
> - # had test numbers, but none in group file ... do nothing
> - touch $tmp.list
> - else
> - # no test numbers, do everything from group file
> - sed -n -e '/^[0-9][0-9][0-9]*/s/^\([0-9]*\).*/\1/p'
> <"$source_iotests/group" >$tmp.list
> - fi
> -fi
> -
> -# should be sort -n, but this did not work for Linux when this
> -# was ported from IRIX
> -#
> -list=$(sort $tmp.list)
> -rm -f $tmp.list $tmp.tmp $tmp.sed
> -
> -if [ -z "$QEMU_PROG" ]
> -then
> - if [ -x "$build_iotests/qemu" ]; then
> - export QEMU_PROG="$build_iotests/qemu"
> - elif [ -x "$build_root/qemu-system-${qemu_arch}" ]; then
> - export QEMU_PROG="$build_root/qemu-system-${qemu_arch}"
> - else
> - pushd "$build_root" > /dev/null
> - for binary in qemu-system-*
> - do
> - if [ -x "$binary" ]
> - then
> - export QEMU_PROG="$build_root/$binary"
> - break
> - fi
> - done
> - popd > /dev/null
> - [ "$QEMU_PROG" = "" ] && _init_error "qemu not found"
> - fi
I think this else branch is kind of important (if there is no system
emulator binary for the host architecture, find _any_ system emulator
binary that was built). I can't find its equivalent in the new code.
> -fi
> -export QEMU_PROG="$(type -p "$QEMU_PROG")"
> -
> -export QEMU_OPTIONS="-nodefaults -display none -accel qtest"
> -case "$QEMU_PROG" in
> - *qemu-system-arm|*qemu-system-aarch64)
> - export QEMU_OPTIONS="$QEMU_OPTIONS -machine virt"
> - ;;
> - *qemu-system-avr)
> - export QEMU_OPTIONS="$QEMU_OPTIONS -machine mega2560"
> - ;;
> - *qemu-system-rx)
> - export QEMU_OPTIONS="$QEMU_OPTIONS -machine gdbsim-r5f562n8"
> - ;;
> - *qemu-system-tricore)
> - export QEMU_OPTIONS="-$QEMU_OPTIONS -machine tricore_testboard"
> - ;;
> -esac
> -
> -if [ -z "$QEMU_IMG_PROG" ]; then
> - if [ -x "$build_iotests/qemu-img" ]; then
> - export QEMU_IMG_PROG="$build_iotests/qemu-img"
> - elif [ -x "$build_root/qemu-img" ]; then
> - export QEMU_IMG_PROG="$build_root/qemu-img"
> - else
> - _init_error "qemu-img not found"
> - fi
> -fi
> -export QEMU_IMG_PROG="$(type -p "$QEMU_IMG_PROG")"
> -
> -if [ -z "$QEMU_IO_PROG" ]; then
> - if [ -x "$build_iotests/qemu-io" ]; then
> - export QEMU_IO_PROG="$build_iotests/qemu-io"
> - elif [ -x "$build_root/qemu-io" ]; then
> - export QEMU_IO_PROG="$build_root/qemu-io"
> - else
> - _init_error "qemu-io not found"
> - fi
> -fi
> -export QEMU_IO_PROG="$(type -p "$QEMU_IO_PROG")"
> -
> -if [ -z $QEMU_NBD_PROG ]; then
> - if [ -x "$build_iotests/qemu-nbd" ]; then
> - export QEMU_NBD_PROG="$build_iotests/qemu-nbd"
> - elif [ -x "$build_root/qemu-nbd" ]; then
> - export QEMU_NBD_PROG="$build_root/qemu-nbd"
> - else
> - _init_error "qemu-nbd not found"
> - fi
> -fi
> -export QEMU_NBD_PROG="$(type -p "$QEMU_NBD_PROG")"
> -
> -if [ -z "$QSD_PROG" ]; then
> - if [ -x "$build_iotests/qemu-storage-daemon" ]; then
> - export QSD_PROG="$build_iotests/qemu-storage-daemon"
> - elif [ -x "$build_root/storage-daemon/qemu-storage-daemon" ]; then
> - export QSD_PROG="$build_root/storage-daemon/qemu-storage-daemon"
> - else
> - _init_error "qemu-storage-daemon not found"
> - fi
> -fi
> -export QSD_PROG="$(type -p "$QSD_PROG")"
> -
> -if [ -x "$build_iotests/socket_scm_helper" ]
> -then
> - export SOCKET_SCM_HELPER="$build_iotests/socket_scm_helper"
> -fi
> -
> -python_usable=false
> -if $PYTHON -c 'import sys; sys.exit(0 if sys.version_info >= (3,6) else 1)'
This is not checked any more. Probably okay because building QEMU
already requires 3.6 now.
> -then
> - # Our python framework also requires virtio-blk
> - if "$QEMU_PROG" -M none -device help | grep -q virtio-blk >/dev/null 2>&1
It looks like we lost this check, too? This will probably cause failures
on more exotic architectures.
> - then
> - python_usable=true
> - else
> - python_unusable_because="Missing virtio-blk in QEMU binary"
> - fi
> -else
> - python_unusable_because="Unsupported Python version"
> -fi
[...]
> +import os
> +import sys
> +import argparse
> +from findtests import TestFinder
> +from testenv import TestEnv
> +from testrunner import TestRunner
> +
> +
> +def make_argparser() -> argparse.ArgumentParser:
> + p = argparse.ArgumentParser(description="Test run options")
> +
> + p.add_argument('-n', '--dry-run', action='store_true',
> + help='show me, do not run tests')
> + p.add_argument('-makecheck', action='store_true',
> + help='pretty print output for make check')
> +
> + p.add_argument('-d', dest='debug', action='store_true', help='debug')
> + p.add_argument('-misalign', action='store_true',
> + help='misalign memory allocations')
> +
> + g_env = p.add_argument_group('test environment options')
> + mg = g_env.add_mutually_exclusive_group()
> + # We don't set default for cachemode, as we need to distinguish dafult
> + # from user input later.
> + mg.add_argument('-nocache', dest='cachemode', action='store_const',
> + const='none', help='set cache mode "none" (O_DIRECT), '
> + 'sets CACHEMODE environment variable')
> + mg.add_argument('-c', dest='cachemode',
> + help='sets CACHEMODE environment variable')
> +
> + g_env.add_argument('-i', dest='aiomode', default='threads',
> + help='sets AIOMODE environment variable')
> +
> + p.set_defaults(imgfmt='raw', imgproto='file')
> +
> + format_list = ['raw', 'bochs', 'cloop', 'parallels', 'qcow', 'qcow2',
> + 'qed', 'vdi', 'vpc', 'vhdx', 'vmdk', 'luks', 'dmg']
> + g_fmt = p.add_argument_group(
> + ' image format options',
> + 'The following options set the IMGFMT environment variable. '
> + 'At most one choice is allowed, default is "raw"')
> + mg = g_fmt.add_mutually_exclusive_group()
> + for fmt in format_list:
> + mg.add_argument('-' + fmt, dest='imgfmt', action='store_const',
> + const=fmt, help=f'test {fmt}')
> +
> + protocol_list = ['file', 'rbd', 'sheepdoc', 'nbd', 'ssh', 'nfs',
> + 'fuse']
> + g_prt = p.add_argument_group(
> + ' image protocol options',
> + 'The following options set the IMGPROTO environment variable. '
> + 'At most one choice is allowed, default is "file"')
> + mg = g_prt.add_mutually_exclusive_group()
> + for prt in protocol_list:
> + mg.add_argument('-' + prt, dest='imgproto', action='store_const',
> + const=prt, help=f'test {prt}')
> +
> + g_bash = p.add_argument_group('bash tests options',
> + 'The following options are ignored by '
> + 'python tests.')
> + # TODO: make support for the following options in iotests.py
> + g_bash.add_argument('-o', dest='imgopts',
> + help='options to pass to qemu-img create/convert, '
> + 'sets IMGOPTS environment variable')
> + g_bash.add_argument('-valgrind', dest='VALGRIND_QEMU',
> + action='store_const', const='y',
> + help='use valgrind, sets VALGRIND_QEMU environment '
> + 'variable')
-valgrind is parsed, but the result is never used.
> + g_sel = p.add_argument_group('test selecting options',
> + 'The following options specify test set '
> + 'to run.')
> + g_sel.add_argument('-g', '--groups', metavar='group1,...',
> + help='include tests from these groups')
> + g_sel.add_argument('-x', '--exclude-groups', metavar='group1,...',
> + help='exclude tests from these groups')
> + g_sel.add_argument('--start-from', metavar='TEST',
> + help='Start from specified test: make sorted sequence
> '
> + 'of tests as usual and then drop tests from the first
> '
> + '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')
> +
> + return p
> +
> +
> +if __name__ == '__main__':
> + args = make_argparser().parse_args()
> +
> + env = TestEnv(imgfmt=args.imgfmt, imgproto=args.imgproto,
> + aiomode=args.aiomode, cachemode=args.cachemode,
> + imgopts=args.imgopts, misalign=args.misalign,
> + debug=args.debug)
> +
> + testfinder = TestFinder(test_dir=env.source_iotests)
> +
> + groups = args.groups.split(',') if args.groups else None
> + x_groups = args.exlude_groups.split(',') if args.exclude_groups else None
s/exlude_groups/exclude_groups/
Here the squashed in part:
> group_local = os.path.join(env.source_iotests, 'group.local')
> if os.path.isfile(group_local):
> try:
> testfinder.add_group_file(group_local)
> except ValueError as e:
> sys.exit(f"Filed to parse group file '{group_local}': {e}")
s/Filed/Failed/
> +
> + try:
> + tests = testfinder.find_tests(groups=groups, exclude_groups=x_groups,
> + tests=args.tests,
> + start_from=args.start_from)
> + if not tests:
> + raise ValueError('No tests selected')
> + except ValueError as e:
> + sys.exit(e)
> +
> + if args.dry_run:
> + print('\n'.join(tests))
> + else:
> + with TestRunner(env, args.makecheck) as tr:
> + tr.run_tests([os.path.join(env.source_iotests, t) for t in
> tests])
Maybe it would be worth catching KeyboardInterrupt somewhere? I always
get frightened when I press Ctrl-C and get a stack trace. :-)
Could even be done in the TestRunner, so that the result summary can
still be printed for those tests that were already completed.
Kevin
- [PATCH v7 09/11] iotests: add testrunner.py, (continued)
[PATCH v7 06/11] iotests: define group in each iotest, Vladimir Sementsov-Ogievskiy, 2021/01/16
Re: [PATCH v7 00/11] Rework iotests/check, Eric Blake, 2021/01/20
Re: [PATCH v7 00/11] Rework iotests/check, Paolo Bonzini, 2021/01/21