| Message ID | 20190502084506.8009-8-thuth@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tests/qemu-iotests: Run basic iotests during "make check" | expand |
Thomas Huth <thuth@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 <thuth@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? It's OK to show more in the failing case to help with diagnosis but the passing tests should be easy on the eye when scanning through the logs. > done > > exit $ret > diff --git a/tests/qemu-iotests-quick.sh b/tests/qemu-iotests-quick.sh > deleted file mode 100755 > index 0e554bb972..0000000000 > --- a/tests/qemu-iotests-quick.sh > +++ /dev/null > @@ -1,8 +0,0 @@ > -#!/bin/sh > - > -cd tests/qemu-iotests > - > -ret=0 > -TEST_DIR=${TEST_DIR:-/tmp/qemu-iotests-quick-$$} ./check -T -qcow2 -g quick || ret=1 > - > -exit $ret -- Alex Bennée
On 03/05/2019 11.53, Alex Bennée wrote: > > Thomas Huth <thuth@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 <thuth@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
On 02.05.19 10:45, Thomas Huth wrote: > 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 <thuth@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 Can I interest you in a Makefile target that explicitly excludes check-block? I run the iotests anyway, but I also run make check. Running some iotests twice would be a bit pointless. Max
On 09/05/2019 20.08, Max Reitz wrote: > On 02.05.19 10:45, Thomas Huth wrote: >> 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 <thuth@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 > > Can I interest you in a Makefile target that explicitly excludes > check-block? I run the iotests anyway, but I also run make check. > Running some iotests twice would be a bit pointless. Can't you simply run ./check -qcow2 -x auto instead? Thomas
On 10.05.19 06:29, Thomas Huth wrote: > On 09/05/2019 20.08, Max Reitz wrote: >> On 02.05.19 10:45, Thomas Huth wrote: >>> 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 <thuth@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 >> >> Can I interest you in a Makefile target that explicitly excludes >> check-block? I run the iotests anyway, but I also run make check. >> Running some iotests twice would be a bit pointless. > > Can't you simply run > > ./check -qcow2 -x auto > > instead? I don’t run just qcow2 tests. I run qcow2, qcow2 with compat=0.10, qcow2 with refcount_bits=1, raw, nbd, qed, vmdk, vhdx, ... A lot. So for which of the protocol/format combinations do I exclude the auto group? check-block.sh says it runs raw, qcow2, qed, vmdk, and vpc. But may that not be subject to change? Max
On 10/05/2019 15.34, Max Reitz wrote: > On 10.05.19 06:29, Thomas Huth wrote: >> On 09/05/2019 20.08, Max Reitz wrote: >>> On 02.05.19 10:45, Thomas Huth wrote: >>>> 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 <thuth@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 >>> >>> Can I interest you in a Makefile target that explicitly excludes >>> check-block? I run the iotests anyway, but I also run make check. >>> Running some iotests twice would be a bit pointless. >> >> Can't you simply run >> >> ./check -qcow2 -x auto >> >> instead? > > I don’t run just qcow2 tests. I run qcow2, qcow2 with compat=0.10, > qcow2 with refcount_bits=1, raw, nbd, qed, vmdk, vhdx, ... A lot. > > So for which of the protocol/format combinations do I exclude the auto > group? check-block.sh says it runs raw, qcow2, qed, vmdk, and vpc. But > may that not be subject to change? With my patch series, the auto group is only used for qcow2. Thomas
On 10.05.19 15:34, Max Reitz wrote: > On 10.05.19 06:29, Thomas Huth wrote: >> On 09/05/2019 20.08, Max Reitz wrote: >>> On 02.05.19 10:45, Thomas Huth wrote: >>>> 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 <thuth@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 >>> >>> Can I interest you in a Makefile target that explicitly excludes >>> check-block? I run the iotests anyway, but I also run make check. >>> Running some iotests twice would be a bit pointless. >> >> Can't you simply run >> >> ./check -qcow2 -x auto >> >> instead? > > I don’t run just qcow2 tests. I run qcow2, qcow2 with compat=0.10, > qcow2 with refcount_bits=1, raw, nbd, qed, vmdk, vhdx, ... A lot. > > So for which of the protocol/format combinations do I exclude the auto > group? check-block.sh says it runs raw, qcow2, qed, vmdk, and vpc. But > may that not be subject to change? Oh, and furthermore I prefer to run the tests on tmpfs. Max
On 10.05.19 15:36, Thomas Huth wrote: > On 10/05/2019 15.34, Max Reitz wrote: >> On 10.05.19 06:29, Thomas Huth wrote: >>> On 09/05/2019 20.08, Max Reitz wrote: >>>> On 02.05.19 10:45, Thomas Huth wrote: >>>>> 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 <thuth@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 >>>> >>>> Can I interest you in a Makefile target that explicitly excludes >>>> check-block? I run the iotests anyway, but I also run make check. >>>> Running some iotests twice would be a bit pointless. >>> >>> Can't you simply run >>> >>> ./check -qcow2 -x auto >>> >>> instead? >> >> I don’t run just qcow2 tests. I run qcow2, qcow2 with compat=0.10, >> qcow2 with refcount_bits=1, raw, nbd, qed, vmdk, vhdx, ... A lot. >> >> So for which of the protocol/format combinations do I exclude the auto >> group? check-block.sh says it runs raw, qcow2, qed, vmdk, and vpc. But >> may that not be subject to change? > > With my patch series, the auto group is only used for qcow2. And that is not subject to change? Like, maybe someone wants to add nbd in the future? I mean, I have a test branch anyway which collects a number of patches on top of master that make everything pass more or less reliably for me (11 patches currently...). I suppose I can just revert your patch on top of that. But that doesn’t feel right. Max
On 10/05/2019 15.47, Max Reitz wrote: > On 10.05.19 15:36, Thomas Huth wrote: >> On 10/05/2019 15.34, Max Reitz wrote: >>> On 10.05.19 06:29, Thomas Huth wrote: >>>> On 09/05/2019 20.08, Max Reitz wrote: >>>>> On 02.05.19 10:45, Thomas Huth wrote: >>>>>> 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 <thuth@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 >>>>> >>>>> Can I interest you in a Makefile target that explicitly excludes >>>>> check-block? I run the iotests anyway, but I also run make check. >>>>> Running some iotests twice would be a bit pointless. >>>> >>>> Can't you simply run >>>> >>>> ./check -qcow2 -x auto >>>> >>>> instead? >>> >>> I don’t run just qcow2 tests. I run qcow2, qcow2 with compat=0.10, >>> qcow2 with refcount_bits=1, raw, nbd, qed, vmdk, vhdx, ... A lot. >>> >>> So for which of the protocol/format combinations do I exclude the auto >>> group? check-block.sh says it runs raw, qcow2, qed, vmdk, and vpc. But >>> may that not be subject to change? >> >> With my patch series, the auto group is only used for qcow2. > > And that is not subject to change? Like, maybe someone wants to add nbd > in the future? The current set of qcow2 auto tests takes already quite a while, so I don't think that this will change soon. And if the "normal" users want to run more tests, they can simply use "make check SPEED=slow" or SPEED=thorough, so IMHO no need to extend the quick default list right now. > I mean, I have a test branch anyway which collects a number of patches > on top of master that make everything pass more or less reliably for me > (11 patches currently...). I suppose I can just revert your patch on > top of that. But that doesn’t feel right. Hmm, sure, non-upstream patches are always a bad thing. But I still don't see why you really need an extra Makefile target for this. In the worst case, you could also simply change your script to run something this instead: make $(grep ^check: tests/Makefile.include | sed s/check-block//) ? Thomas
On 10.05.19 18:20, Thomas Huth wrote: > On 10/05/2019 15.47, Max Reitz wrote: >> On 10.05.19 15:36, Thomas Huth wrote: >>> On 10/05/2019 15.34, Max Reitz wrote: >>>> On 10.05.19 06:29, Thomas Huth wrote: >>>>> On 09/05/2019 20.08, Max Reitz wrote: >>>>>> On 02.05.19 10:45, Thomas Huth wrote: >>>>>>> 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 <thuth@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 >>>>>> >>>>>> Can I interest you in a Makefile target that explicitly excludes >>>>>> check-block? I run the iotests anyway, but I also run make check. >>>>>> Running some iotests twice would be a bit pointless. >>>>> >>>>> Can't you simply run >>>>> >>>>> ./check -qcow2 -x auto >>>>> >>>>> instead? >>>> >>>> I don’t run just qcow2 tests. I run qcow2, qcow2 with compat=0.10, >>>> qcow2 with refcount_bits=1, raw, nbd, qed, vmdk, vhdx, ... A lot. >>>> >>>> So for which of the protocol/format combinations do I exclude the auto >>>> group? check-block.sh says it runs raw, qcow2, qed, vmdk, and vpc. But >>>> may that not be subject to change? >>> >>> With my patch series, the auto group is only used for qcow2. >> >> And that is not subject to change? Like, maybe someone wants to add nbd >> in the future? > > The current set of qcow2 auto tests takes already quite a while, so I > don't think that this will change soon. > And if the "normal" users want to run more tests, they can simply use > "make check SPEED=slow" or SPEED=thorough, so IMHO no need to extend the > quick default list right now. > >> I mean, I have a test branch anyway which collects a number of patches >> on top of master that make everything pass more or less reliably for me >> (11 patches currently...). I suppose I can just revert your patch on >> top of that. But that doesn’t feel right. > > Hmm, sure, non-upstream patches are always a bad thing. But I still > don't see why you really need an extra Makefile target for this. In the > worst case, you could also simply change your script to run something > this instead: > > make $(grep ^check: tests/Makefile.include | sed s/check-block//) > > ? Hm. I didn’t think of that. Thanks. :-) Max
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 done exit $ret diff --git a/tests/qemu-iotests-quick.sh b/tests/qemu-iotests-quick.sh deleted file mode 100755 index 0e554bb972..0000000000 --- a/tests/qemu-iotests-quick.sh +++ /dev/null @@ -1,8 +0,0 @@ -#!/bin/sh - -cd tests/qemu-iotests - -ret=0 -TEST_DIR=${TEST_DIR:-/tmp/qemu-iotests-quick-$$} ./check -T -qcow2 -g quick || ret=1 - -exit $ret
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 <thuth@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