[PULL,2/3] tests: Run the iotests during "make check" again
diff mbox series

Message ID 20190817085443.11471-3-thuth@redhat.com
State New
Headers show
Series
  • [PULL,1/3] block: fix NetBSD qemu-iotests failure
Related show

Commit Message

Thomas Huth Aug. 17, 2019, 8:54 a.m. UTC
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 seems rather
confusing than helpful).

Message-Id: <20190717111947.30356-4-thuth@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
[AJB: -makecheck to check-block.sh, move check-block to start and gate it]
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 tests/Makefile.include      | 10 +++++----
 tests/check-block.sh        | 44 ++++++++++++++++++++++++++++---------
 tests/qemu-iotests-quick.sh |  8 -------
 3 files changed, 40 insertions(+), 22 deletions(-)
 delete mode 100755 tests/qemu-iotests-quick.sh

Comments

Paolo Bonzini Aug. 22, 2019, 10:34 p.m. UTC | #1
On 17/08/19 10:54, 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 seems rather
> confusing than helpful).
> 
> Message-Id: <20190717111947.30356-4-thuth@redhat.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> [AJB: -makecheck to check-block.sh, move check-block to start and gate it]
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

This breaks when sanitizers are enabled.  There are leaks reported,
though I'm not sure if they are real, and in additions the warning lines
break qemu-iotests' output comparison.

Paolo
Thomas Huth Aug. 23, 2019, 7:12 a.m. UTC | #2
On 8/23/19 12:34 AM, Paolo Bonzini wrote:
> On 17/08/19 10:54, 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 seems rather
>> confusing than helpful).
>>
>> Message-Id: <20190717111947.30356-4-thuth@redhat.com>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> [AJB: -makecheck to check-block.sh, move check-block to start and gate it]
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> 
> This breaks when sanitizers are enabled.  There are leaks reported,
> though I'm not sure if they are real, and in additions the warning lines
> break qemu-iotests' output comparison.

D'oh, I already thought that it was too easy ;-) I'll have a look at it...

 Thomas
Thomas Huth Aug. 23, 2019, 8:59 a.m. UTC | #3
On 8/23/19 9:12 AM, Thomas Huth wrote:
> On 8/23/19 12:34 AM, Paolo Bonzini wrote:
>> On 17/08/19 10:54, 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 seems rather
>>> confusing than helpful).
>>>
>>> Message-Id: <20190717111947.30356-4-thuth@redhat.com>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> [AJB: -makecheck to check-block.sh, move check-block to start and gate it]
>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>
>> This breaks when sanitizers are enabled.  There are leaks reported,
>> though I'm not sure if they are real, and in additions the warning lines
>> break qemu-iotests' output comparison.
> 
> D'oh, I already thought that it was too easy ;-) I'll have a look at it...

If I get that right, the problem only exists when compiling with Clang
and sanitizers enabled - Clang outputs lots of warnings that some things
are not implemented yet and then prints out these leak reports which are
likely false positives...

I've sent a patch now to disable the iotests during "make check" when
sanitizers are enabled. But I wonder whether we should maybe also check
for these incomplete sanitizer implementations in the configure script
to avoid to enable them in this case? (e.g. there are also lots of
warnings during "make check-qtest" in this case, which is somewhat ugly)

 Thomas

Patch
diff mbox series

diff --git a/tests/Makefile.include b/tests/Makefile.include
index fd7fdb8658..6f02dfcc01 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -142,7 +142,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-$(call land,$(CONFIG_POSIX),$(CONFIG_SOFTMMU)) += tests/check-block.sh
 
 # All QTests for now are POSIX-only, but the dependencies are
 # really in libqtest, not in the testcases themselves.
@@ -1092,8 +1092,10 @@  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) \
+		$(patsubst %,%/all,$(filter %-softmmu,$(TARGET_DIRS)))
 	$<
 
 .PHONY: $(patsubst %, check-%, $(check-qapi-schema-y))
@@ -1167,7 +1169,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-block check-qapi-schema check-unit check-softfloat check-qtest check-decodetree
 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..c8b6cec3f6 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 -makecheck -$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