On 03/05/2019 11.53, Alex Bennée wrote: > > Thomas Huth <th...@redhat.com> writes: > >> People often forget to run the iotests before submitting patches or >> pull requests - this is likely due to the fact that we do not run the >> tests during our mandatory "make check" tests yet. Now that we've got >> a proper "auto" group of iotests that should be fine to run in every >> environment, we can enable the iotests during "make check" again by >> running the "auto" tests by default from the check-block.sh script. >> >> Some cases still need to be checked first, though: iotests need bash >> and GNU sed (otherwise they fail), and if gprof is enabled, it spoils >> the output of some test cases causing them to fail. So if we detect >> that one of the required programs is missing or that gprof is enabled, >> we still have to skip the iotests to avoid failures. >> >> And finally, since we are using check-block.sh now again, this patch also >> removes the qemu-iotests-quick.sh script since we do not need that anymore >> (and having two shell wrapper scripts around the block tests seem >> rather confusing than helpful). >> >> Signed-off-by: Thomas Huth <th...@redhat.com> >> --- >> tests/Makefile.include | 8 +++---- >> tests/check-block.sh | 44 ++++++++++++++++++++++++++++--------- >> tests/qemu-iotests-quick.sh | 8 ------- >> 3 files changed, 38 insertions(+), 22 deletions(-) >> delete mode 100755 tests/qemu-iotests-quick.sh >> >> diff --git a/tests/Makefile.include b/tests/Makefile.include >> index e2432d5e77..3bb7793d4a 100644 >> --- a/tests/Makefile.include >> +++ b/tests/Makefile.include >> @@ -140,7 +140,7 @@ check-unit-y += tests/test-uuid$(EXESUF) >> check-unit-y += tests/ptimer-test$(EXESUF) >> check-unit-y += tests/test-qapi-util$(EXESUF) >> >> -check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh >> +check-block-$(CONFIG_POSIX) += tests/check-block.sh >> >> # All QTests for now are POSIX-only, but the dependencies are >> # really in libqtest, not in the testcases themselves. >> @@ -1096,8 +1096,8 @@ clean-tcg: $(CLEAN_TCG_TARGET_RULES) >> >> QEMU_IOTESTS_HELPERS-$(call land,$(CONFIG_SOFTMMU),$(CONFIG_LINUX)) = >> tests/qemu-iotests/socket_scm_helper$(EXESUF) >> >> -.PHONY: check-tests/qemu-iotests-quick.sh >> -check-tests/qemu-iotests-quick.sh: tests/qemu-iotests-quick.sh >> qemu-img$(EXESUF) qemu-io$(EXESUF) qemu-nbd$(EXESUF) >> $(QEMU_IOTESTS_HELPERS-y) >> +.PHONY: check-tests/check-block.sh >> +check-tests/check-block.sh: tests/check-block.sh qemu-img$(EXESUF) >> qemu-io$(EXESUF) qemu-nbd$(EXESUF) $(QEMU_IOTESTS_HELPERS-y) >> $< >> >> .PHONY: $(patsubst %, check-%, $(check-qapi-schema-y)) >> @@ -1168,7 +1168,7 @@ check-acceptance: check-venv $(TESTS_RESULTS_DIR) >> check-qapi-schema: $(patsubst %,check-%, $(check-qapi-schema-y)) >> check-tests/qapi-schema/doc-good.texi >> check-qtest: $(patsubst %,check-qtest-%, $(QTEST_TARGETS)) >> check-block: $(patsubst %,check-%, $(check-block-y)) >> -check: check-qapi-schema check-unit check-softfloat check-qtest >> check-decodetree >> +check: check-qapi-schema check-unit check-softfloat check-qtest >> check-decodetree check-block >> check-clean: >> rm -rf $(check-unit-y) tests/*.o $(QEMU_IOTESTS_HELPERS-y) >> rm -rf $(sort $(foreach target,$(SYSEMU_TARGET_LIST), >> $(check-qtest-$(target)-y)) $(check-qtest-generic-y)) >> diff --git a/tests/check-block.sh b/tests/check-block.sh >> index f3d12fd602..3b971d6cf4 100755 >> --- a/tests/check-block.sh >> +++ b/tests/check-block.sh >> @@ -1,24 +1,48 @@ >> #!/bin/sh >> >> -FORMAT_LIST="raw qcow2 qed vmdk vpc" >> +# Honor the SPEED environment variable, just like we do it for the qtests. >> +if [ "$SPEED" = "slow" ]; then >> + format_list="raw qcow2" >> + group= >> +elif [ "$SPEED" = "thorough" ]; then >> + format_list="raw qcow2 qed vmdk vpc" >> + group= >> +else >> + format_list=qcow2 >> + group="-g auto" >> +fi >> + >> if [ "$#" -ne 0 ]; then >> - FORMAT_LIST="$@" >> + format_list="$@" >> +fi >> + >> +if grep -q "TARGET_GPROF=y" *-softmmu/config-target.mak 2>/dev/null ; then >> + echo "GPROF is enabled ==> Not running the qemu-iotests." >> + exit 0 >> fi >> >> -export QEMU_PROG="$PWD/x86_64-softmmu/qemu-system-x86_64" >> -export QEMU_IMG_PROG="$PWD/qemu-img" >> -export QEMU_IO_PROG="$PWD/qemu-io" >> +if [ -z "$(find . -name 'qemu-system-*' -print)" ]; then >> + echo "No qemu-system binary available ==> Not running the qemu-iotests." >> + exit 0 >> +fi >> + >> +if ! command -v bash >/dev/null 2>&1 ; then >> + echo "bash not available ==> Not running the qemu-iotests." >> + exit 0 >> +fi >> >> -if [ ! -x $QEMU_PROG ]; then >> - echo "'make check-block' requires qemu-system-x86_64" >> - exit 1 >> +if ! (sed --version | grep 'GNU sed') > /dev/null 2>&1 ; then >> + if ! command -v gsed >/dev/null 2>&1; then >> + echo "GNU sed not available ==> Not running the qemu-iotests." >> + exit 0 >> + fi >> fi >> >> cd tests/qemu-iotests >> >> ret=0 >> -for FMT in $FORMAT_LIST ; do >> - ./check -T -nocache -$FMT || ret=1 >> +for fmt in $format_list ; do >> + ./check -$fmt $group || ret=1 > > This is all looking good and my only remaining objection is aesthetic. > If you run "make check -jN" you end up with the with the block test > output intermingled with the rest of the output which is now fairly > uniform. > > Any chance we could hide the verbosity unless requested and have > something like: > > TEST check-block: test xxx > > emitted for each passing test? Yeah, I noticed this already, too. I was already thinking of changing the tests/qemu-iotests/check script a little bit (maybe even teaching it the TAP protocol?), but I felt it was too much for this series. Considering that the iotests have been broken two times already after the 4.1 tree has been opened, I think we should get this merged as soon as possible to avoid at least partly future regressions. Clean up of the test output can and will be done in a separate patch later.
Thomas
