Re: [Qemu-devel] [PATCH 2/2] test: replace gtester with a TAP driver

[Philippe Mathieu-Daudé]

Hi Paolo,

On 12/6/18 10:50 PM, Paolo Bonzini wrote:
> gtester is deprecated by upstream glib (see for example the announcement
> at https://blog.gtk.org/2018/07/11/news-from-glib-2-58/) and it does
> not support tests that call g_test_skip in some glib stable releases.
> 
> glib suggests instead using Automake's TAP support, which gtest itself
> supports since version 2.38 (QEMU's minimum requirement is 2.40).
> We do not support Automake, but we can use Automake's code to beautify
> the TAP output.  I chose to use the Perl copy rather than the shell/awk
> one, with some changes so that it can accept TAP through stdin, in order
> to reuse Perl's TAP parsing package.  This also avoids duplicating the
> parser between tap-driver.pl and tap-merge.pl.
> 
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
> v1->v2: show failures even in non-verbose mode
>         show executable name next to PASS/FAIL
>         tweak colored output
>         improved support for "make -k check"
>         switch license blurb to https
>         support TAP version line
>         removed Eamcs epilogs
> 
[...]
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index fb0b449c02..1dda5e2596 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -799,41 +799,53 @@ tests/test-qga$(EXESUF): qemu-ga$(EXESUF)
>  tests/test-qga$(EXESUF): tests/test-qga.o $(qtest-obj-y)
>  
>  SPEED = quick
> -GTESTER_OPTIONS = -k $(if $(V),--verbose,-q)
> -GCOV_OPTIONS = -n $(if $(V),-f,)
>  
>  # gtester tests, possibly with verbose output
> +# do_test_tap runs all tests, even if some of them fail, while do_test_human
> +# stops at the first failure unless -k is given on the command line
> +
> +do_test_human = \
> +     $(call quiet-command, rc=0; \
> +       { $(foreach COMMAND, $1, \
> +         MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$(( $${RANDOM:-0} % 255 + 1))} 
> \
> +           $2 $(COMMAND) -m=$(SPEED) -k --tap \
> +           | ./scripts/tap-driver.pl --test-name="$(notdir $(COMMAND))" 
> --color=always $(if $(V),, --show-failures-only) \
> +           || $(if $(findstring k, $(MAKEFLAGS)), rc=$$?, exit $$?); ) }; 
> exit $$rc, \
> +       "TEST", "$*")
> +
> +do_test_tap = \
> +     $(call quiet-command, \
> +       { $(foreach COMMAND, $1, \
> +         MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$(( $${RANDOM:-0} % 255 + 1))} 
> \
> +           $2 $(COMMAND) -m=$(SPEED) -k --tap | sed "s/^[a-z][a-z]* [0-9]* 
> /&$(notdir $(COMMAND)) /" || true; ) } \
> +           | ./scripts/tap-merge.pl | tee "$@" \
> +           | ./scripts/tap-driver.pl --color=always $(if $(V),, 
> --show-failures-only), \
> +       "TAP","$@")
>  
>  .PHONY: $(patsubst %, check-qtest-%, $(QTEST_TARGETS))
>  $(patsubst %, check-qtest-%, $(QTEST_TARGETS)): check-qtest-%: 
> subdir-%-softmmu $(check-qtest-y)
> -     $(call quiet-command,QTEST_QEMU_BINARY=$*-softmmu/qemu-system-$* \
> -             QTEST_QEMU_IMG=qemu-img$(EXESUF) \
> -             MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$(( $${RANDOM:-0} % 255 + 
> 1))} \
> -             gtester $(GTESTER_OPTIONS) -m=$(SPEED) $(check-qtest-$*-y) 
> $(check-qtest-generic-y),"GTESTER","$@")
> +     $(call do_test_human,$(check-qtest-$*-y) $(check-qtest-generic-y), \
> +       QTEST_QEMU_BINARY=$*-softmmu/qemu-system-$* \
> +       QTEST_QEMU_IMG=qemu-img$(EXESUF))

I was used to gtester filter/skip 'testpath' param:

$ gtester -h
[...]
Utility Options:
  -k, --keep-going              Continue running after tests failed
  -l                            List paths of available test cases
  -m {perf|slow|thorough|quick} Run test cases according to mode
  -m {undefined|no-undefined}   Run test cases according to mode
  -p=TESTPATH                   Only start test cases matching TESTPATH
  -s=TESTPATH                   Skip test cases matching TESTPATH

Using:

$ make check-qtest-x86_64 GTESTER_OPTIONS="-p /x86_64/acpi"

But this commit removed the use of $GTESTER_OPTIONS, however it is
still documented:

$ make check-help
[...]
The variable SPEED can be set to control the gtester speed setting.
Default options are -k and (for make V=1) --verbose; they can be
changed with variable GTESTER_OPTIONS.

Is it possible to filter the tests to run with the TAP driver?

>  .PHONY: $(patsubst %, check-%, $(check-unit-y) $(check-speed-y))
>  $(patsubst %, check-%, $(check-unit-y) $(check-speed-y)): check-%: %
> -     $(call quiet-command, \
> -             MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$(( $${RANDOM:-0} % 255 + 
> 1))} \
> -             gtester $(GTESTER_OPTIONS) -m=$(SPEED) $*,"GTESTER","$*")
> +     $(call do_test_human, $*)
>  
> -# gtester tests with XML output
> +# gtester tests with TAP output
>  
> -$(patsubst %, check-report-qtest-%.xml, $(QTEST_TARGETS)): 
> check-report-qtest-%.xml: $(check-qtest-y)
> -     $(call quiet-command,QTEST_QEMU_BINARY=$*-softmmu/qemu-system-$* \
> -             QTEST_QEMU_IMG=qemu-img$(EXESUF) \
> -       gtester -q $(GTESTER_OPTIONS) -o $@ -m=$(SPEED) $(check-qtest-$*-y) 
> $(check-qtest-generic-y),"GTESTER","$@")
> +$(patsubst %, check-report-qtest-%.tap, $(QTEST_TARGETS)): 
> check-report-qtest-%.tap: $(check-qtest-y)
> +     $(call do_test_tap, $(check-qtest-$*-y) $(check-qtest-generic-y), \
> +       QTEST_QEMU_BINARY=$*-softmmu/qemu-system-$* \
> +       QTEST_QEMU_IMG=qemu-img$(EXESUF))
>  
> -check-report-unit.xml: $(check-unit-y)
> -     $(call quiet-command,gtester -q $(GTESTER_OPTIONS) -o $@ -m=$(SPEED) 
> $^,"GTESTER","$@")
> +check-report-unit.tap: $(check-unit-y)
> +     $(call do_test_tap,$^)
>  
>  # Reports and overall runs
>  
> -check-report.xml: $(patsubst %,check-report-qtest-%.xml, $(QTEST_TARGETS)) 
> check-report-unit.xml
> -     $(call quiet-command,$(SRC_PATH)/scripts/gtester-cat $^ > $@,"GEN","$@")
> -
> -check-report.html: check-report.xml
> -     $(call quiet-command,gtester-report $< > $@,"GEN","$@")
> +check-report.tap: $(patsubst %,check-report-qtest-%.tap, $(QTEST_TARGETS)) 
> check-report-unit.tap
> +     $(call quiet-command,./scripts/tap-merge.py $^ > $@,"GEN","$@")
>  
>  # Per guest TCG tests
>  
> diff --git a/tests/docker/dockerfiles/centos7.docker 
> b/tests/docker/dockerfiles/centos7.docker
> index 0a04bfbed8..e0f18f5a41 100644
> --- a/tests/docker/dockerfiles/centos7.docker
> +++ b/tests/docker/dockerfiles/centos7.docker
> @@ -22,6 +22,7 @@ ENV PACKAGES \
>      mesa-libEGL-devel \
>      mesa-libgbm-devel \
>      nettle-devel \
> +    perl-Test-Harness \
>      pixman-devel \
>      SDL-devel \
>      spice-glib-devel \
> diff --git a/tests/docker/dockerfiles/debian-amd64.docker 
> b/tests/docker/dockerfiles/debian-amd64.docker
> index 24b113b76f..c66e341e5f 100644
> --- a/tests/docker/dockerfiles/debian-amd64.docker
> +++ b/tests/docker/dockerfiles/debian-amd64.docker
> @@ -16,6 +16,7 @@ RUN DEBIAN_FRONTEND=noninteractive eatmydata \
>          liblzo2-dev \
>          librdmacm-dev \
>          libsnappy-dev \
> +        libtest-harness-perl \
>          libvte-dev
>  
>  # virgl
> diff --git a/tests/docker/dockerfiles/debian-ports.docker 
> b/tests/docker/dockerfiles/debian-ports.docker
> index e05a9a9802..514ab53b80 100644
> --- a/tests/docker/dockerfiles/debian-ports.docker
> +++ b/tests/docker/dockerfiles/debian-ports.docker
> @@ -29,6 +29,7 @@ RUN DEBIAN_FRONTEND=noninteractive eatmydata \
>          flex \
>          gettext \
>          git \
> +        libtest-harness-perl \
>          pkg-config \
>          psmisc \
>          python \
> diff --git a/tests/docker/dockerfiles/debian-sid.docker 
> b/tests/docker/dockerfiles/debian-sid.docker
> index 9a3d168705..b30cbe7fc0 100644
> --- a/tests/docker/dockerfiles/debian-sid.docker
> +++ b/tests/docker/dockerfiles/debian-sid.docker
> @@ -26,6 +26,7 @@ RUN DEBIAN_FRONTEND=noninteractive eatmydata \
>          ca-certificates \
>          flex \
>          git \
> +        libtest-harness-perl \
>          pkg-config \
>          psmisc \
>          python \
> diff --git a/tests/docker/dockerfiles/debian8.docker 
> b/tests/docker/dockerfiles/debian8.docker
> index 52945631cd..cdc3f11e06 100644
> --- a/tests/docker/dockerfiles/debian8.docker
> +++ b/tests/docker/dockerfiles/debian8.docker
> @@ -29,6 +29,7 @@ RUN DEBIAN_FRONTEND=noninteractive eatmydata \
>          gettext \
>          git \
>          gnupg \
> +        libtest-harness-perl \
>          pkg-config \
>          python-minimal
>  
> diff --git a/tests/docker/dockerfiles/debian9.docker 
> b/tests/docker/dockerfiles/debian9.docker
> index 154ae2a455..9561d4f225 100644
> --- a/tests/docker/dockerfiles/debian9.docker
> +++ b/tests/docker/dockerfiles/debian9.docker
> @@ -24,6 +24,7 @@ RUN DEBIAN_FRONTEND=noninteractive eatmydata \
>          flex \
>          gettext \
>          git \
> +        libtest-harness-perl \
>          pkg-config \
>          psmisc \
>          python \
> diff --git a/tests/docker/dockerfiles/fedora.docker 
> b/tests/docker/dockerfiles/fedora.docker
> index 0c4eb9e49c..1d0e3dc4ec 100644
> --- a/tests/docker/dockerfiles/fedora.docker
> +++ b/tests/docker/dockerfiles/fedora.docker
> @@ -70,6 +70,7 @@ ENV PACKAGES \
>      nss-devel \
>      numactl-devel \
>      perl \
> +    perl-Test-Harness \
>      pixman-devel \
>      python3 \
>      PyYAML \
> diff --git a/tests/docker/dockerfiles/ubuntu.docker 
> b/tests/docker/dockerfiles/ubuntu.docker
> index 36e2b17de5..229add533c 100644
> --- a/tests/docker/dockerfiles/ubuntu.docker
> +++ b/tests/docker/dockerfiles/ubuntu.docker
> @@ -45,6 +45,7 @@ ENV PACKAGES flex bison \
>      libspice-protocol-dev \
>      libspice-server-dev \
>      libssh2-1-dev \
> +    libtest-harness-perl \
>      libusb-1.0-0-dev \
>      libusbredirhost-dev \
>      libvdeplug-dev \
>