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

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

[Paolo Bonzini]

gtester is deprecated by upstream glib and it does not support tests
that call g_test_skip in some glib stable releases.

glib suggests instead using Automake's TAP support.  We do not support
Automake, but we can copy the code that beautifies the TAP output and
use it.  I chose to use the Perl copy rather than the shell/awk one,
in order to reuse Perl's TAP parsing package, but I'm open to suggestions
about which language to use.

Thanks,

Paolo

Paolo Bonzini (2):
  test: execute g_test_run when tests are skipped
  test: replace gtester with a TAP driver

 scripts/gtester-cat                          |  26 --
 scripts/tap-driver.pl                        | 386 +++++++++++++++++++++++++++
 scripts/tap-merge.pl                         | 108 ++++++++
 tests/Makefile.include                       |  54 ++--
 tests/cdrom-test.c                           |   2 +-
 tests/docker/dockerfiles/centos7.docker      |   1 +
 tests/docker/dockerfiles/debian-amd64.docker |   1 +
 tests/docker/dockerfiles/debian-ports.docker |   1 +
 tests/docker/dockerfiles/debian-sid.docker   |   1 +
 tests/docker/dockerfiles/debian8.docker      |   1 +
 tests/docker/dockerfiles/debian9.docker      |   1 +
 tests/docker/dockerfiles/fedora.docker       |   1 +
 tests/docker/dockerfiles/ubuntu.docker       |   1 +
 tests/migration-test.c                       |   8 +-
 14 files changed, 540 insertions(+), 52 deletions(-)
 delete mode 100755 scripts/gtester-cat
 create mode 100755 scripts/tap-driver.pl
 create mode 100755 scripts/tap-merge.pl

-- 
1.8.3.1

[Qemu-devel] [PATCH 1/2] test: execute g_test_run when tests are skipped

[Paolo Bonzini]

Sometimes a test's main() function recognizes that the environment
does not support the test, and therefore exits.  In this case, we
still should run g_test_run() so that a TAP harness will print the
test plan ("1..0") and the test will be marked as skipped.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/cdrom-test.c     | 2 +-
 tests/migration-test.c | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/cdrom-test.c b/tests/cdrom-test.c
index 9b43dc9..14bd981 100644
--- a/tests/cdrom-test.c
+++ b/tests/cdrom-test.c
@@ -169,7 +169,7 @@ int main(int argc, char **argv)
 
     if (exec_genisoimg(genisocheck)) {
         /* genisoimage not available - so can't run tests */
-        return 0;
+        return g_test_run();
     }
 
     ret = prepare_image(arch, isoimage);
diff --git a/tests/migration-test.c b/tests/migration-test.c
index 06ca506..8352612 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -789,7 +789,7 @@ int main(int argc, char **argv)
     g_test_init(&argc, &argv, NULL);
 
     if (!ufd_version_check()) {
-        return 0;
+        return g_test_run();
     }
 
     /*
@@ -800,7 +800,7 @@ int main(int argc, char **argv)
     if (g_str_equal(qtest_get_arch(), "ppc64") &&
         access("/sys/module/kvm_hv", F_OK)) {
         g_test_message("Skipping test: kvm_hv not available");
-        return 0;
+        return g_test_run();
     }
 
     /*
@@ -811,11 +811,11 @@ int main(int argc, char **argv)
 #if defined(HOST_S390X)
         if (access("/dev/kvm", R_OK | W_OK)) {
             g_test_message("Skipping test: kvm not available");
-            return 0;
+            return g_test_run();
         }
 #else
         g_test_message("Skipping test: Need s390x host to work properly");
-        return 0;
+        return g_test_run();
 #endif
     }
 
-- 
1.8.3.1

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

[Paolo Bonzini]

gtester is deprecated by upstream glib and it does not support tests
that call g_test_skip in some glib stable releases.

glib suggests instead using Automake's TAP support.  We do not support
Automake, but we can copy the code that beautifies the TAP output and
use it.  I chose to use the Perl copy rather than the shell/awk one,
in order to reuse Perl's TAP parsing package, but I'm open to suggestions
about which language to use.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/gtester-cat                          |  26 --
 scripts/tap-driver.pl                        | 386 +++++++++++++++++++++++++++
 scripts/tap-merge.pl                         | 108 ++++++++
 tests/Makefile.include                       |  54 ++--
 tests/docker/dockerfiles/centos7.docker      |   1 +
 tests/docker/dockerfiles/debian-amd64.docker |   1 +
 tests/docker/dockerfiles/debian-ports.docker |   1 +
 tests/docker/dockerfiles/debian-sid.docker   |   1 +
 tests/docker/dockerfiles/debian8.docker      |   1 +
 tests/docker/dockerfiles/debian9.docker      |   1 +
 tests/docker/dockerfiles/fedora.docker       |   1 +
 tests/docker/dockerfiles/ubuntu.docker       |   1 +
 12 files changed, 535 insertions(+), 47 deletions(-)
 delete mode 100755 scripts/gtester-cat
 create mode 100755 scripts/tap-driver.pl
 create mode 100755 scripts/tap-merge.pl

diff --git a/scripts/gtester-cat b/scripts/gtester-cat
deleted file mode 100755
index 061a952..0000000
--- a/scripts/gtester-cat
+++ /dev/null
@@ -1,26 +0,0 @@
-#!/bin/sh
-#
-# Copyright IBM, Corp. 2012
-#
-# Authors:
-#  Anthony Liguori <aliguori@us.ibm.com>
-#
-# This work is licensed under the terms of the GNU GPLv2 or later.
-# See the COPYING file in the top-level directory.
-
-cat <<EOF
-<?xml version="1.0"?>
-<gtester>
- <info>
-  <package>qemu</package>
-  <version>0.0</version>
-  <revision>rev</revision>
- </info>
-EOF
-
-sed \
-  -e '/<?xml/d' \
-  -e '/^<gtester>$/d' \
-  -e '/<info>/,/<\/info>/d' \
-  -e '$b' \
-  -e '/^<\/gtester>$/d' "$@"
diff --git a/scripts/tap-driver.pl b/scripts/tap-driver.pl
new file mode 100755
index 0000000..2e674c0
--- /dev/null
+++ b/scripts/tap-driver.pl
@@ -0,0 +1,386 @@
+#! /usr/bin/env perl
+# Copyright (C) 2011-2013 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# As a special exception to the GNU General Public License, if you
+# distribute this file as part of a program that contains a
+# configuration script generated by Autoconf, you may include it under
+# the same distribution terms that you use for the rest of that program.
+
+# ---------------------------------- #
+#  Imports, static data, and setup.  #
+# ---------------------------------- #
+
+use warnings FATAL => 'all';
+use strict;
+use Getopt::Long ();
+use TAP::Parser;
+use Term::ANSIColor qw(:constants);
+
+my $VERSION = '2012-02-01.19'; # UTC
+
+my $ME = "tap-driver.pl";
+
+my $USAGE = <<'END';
+Usage:
+  tap-driver [--color={always|never|auto}]
+             [--verbose]
+END
+
+my $HELP = "$ME: TAP-aware test driver for QEMU testsuite harness." .
+           "\n" . $USAGE;
+
+# It's important that NO_PLAN evaluates "false" as a boolean.
+use constant NO_PLAN => 0;
+use constant EARLY_PLAN => 1;
+use constant LATE_PLAN => 2;
+
+# ------------------- #
+#  Global variables.  #
+# ------------------- #
+
+my $testno = 0;     # Number of test results seen so far.
+my $bailed_out = 0; # Whether a "Bail out!" directive has been seen.
+
+# Whether the TAP plan has been seen or not, and if yes, which kind
+# it is ("early" is seen before any test result, "late" otherwise).
+my $plan_seen = NO_PLAN;
+
+# ----------------- #
+#  Option parsing.  #
+# ----------------- #
+
+my %cfg = (
+  "color" => 0,
+  "verbose" => 0,
+);
+
+my $diag_string = "#";
+my $color = "auto";
+
+# Perl's Getopt::Long allows options to take optional arguments after a space.
+# Prevent --color by itself from consuming other arguments
+foreach (@ARGV) {
+  if ($_ eq "--color" || $_ eq "-color") {
+    $_ = "--color=$color";
+  }
+}
+
+Getopt::Long::GetOptions
+  (
+    'help' => sub { print $HELP; exit 0; },
+    'version' => sub { print "$ME $VERSION\n"; exit 0; },
+    'color=s'  => \$color,
+    'verbose' => sub { $cfg{"verbose"} = 1; },
+  ) or exit 1;
+
+if ($color =~ /^always$/i) {
+  $cfg{'color'} = 1;
+} elsif ($color =~ /^never$/i) {
+  $cfg{'color'} = 0;
+} elsif ($color =~ /^auto$/i) {
+  $cfg{'color'} = (-t STDOUT);
+} else {
+  die "Invalid color mode: $color\n";
+}
+
+# ------------- #
+#  Prototypes.  #
+# ------------- #
+
+sub bool_opt ($$);
+sub colored ($$);
+sub decorate_result ($);
+sub extract_tap_comment ($);
+sub handle_tap_bailout ($);
+sub handle_tap_plan ($);
+sub handle_tap_result ($);
+sub is_null_string ($);
+sub main (@);
+sub report ($;$);
+sub stringify_result_obj ($);
+sub testsuite_error ($);
+
+# -------------- #
+#  Subroutines.  #
+# -------------- #
+
+sub bool_opt ($$)
+{
+  my ($opt, $val) = @_;
+  if ($val =~ /^(?:y|yes)\z/i)
+    {
+      $cfg{$opt} = 1;
+    }
+  elsif ($val =~ /^(?:n|no)\z/i)
+    {
+      $cfg{$opt} = 0;
+    }
+  else
+    {
+      die "$ME: invalid argument '$val' for option '$opt'\n";
+    }
+}
+
+# If the given string is undefined or empty, return true, otherwise
+# return false.  This function is useful to avoid pitfalls like:
+#   if ($message) { print "$message\n"; }
+# which wouldn't print anything if $message is the literal "0".
+sub is_null_string ($)
+{
+  my $str = shift;
+  return ! (defined $str and length $str);
+}
+
+sub stringify_result_obj ($)
+{
+  my $result_obj = shift;
+  if ($result_obj->is_unplanned || $result_obj->number != $testno)
+    {
+      return "ERROR";
+    }
+  elsif ($plan_seen == LATE_PLAN)
+    {
+      return "ERROR";
+    }
+  elsif (!$result_obj->directive)
+    {
+      return $result_obj->is_ok ? "PASS" : "FAIL";
+    }
+  elsif ($result_obj->has_todo)
+    {
+      return $result_obj->is_actual_ok ? "XPASS" : "XFAIL";
+    }
+  elsif ($result_obj->has_skip)
+    {
+      return $result_obj->is_ok ? "SKIP" : "FAIL";
+    }
+  die "$ME: INTERNAL ERROR"; # NOTREACHED
+}
+
+sub colored ($$)
+{
+  my ($color_string, $text) = @_;
+  return $color_string . $text . RESET;
+}
+
+sub decorate_result ($)
+{
+  my $result = shift;
+  return $result unless $cfg{"color"};
+  my %color_for_result =
+    (
+      "ERROR" => MAGENTA,
+      "PASS"  => GREEN,
+      "XPASS" => RED,
+      "FAIL"  => BOLD.RED,
+      "XFAIL" => BOLD.GREEN,
+      "SKIP"  => BLUE,
+    );
+  if (my $color = $color_for_result{$result})
+    {
+      return colored ($color, $result);
+    }
+  else
+    {
+      return $result; # Don't colorize unknown stuff.
+    }
+}
+
+sub report ($;$)
+{
+  my ($msg, $result, $explanation) = (undef, @_);
+  if ($result =~ /^(?:X?(?:PASS|FAIL)|SKIP|ERROR)/)
+    {
+      # Output on console might be colorized.
+      $msg = decorate_result($result);
+    }
+  elsif ($result eq "#")
+    {
+      $msg = "  ";
+    }
+  else
+    {
+      die "$ME: INTERNAL ERROR"; # NOTREACHED
+    }
+  $msg .= " $explanation" if defined $explanation;
+  print $msg . "\n";
+}
+
+sub testsuite_error ($)
+{
+  report "ERROR", "- $_[0]";
+}
+
+sub handle_tap_result ($)
+{
+  $testno++;
+  my $result_obj = shift;
+
+  my $test_result = stringify_result_obj $result_obj;
+  my $string = $result_obj->number;
+
+  my $description = $result_obj->description;
+  $string .= " $description"
+    unless is_null_string $description;
+
+  if ($plan_seen == LATE_PLAN)
+    {
+      $string .= " # AFTER LATE PLAN";
+    }
+  elsif ($result_obj->is_unplanned)
+    {
+      $string .= " # UNPLANNED";
+    }
+  elsif ($result_obj->number != $testno)
+    {
+      $string .= " # OUT-OF-ORDER (expecting $testno)";
+    }
+  elsif (my $directive = $result_obj->directive)
+    {
+      $string .= " # $directive";
+      my $explanation = $result_obj->explanation;
+      $string .= " $explanation"
+        unless is_null_string $explanation;
+    }
+
+  report $test_result, $string;
+}
+
+sub handle_tap_plan ($)
+{
+  my $plan = shift;
+  if ($plan_seen)
+    {
+      # Error, only one plan per stream is acceptable.
+      testsuite_error "multiple test plans";
+      return;
+    }
+  # The TAP plan can come before or after *all* the TAP results; we speak
+  # respectively of an "early" or a "late" plan.  If we see the plan line
+  # after at least one TAP result has been seen, assume we have a late
+  # plan; in this case, any further test result seen after the plan will
+  # be flagged as an error.
+  $plan_seen = ($testno >= 1 ? LATE_PLAN : EARLY_PLAN);
+  # If $testno > 0, we have an error ("too many tests run") that will be
+  # automatically dealt with later, so don't worry about it here.  If
+  # $plan_seen is true, we have an error due to a repeated plan, and that
+  # has already been dealt with above.  Otherwise, we have a valid "plan
+  # with SKIP" specification, and should report it as a particular kind
+  # of SKIP result.
+  if ($plan->directive && $testno == 0)
+    {
+      my $explanation = is_null_string ($plan->explanation) ?
+                        undef : "- " . $plan->explanation;
+      report "SKIP", $explanation;
+    }
+}
+
+sub handle_tap_bailout ($)
+{
+  my ($bailout, $msg) = ($_[0], "Bail out!");
+  $bailed_out = 1;
+  $msg .= " " . $bailout->explanation
+    unless is_null_string $bailout->explanation;
+  testsuite_error $msg;
+}
+
+sub extract_tap_comment ($)
+{
+  my $line = shift;
+  if (index ($line, $diag_string) == 0)
+    {
+      # Strip leading `$diag_string' from `$line'.
+      $line = substr ($line, length ($diag_string));
+      # And strip any leading and trailing whitespace left.
+      $line =~ s/(?:^\s*|\s*$)//g;
+      # Return what is left (if any).
+      return $line;
+    }
+  return "";
+}
+
+sub main (@)
+{
+  my $iterator = TAP::Parser::Iterator::Stream->new(\*STDIN);
+  my $parser = TAP::Parser->new ({iterator => $iterator });
+
+  while (defined (my $cur = $parser->next))
+    {
+      # Parsing of TAP input should stop after a "Bail out!" directive.
+      next if $bailed_out;
+
+      if ($cur->is_plan)
+        {
+          handle_tap_plan ($cur);
+        }
+      elsif ($cur->is_test)
+        {
+          handle_tap_result ($cur);
+        }
+      elsif ($cur->is_bailout)
+        {
+          handle_tap_bailout ($cur);
+        }
+      elsif ($cfg{"verbose"})
+        {
+          my $comment = extract_tap_comment ($cur->raw);
+          report "#", "$comment" if length $comment;
+       }
+    }
+  # A "Bail out!" directive should cause us to ignore any following TAP
+  # error, as well as a non-zero exit status from the TAP producer.
+  if (!$bailed_out)
+    {
+      if (!$plan_seen)
+        {
+          testsuite_error "missing test plan";
+        }
+      elsif ($parser->tests_planned != $parser->tests_run)
+        {
+          my ($planned, $run) = ($parser->tests_planned, $parser->tests_run);
+          my $bad_amount = $run > $planned ? "many" : "few";
+          testsuite_error (sprintf "too %s tests run (expected %d, got %d)",
+                                   $bad_amount, $planned, $run);
+        }
+    }
+}
+
+# ----------- #
+#  Main code. #
+# ----------- #
+
+main @ARGV;
+
+# Local Variables:
+# perl-indent-level: 2
+# perl-continued-statement-offset: 2
+# perl-continued-brace-offset: 0
+# perl-brace-offset: 0
+# perl-brace-imaginary-offset: 0
+# perl-label-offset: -2
+# cperl-indent-level: 2
+# cperl-brace-offset: 0
+# cperl-continued-brace-offset: 0
+# cperl-label-offset: -2
+# cperl-extra-newline-before-brace: t
+# cperl-merge-trailing-else: nil
+# cperl-continued-statement-offset: 2
+# eval: (add-hook 'write-file-hooks 'time-stamp)
+# time-stamp-start: "my $VERSION = "
+# time-stamp-format: "'%:y-%02m-%02d.%02H'"
+# time-stamp-time-zone: "UTC"
+# time-stamp-end: "; # UTC"
+# End:
diff --git a/scripts/tap-merge.pl b/scripts/tap-merge.pl
new file mode 100755
index 0000000..7e0de90
--- /dev/null
+++ b/scripts/tap-merge.pl
@@ -0,0 +1,108 @@
+#! /usr/bin/env perl
+# Copyright (C) 2018 Red Hat, Inc.
+#
+# Author: Paolo Bonzini <pbonzini@redhat.com>
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# ---------------------------------- #
+#  Imports, static data, and setup.  #
+# ---------------------------------- #
+
+use warnings FATAL => 'all';
+use strict;
+use Getopt::Long ();
+use TAP::Parser;
+
+my $ME = "tap-merge.pl";
+my $VERSION = "2018-11-29";
+
+my $HELP = "$ME: merge multiple TAP inputs from stdin.";
+
+# ----------------- #
+#  Option parsing.  #
+# ----------------- #
+
+Getopt::Long::GetOptions
+  (
+    'help' => sub { print $HELP; exit 0; },
+    'version' => sub { print "$ME $VERSION\n"; exit 0; },
+  );
+
+# -------------- #
+#  Subroutines.  #
+# -------------- #
+
+sub main ()
+{
+  my $iterator = TAP::Parser::Iterator::Stream->new(\*STDIN);
+  my $parser = TAP::Parser->new ({iterator => $iterator });
+  my $testno = 0;     # Number of test results seen so far.
+  my $bailed_out = 0; # Whether a "Bail out!" directive has been seen.
+
+  while (defined (my $cur = $parser->next))
+    {
+      if ($cur->is_bailout)
+        {
+          $bailed_out = 1;
+          next;
+        }
+      if ($cur->is_plan)
+        {
+          $bailed_out = 0;
+          next;
+        }
+      if ($cur->is_test)
+        {
+          $bailed_out = 0 if $cur->number == 1;
+          $testno++;
+          $cur = TAP::Parser::Result::Test->new({
+                          ok => $cur->ok,
+                          test_num => $testno,
+                          directive => $cur->directive,
+                          explanation => $cur->explanation,
+                          description => $cur->description
+                  });
+        }
+      print $cur->as_string . "\n" unless $bailed_out;
+    }
+  print "1..$testno\n";
+}
+
+# ----------- #
+#  Main code. #
+# ----------- #
+
+main;
+
+# Local Variables:
+# perl-indent-level: 2
+# perl-continued-statement-offset: 2
+# perl-continued-brace-offset: 0
+# perl-brace-offset: 0
+# perl-brace-imaginary-offset: 0
+# perl-label-offset: -2
+# cperl-indent-level: 2
+# cperl-brace-offset: 0
+# cperl-continued-brace-offset: 0
+# cperl-label-offset: -2
+# cperl-extra-newline-before-brace: t
+# cperl-merge-trailing-else: nil
+# cperl-continued-statement-offset: 2
+# eval: (add-hook 'write-file-hooks 'time-stamp)
+# time-stamp-start: "my $VERSION = "
+# time-stamp-format: "'%:y-%02m-%02d.%02H'"
+# time-stamp-time-zone: "UTC"
+# time-stamp-end: "; # UTC"
+# End:
diff --git a/tests/Makefile.include b/tests/Makefile.include
index fb0b449..d27e137 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_human = \
+	$(call quiet-command, \
+	  { $(foreach COMMAND, $1, \
+	    $(if $(V), echo $(notdir $(COMMAND)):;) \
+	    MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$(( $${RANDOM:-0} % 255 + 1))} \
+	      $2 $(COMMAND) -m=$(SPEED) -k $(if $(V), --tap | ./scripts/tap-driver.pl, \
+		   >/dev/null );) }, \
+	  "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)) /" ;) } \
+	      | $(SRC_PATH)/scripts/tap-merge.pl \
+	      $(if $(V), | tee "$@" | ./scripts/tap-driver.pl, >"$@"), \
+	  "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))
 
 .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,$^, \
+	  QTEST_QEMU_BINARY=$*-softmmu/qemu-system-$* \
+	  QTEST_QEMU_IMG=qemu-img$(EXESUF))
 
 # 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,$(SRC_PATH)/scripts/tap-merge.py $^ > $@,"GEN","$@")
 
 # Per guest TCG tests
 
diff --git a/tests/docker/dockerfiles/centos7.docker b/tests/docker/dockerfiles/centos7.docker
index 0a04bfb..e0f18f5 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 24b113b..c66e341 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 e05a9a9..514ab53 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 9a3d168..b30cbe7 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 5294563..cdc3f11 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 154ae2a..9561d4f 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 0c4eb9e..1d0e3dc 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 36e2b17..229add5 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 \
-- 
1.8.3.1

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

[Eric Blake]

On 11/29/18 11:45 AM, Paolo Bonzini wrote:
> gtester is deprecated by upstream glib and it does not support tests
> that call g_test_skip in some glib stable releases.
> 
> glib suggests instead using Automake's TAP support.  We do not support
> Automake, but we can copy the code that beautifies the TAP output and
> use it.  I chose to use the Perl copy rather than the shell/awk one,
> in order to reuse Perl's TAP parsing package, but I'm open to suggestions
> about which language to use.

I'm less familiar with the TAP protocol than I'd like to admit, but I 
did find:

https://blog.gtk.org/2018/07/11/news-from-glib-2-58/

which corroborates your claim that switching to Automake's TAP parser is 
indeed a recommended approach to avoid the now-deprecated gtester.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 1/2] test: execute g_test_run when tests are skipped

[Eric Blake]

On 11/29/18 11:45 AM, Paolo Bonzini wrote:
> Sometimes a test's main() function recognizes that the environment
> does not support the test, and therefore exits.  In this case, we
> still should run g_test_run() so that a TAP harness will print the
> test plan ("1..0") and the test will be marked as skipped.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   tests/cdrom-test.c     | 2 +-
>   tests/migration-test.c | 8 ++++----
>   2 files changed, 5 insertions(+), 5 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

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

[Eric Blake]

On 11/29/18 11:45 AM, Paolo Bonzini wrote:
> gtester is deprecated by upstream glib and it does not support tests
> that call g_test_skip in some glib stable releases.
> 
> glib suggests instead using Automake's TAP support.  We do not support
> Automake, but we can copy the code that beautifies the TAP output and
> use it.  I chose to use the Perl copy rather than the shell/awk one,
> in order to reuse Perl's TAP parsing package, but I'm open to suggestions
> about which language to use.

Maybe a reference to a URL documenting the glib deprecation would be in 
order? I found https://blog.gtk.org/2018/07/11/news-from-glib-2-58/

> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---

> +++ b/scripts/tap-driver.pl
> @@ -0,0 +1,386 @@
> +#! /usr/bin/env perl
> +# Copyright (C) 2011-2013 Free Software Foundation, Inc.

This is not the latest version of automake.git/contrib/tap-driver.pl - 
which Automake version is it from?  Automake moved its perl driver out 
to pasture in 2013, stating that the awk+shell version is preferred in 
new automake projects.  I don't have a strong preference for which one 
you pick, but do worry that if automake adds future enhancements to the 
awk+shell, then the perl version won't keep up and we'll be stuck 
redoing things again in a few years.  On the other hand, TAP doesn't 
seem to be gaining new features at a very fast rate.

> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.

Among other things, the most recent version of tap-driver.pl switched 
all references to https://.

> +
> +# As a special exception to the GNU General Public License, if you
> +# distribute this file as part of a program that contains a
> +# configuration script generated by Autoconf, you may include it under
> +# the same distribution terms that you use for the rest of that program.

We do not use Autoconf, so this exception does not apply to our use of 
this file.  But since our project is GPL, I don't see including this 
file as a problem, nor do I find any problem with leaving the exception 
in place.

> +
> +my $VERSION = '2012-02-01.19'; # UTC

Okay, so this matches the version in automake.git commit 6b819187, which 
was shipped in automake 1.11b.  Six years old, but still does what we 
need, at least for today.

Otherwise, looks good to me, although I'm weak enough with TAP in 
general that this is a fairly weak:
Reviewed-by: Eric Blake <eblake@redhat.com>

(The real test is if all the CI systems are happy with it...)

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

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

[Paolo Bonzini]

On 29/11/18 22:06, Eric Blake wrote:
> On 11/29/18 11:45 AM, Paolo Bonzini wrote:
>> gtester is deprecated by upstream glib and it does not support tests
>> that call g_test_skip in some glib stable releases.
>>
>> glib suggests instead using Automake's TAP support.  We do not support
>> Automake, but we can copy the code that beautifies the TAP output and
>> use it.  I chose to use the Perl copy rather than the shell/awk one,
>> in order to reuse Perl's TAP parsing package, but I'm open to suggestions
>> about which language to use.
> 
> Maybe a reference to a URL documenting the glib deprecation would be in
> order? I found https://blog.gtk.org/2018/07/11/news-from-glib-2-58/

Good idea.

>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
> 
>> +++ b/scripts/tap-driver.pl
>> @@ -0,0 +1,386 @@
>> +#! /usr/bin/env perl
>> +# Copyright (C) 2011-2013 Free Software Foundation, Inc.
> 
> This is not the latest version of automake.git/contrib/tap-driver.pl -
> which Automake version is it from?  Automake moved its perl driver out
> to pasture in 2013, stating that the awk+shell version is preferred in
> new automake projects.  I don't have a strong preference for which one
> you pick, but do worry that if automake adds future enhancements to the
> awk+shell, then the perl version won't keep up and we'll be stuck
> redoing things again in a few years.  On the other hand, TAP doesn't
> seem to be gaining new features at a very fast rate.

It does get some new features from time to time, and that's why I
preferred an external parser to a home-grown one---especially if we
would need to duplicate the awk parser in tap-merge.pl.

The version I used is just the one in the machine where I developed it
(RHEL 7), but there have been no relevant changes after that commit, so
it's not important.

>> +# You should have received a copy of the GNU General Public License
>> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> 
> Among other things, the most recent version of tap-driver.pl switched
> all references to https://.

I can change this too.

>> +# As a special exception to the GNU General Public License, if you
>> +# distribute this file as part of a program that contains a
>> +# configuration script generated by Autoconf, you may include it under
>> +# the same distribution terms that you use for the rest of that program.
> 
> We do not use Autoconf, so this exception does not apply to our use of
> this file.  But since our project is GPL, I don't see including this
> file as a problem, nor do I find any problem with leaving the exception
> in place.

Yes, I find removing exceptions to be a bit unkind.

Paolo

Re: [Qemu-devel] [PATCH 1/2] test: execute g_test_run when tests are skipped

[Thomas Huth]

On 2018-11-29 18:45, Paolo Bonzini wrote:
> Sometimes a test's main() function recognizes that the environment
> does not support the test, and therefore exits.  In this case, we
> still should run g_test_run() so that a TAP harness will print the
> test plan ("1..0") and the test will be marked as skipped.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  tests/cdrom-test.c     | 2 +-
>  tests/migration-test.c | 8 ++++----
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/cdrom-test.c b/tests/cdrom-test.c
> index 9b43dc9..14bd981 100644
> --- a/tests/cdrom-test.c
> +++ b/tests/cdrom-test.c
> @@ -169,7 +169,7 @@ int main(int argc, char **argv)
>  
>      if (exec_genisoimg(genisocheck)) {
>          /* genisoimage not available - so can't run tests */
> -        return 0;
> +        return g_test_run();
>      }
>  
>      ret = prepare_image(arch, isoimage);
> diff --git a/tests/migration-test.c b/tests/migration-test.c
> index 06ca506..8352612 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -789,7 +789,7 @@ int main(int argc, char **argv)
>      g_test_init(&argc, &argv, NULL);
>  
>      if (!ufd_version_check()) {
> -        return 0;
> +        return g_test_run();
>      }
>  
>      /*
> @@ -800,7 +800,7 @@ int main(int argc, char **argv)
>      if (g_str_equal(qtest_get_arch(), "ppc64") &&
>          access("/sys/module/kvm_hv", F_OK)) {
>          g_test_message("Skipping test: kvm_hv not available");
> -        return 0;
> +        return g_test_run();
>      }
>  
>      /*
> @@ -811,11 +811,11 @@ int main(int argc, char **argv)
>  #if defined(HOST_S390X)
>          if (access("/dev/kvm", R_OK | W_OK)) {
>              g_test_message("Skipping test: kvm not available");
> -            return 0;
> +            return g_test_run();
>          }
>  #else
>          g_test_message("Skipping test: Need s390x host to work properly");
> -        return 0;
> +        return g_test_run();
>  #endif
>      }

Reviewed-by: Thomas Huth <thuth@redhat.com>

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

[Thomas Huth]

On 2018-11-29 21:43, Eric Blake wrote:
> On 11/29/18 11:45 AM, Paolo Bonzini wrote:
>> gtester is deprecated by upstream glib and it does not support tests
>> that call g_test_skip in some glib stable releases.
>>
>> glib suggests instead using Automake's TAP support.  We do not support
>> Automake, but we can copy the code that beautifies the TAP output and
>> use it.  I chose to use the Perl copy rather than the shell/awk one,
>> in order to reuse Perl's TAP parsing package, but I'm open to suggestions
>> about which language to use.
> 
> I'm less familiar with the TAP protocol than I'd like to admit, but I
> did find:
> 
> https://blog.gtk.org/2018/07/11/news-from-glib-2-58/
> 
> which corroborates your claim that switching to Automake's TAP parser is
> indeed a recommended approach to avoid the now-deprecated gtester.

Hmm, do we really have to maintain our own version of a test runner now?
There's already some effort going on to use avocado in "make
check-acceptance" ... maybe we could use avocado as replacement for
gtester, too?

 Thomas

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

[Daniel P. Berrangé]

On Thu, Nov 29, 2018 at 02:43:03PM -0600, Eric Blake wrote:
> On 11/29/18 11:45 AM, Paolo Bonzini wrote:
> > gtester is deprecated by upstream glib and it does not support tests
> > that call g_test_skip in some glib stable releases.
> > 
> > glib suggests instead using Automake's TAP support.  We do not support
> > Automake, but we can copy the code that beautifies the TAP output and
> > use it.  I chose to use the Perl copy rather than the shell/awk one,
> > in order to reuse Perl's TAP parsing package, but I'm open to suggestions
> > about which language to use.
> 
> I'm less familiar with the TAP protocol than I'd like to admit, but I did
> find:
> 
> https://blog.gtk.org/2018/07/11/news-from-glib-2-58/
> 
> which corroborates your claim that switching to Automake's TAP parser is
> indeed a recommended approach to avoid the now-deprecated gtester.

FYI, for docs about the TAP protocol see:

   https://testanything.org/tap-specification.html

I've always liked the TAP protocol approach to test output where you print
an ok/not ok  status for each piece of the test, as opposed to assertion
style tests where you just abort at the first piece that fails. 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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

[Paolo Bonzini]

On 30/11/18 10:54, Daniel P. Berrangé wrote:
> FYI, for docs about the TAP protocol see:
> 
>    https://testanything.org/tap-specification.html
> 
> I've always liked the TAP protocol approach to test output where you print
> an ok/not ok  status for each piece of the test, as opposed to assertion
> style tests where you just abort at the first piece that fails. 

FWIW, this is not what this series does.  Each ok/not ok line
corresponds to one assertion-style test.

Paolo

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

[Paolo Bonzini]

On 30/11/18 08:21, Thomas Huth wrote:
> On 2018-11-29 21:43, Eric Blake wrote:
>> On 11/29/18 11:45 AM, Paolo Bonzini wrote:
>>> gtester is deprecated by upstream glib and it does not support tests
>>> that call g_test_skip in some glib stable releases.
>>>
>>> glib suggests instead using Automake's TAP support.  We do not support
>>> Automake, but we can copy the code that beautifies the TAP output and
>>> use it.  I chose to use the Perl copy rather than the shell/awk one,
>>> in order to reuse Perl's TAP parsing package, but I'm open to suggestions
>>> about which language to use.
>>
>> I'm less familiar with the TAP protocol than I'd like to admit, but I
>> did find:
>>
>> https://blog.gtk.org/2018/07/11/news-from-glib-2-58/
>>
>> which corroborates your claim that switching to Automake's TAP parser is
>> indeed a recommended approach to avoid the now-deprecated gtester.
> 
> Hmm, do we really have to maintain our own version of a test runner now?

Well, that's what you get for using a custom build system but it's
"just" an output beautifier really.  I could just "grep -ve ^# -e ^ok"
but it wouldn't look very nice (plus things that start as 1-line shell
scripts end up being the same as the thing you're trying to replace).

I don't think it will be a big maintenance headache---just like most
other files in scripts/, some of which have had exactly zero commits
since they were added.  In fact, Automake has hardly changed its driver
since the first commit.

> There's already some effort going on to use avocado in "make
> check-acceptance" ... maybe we could use avocado as replacement for
> gtester, too?

Avocado does not have the required functionality, unfortunately.  It can
produce TAP, but not consume it.  Also, Python libraries for TAP do not
abound and are generally not shipped with the OS (apart from Perl, which
basically invented TAP, the language with the most TAP libraries seem to
be JavaScript!).

Paolo

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

[Daniel P. Berrangé]

On Fri, Nov 30, 2018 at 10:56:01AM +0100, Paolo Bonzini wrote:
> On 30/11/18 10:54, Daniel P. Berrangé wrote:
> > FYI, for docs about the TAP protocol see:
> > 
> >    https://testanything.org/tap-specification.html
> > 
> > I've always liked the TAP protocol approach to test output where you print
> > an ok/not ok  status for each piece of the test, as opposed to assertion
> > style tests where you just abort at the first piece that fails. 
> 
> FWIW, this is not what this series does.  Each ok/not ok line
> corresponds to one assertion-style test.

Oh sure, that's the way they've integrated it into the glib test APIs.
I just meant that in terms of the output format, I like its flexibility
in this manner.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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

[Cleber Rosa]

On 11/30/18 4:58 AM, Paolo Bonzini wrote:
> On 30/11/18 08:21, Thomas Huth wrote:
>> On 2018-11-29 21:43, Eric Blake wrote:
>>> On 11/29/18 11:45 AM, Paolo Bonzini wrote:
>>>> gtester is deprecated by upstream glib and it does not support tests
>>>> that call g_test_skip in some glib stable releases.
>>>>
>>>> glib suggests instead using Automake's TAP support.  We do not support
>>>> Automake, but we can copy the code that beautifies the TAP output and
>>>> use it.  I chose to use the Perl copy rather than the shell/awk one,
>>>> in order to reuse Perl's TAP parsing package, but I'm open to suggestions
>>>> about which language to use.
>>>
>>> I'm less familiar with the TAP protocol than I'd like to admit, but I
>>> did find:
>>>
>>> https://blog.gtk.org/2018/07/11/news-from-glib-2-58/
>>>
>>> which corroborates your claim that switching to Automake's TAP parser is
>>> indeed a recommended approach to avoid the now-deprecated gtester.
>>
>> Hmm, do we really have to maintain our own version of a test runner now?
> 
> Well, that's what you get for using a custom build system but it's
> "just" an output beautifier really.  I could just "grep -ve ^# -e ^ok"
> but it wouldn't look very nice (plus things that start as 1-line shell
> scripts end up being the same as the thing you're trying to replace).
> 
> I don't think it will be a big maintenance headache---just like most
> other files in scripts/, some of which have had exactly zero commits
> since they were added.  In fact, Automake has hardly changed its driver
> since the first commit.
> 
>> There's already some effort going on to use avocado in "make
>> check-acceptance" ... maybe we could use avocado as replacement for
>> gtester, too?
> 
> Avocado does not have the required functionality, unfortunately.  It can
> produce TAP, but not consume it.  Also, Python libraries for TAP do not
> abound and are generally not shipped with the OS (apart from Perl, which
> basically invented TAP, the language with the most TAP libraries seem to
> be JavaScript!).
> 

Avocado can't currently consume TAP (it produces it, though), but it has
some integration with glib based tests:

https://avocado-framework.readthedocs.io/en/66.0/optional_plugins/glib.html

But, there may be bits missing to fully replae gtests.  Given that we
have been adding features to Avocado based on QEMU requirements, we can
certainly look at adding missing pieces there.  If this sounds like
something you'd be interested in, I can come up with an integration
proposal.

> Paolo
> 

Cheers,
- Cleber.

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

[Thomas Huth]

On 2018-11-30 15:47, Cleber Rosa wrote:
> 
> 
> On 11/30/18 4:58 AM, Paolo Bonzini wrote:
>> On 30/11/18 08:21, Thomas Huth wrote:
>>> On 2018-11-29 21:43, Eric Blake wrote:
>>>> On 11/29/18 11:45 AM, Paolo Bonzini wrote:
>>>>> gtester is deprecated by upstream glib and it does not support tests
>>>>> that call g_test_skip in some glib stable releases.
>>>>>
>>>>> glib suggests instead using Automake's TAP support.  We do not support
>>>>> Automake, but we can copy the code that beautifies the TAP output and
>>>>> use it.  I chose to use the Perl copy rather than the shell/awk one,
>>>>> in order to reuse Perl's TAP parsing package, but I'm open to suggestions
>>>>> about which language to use.
>>>>
>>>> I'm less familiar with the TAP protocol than I'd like to admit, but I
>>>> did find:
>>>>
>>>> https://blog.gtk.org/2018/07/11/news-from-glib-2-58/
>>>>
>>>> which corroborates your claim that switching to Automake's TAP parser is
>>>> indeed a recommended approach to avoid the now-deprecated gtester.
>>>
>>> Hmm, do we really have to maintain our own version of a test runner now?
>>
>> Well, that's what you get for using a custom build system but it's
>> "just" an output beautifier really.  I could just "grep -ve ^# -e ^ok"
>> but it wouldn't look very nice (plus things that start as 1-line shell
>> scripts end up being the same as the thing you're trying to replace).
>>
>> I don't think it will be a big maintenance headache---just like most
>> other files in scripts/, some of which have had exactly zero commits
>> since they were added.  In fact, Automake has hardly changed its driver
>> since the first commit.

OK, fair.

>>> There's already some effort going on to use avocado in "make
>>> check-acceptance" ... maybe we could use avocado as replacement for
>>> gtester, too?
>>
>> Avocado does not have the required functionality, unfortunately.  It can
>> produce TAP, but not consume it.  Also, Python libraries for TAP do not
>> abound and are generally not shipped with the OS (apart from Perl, which
>> basically invented TAP, the language with the most TAP libraries seem to
>> be JavaScript!).
> 
> Avocado can't currently consume TAP (it produces it, though), but it has
> some integration with glib based tests:
> 
> https://avocado-framework.readthedocs.io/en/66.0/optional_plugins/glib.html
> 
> But, there may be bits missing to fully replae gtests.  Given that we
> have been adding features to Avocado based on QEMU requirements, we can
> certainly look at adding missing pieces there.  If this sounds like
> something you'd be interested in, I can come up with an integration
> proposal.

FWIW, it seems basically to work, indeed:

$ export QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64
$ avocado run --loaders glib -- tests/pxe-test tests/cdrom-test
JOB ID     : 67657631c2f1f80e34d90fe0c2c05e05aab6f1ba
JOB LOG    : /home/thuth/avocado/job-results/job-2018-11-30T10.02-6765763/job.log
 (01/12) tests/pxe-test:/x86_64/pxe/ipv4/pc/e1000: PASS (6.46 s)
 (02/12) tests/pxe-test:/x86_64/pxe/ipv4/pc/virtio-net-pci: PASS (6.47 s)
 (03/12) tests/pxe-test:/x86_64/pxe/ipv4/q35/e1000e: PASS (6.46 s)
 (04/12) tests/pxe-test:/x86_64/pxe/ipv4/q35/virtio-net-pci: PASS (6.46 s)
 (05/12) tests/cdrom-test:/x86_64/cdrom/boot/default: PASS (0.25 s)
 (06/12) tests/cdrom-test:/x86_64/cdrom/boot/virtio-scsi: PASS (0.25 s)
 (07/12) tests/cdrom-test:/x86_64/cdrom/boot/isapc: PASS (0.24 s)
 (08/12) tests/cdrom-test:/x86_64/cdrom/boot/am53c974: PASS (0.25 s)
 (09/12) tests/cdrom-test:/x86_64/cdrom/boot/dc390: PASS (0.25 s)
 (10/12) tests/cdrom-test:/x86_64/cdrom/boot/lsi53c895a: PASS (0.26 s)
 (11/12) tests/cdrom-test:/x86_64/cdrom/boot/megasas: PASS (0.15 s)
 (12/12) tests/cdrom-test:/x86_64/cdrom/boot/megasas-gen2: PASS (0.15 s)
RESULTS    : PASS 12 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
JOB TIME   : 28.31 s
JOB HTML   : /home/thuth/avocado/job-results/job-2018-11-30T10.02-6765763/results.html

 Thomas

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

[Daniel P. Berrangé]

On Thu, Nov 29, 2018 at 06:45:31PM +0100, Paolo Bonzini wrote:
> gtester is deprecated by upstream glib and it does not support tests
> that call g_test_skip in some glib stable releases.
> 
> glib suggests instead using Automake's TAP support.  We do not support
> Automake, but we can copy the code that beautifies the TAP output and
> use it.  I chose to use the Perl copy rather than the shell/awk one,
> in order to reuse Perl's TAP parsing package, but I'm open to suggestions
> about which language to use.

Do we even need the beutifier ?

IIUC, this output is only seen when you with 'make check' passing V=1,
so most people won't see it.

When V=1 is set, it just changes:

$ ./tests/test-io-channel-socket -k --tap
# random seed: R02Sb9d02d2a9658ae1be211540f5ceebc01
1..9
# Start of io tests
# Start of channel tests
# Start of socket tests
ok 1 /io/channel/socket/ipv4-sync
ok 2 /io/channel/socket/ipv4-async
ok 3 /io/channel/socket/ipv4-fd
ok 4 /io/channel/socket/ipv6-sync
ok 5 /io/channel/socket/ipv6-async
ok 6 /io/channel/socket/unix-sync
ok 7 /io/channel/socket/unix-async
**
ERROR:tests/test-io-channel-socket.c:424:test_io_channel_unix_fd_pass: assertion failed: (0)
Bail out! ERROR:tests/test-io-channel-socket.c:424:test_io_channel_unix_fd_pass: assertion failed: (0)
Aborted (core dumped)


To look like:

$ ./tests/test-io-channel-socket -k --tap | ./scripts/tap-driver.pl 
PASS 1 /io/channel/socket/ipv4-sync
PASS 2 /io/channel/socket/ipv4-async
PASS 3 /io/channel/socket/ipv4-fd
PASS 4 /io/channel/socket/ipv6-sync
PASS 5 /io/channel/socket/ipv6-async
PASS 6 /io/channel/socket/unix-sync
PASS 7 /io/channel/socket/unix-async
ERROR - Bail out! ERROR:tests/test-io-channel-socket.c:424:test_io_channel_unix_fd_pass: assertion failed: (0)

where 'PASS' is coloured in green and 'ERROR' is red. I'm not
convinced this change justifies having the tap-driver.pl script

Since this commit already proposes a dep on the perl-Test-Harness
package, the host will have the 'prove' command, which can be used
for invoking tests.

Prove just needs a wrapper to know how to invoke glib tests:

  $ cat glibharness.sh 
  #!/bin/sh

  exec "$@" -k --tap

Now you can run :

$ prove -v -e ./glibharness.sh  tests/test-io-channel-socket
tests/test-io-channel-socket .. 
# random seed: R02S7b47ca0de1bc27cbbcf22dcc130c2b7f
1..9
# Start of io tests
# Start of channel tests
# Start of socket tests
ok 1 /io/channel/socket/ipv4-sync
ok 2 /io/channel/socket/ipv4-async
ok 3 /io/channel/socket/ipv4-fd
ok 4 /io/channel/socket/ipv6-sync
ok 5 /io/channel/socket/ipv6-async
ok 6 /io/channel/socket/unix-sync
ok 7 /io/channel/socket/unix-async
**
ERROR:tests/test-io-channel-socket.c:424:test_io_channel_unix_fd_pass: assertion failed: (0)
Bailout called.  Further testing stopped:  ERROR:tests/test-io-channel-socket.c:424:test_io_channel_unix_fd_pass: assertion failed: (0)
Bail out! ERROR:tests/test-io-channel-socket.c:424:test_io_channel_unix_fd_pass: assertion failed: (0)
FAILED--Further testing stopped: ERROR:tests/test-io-channel-socket.c:424:test_io_channel_unix_fd_pass: assertion failed: (0)


Where the failure line is now coloured in Red (or sucess is coloured
in green)

prove can also run tests in batches, giving progress of execution of
each test as it runs

$ prove  -e ./glibharness.sh  tests/test-io-channel-{buffer,command,file,socket,tls} 
tests/test-io-channel-buffer ... ok   
tests/test-io-channel-command .. ok   
tests/test-io-channel-file ..... ok   
tests/test-io-channel-socket ... 1/? **
ERROR:tests/test-io-channel-socket.c:424:test_io_channel_unix_fd_pass: assertion failed: (0)
Bailout called.  Further testing stopped:  ERROR:tests/test-io-channel-socket.c:424:test_io_channel_unix_fd_pass: assertion failed: (0)
FAILED--Further testing stopped: ERROR:tests/test-io-channel-socket.c:424:test_io_channel_unix_fd_pass: assertion failed: (0)


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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

[Paolo Bonzini]

On 30/11/18 16:50, Daniel P. Berrangé wrote:
> IIUC, this output is only seen when you with 'make check' passing V=1,
> so most people won't see it.

V=1 should be enabled by default on Patchew, so it will be visible in
the CI logs.  But anyway that's minor, there are more reasons why a
custom beautifier is better than prove.

First, at the very least, failures should be included in the output even
if V=1 is not provided.  Therefore, for failing tests we'd have:

  $ scripts/tap-driver.pl < f.tap
  FAIL 2 bbb
  XPASS 3 ccc # TODO
  FAIL 6 fff # SKIP cannot find frobnicator

And the output of prove in that case would be inferior for developers,
unless you use "V=1":

  $ prove -e cat f.tap
  f.tap .. Failed 2/6 subtests
	(less 2 skipped subtests: 2 okay)
	(1 TODO test unexpectedly succeeded)

  Test Summary Report
  -------------------
  f.tap (Wstat: 0 Tests: 6 Failed: 2)
    Failed tests:  2, 6
    TODO passed:   3
  Files=1, Tests=6,  0 wallclock secs ( 0.02 usr +  0.00 sys =  0.02 CPU)
  Result: FAIL

where you don't really know what tests 2 and 6 are without looking at
the TAP output.  The part before the summary provides a nice progress
report, as you point out, but it adds a bunch of noise with respect to
skipped subtests (skipped tests will be more common once patch 1 is
merged; qgraph also adds some) that I personally dislike.

You can hide it with -Q, but then you cannot invoke anymore more than
one test with a single invocation of prove, and the Makefile output
becomes much bigger:

   TEST    check-foo

vs.

   TEST    check-foo
  All tests successful.
  Files=1, Tests=3,  0 wallclock secs ( 0.01 usr +  0.00 sys =  0.01 CPU)
  Result: PASS

There are two other smaller "philosophical" differences.  First, prove
treats XPASS as passes, while tap-driver.pl treats them as failures.
Second, skipped tests really stand out in the "prove -v" output, with
white-on-blue text.

I'd rather avoid getting into the business of beautifying the output of
prove.

Paolo

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

[Paolo Bonzini]

On 01/07/19 16:42, Philippe Mathieu-Daudé wrote:
> $ 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?

Yes, we can just add $GTESTER_OPTIONS to the invocation of the test
executables.

Paolo

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 <pbonzini@redhat.com>
> ---
> 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 \
> 

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

[Paolo Bonzini]

On 11/02/19 15:32, Kevin Wolf wrote:
> $(patsubst %, check-%, $(check-unit-y) $(check-speed-y)): check-%: %
>     $(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda,)
>     $(call quiet-command, \
>         MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$(( $${RANDOM:-0} % 255 + 1))} \
>         gtester $(GTESTER_OPTIONS) -m=$(SPEED) $*,"GTESTER","$*")
>     $(if $(CONFIG_GCOV),@for f in $(gcov-files-$(subst tests/,,$*)-y) $(gcov-files-generic-y); do \
>       echo Gcov report for $$f:;\
>       $(GCOV) $(GCOV_OPTIONS) $$f -o `dirname $$f`; \
>     done,)
> 
> So it used quiet-command (but I'm okay with changing this) and it set
> MALLOC_PERTURB_ (I think this would still be nice to have).

Yes, of course.  I'll send a real patch.

Paolo

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

[Kevin Wolf]

Am 08.02.2019 um 18:16 hat Paolo Bonzini geschrieben:
> On 08/02/19 17:00, Kevin Wolf wrote:
> > Am 08.02.2019 um 14:46 hat Paolo Bonzini geschrieben:
> >> On 08/02/19 13:48, Kevin Wolf wrote:
> >>> I just wanted to work on a unit test and found that a simple 'make
> >>> check-tests/test-bdrv-drain' (which used to build and run the test with
> >>> a single command) doesn't work any more.
> >>>
> >>> git bisect points to this commit.
> >>
> >> What's wrong with "tests/test-bdrv-drain"?  (Before there were some
> >> differences in test environment, but not now).
> > 
> > It runs the old version without building the source I just modified.
> 
> Oh.  Since it wasn't documented, it seemed to be just an implementation
> detail rather than something that would be used by people...  So
> basically something like this?
> 
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 048cf5639c..6ac3f2c7eb 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -861,6 +861,9 @@ check-unit: $(check-unit-y)
>  check-speed: $(check-speed-y)
>  	$(call do_test_human, $^)
> 
> +$(patsubst %, check-%, $(check-unit-y) $(check-speed-y): check-%: %
> +        $<

Apart from the missing closing bracket. :-)

The old rule seems to have been like this:

.PHONY: $(patsubst %, check-%, $(check-unit-y) $(check-speed-y))
$(patsubst %, check-%, $(check-unit-y) $(check-speed-y)): check-%: %
    $(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda,)
    $(call quiet-command, \
        MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$(( $${RANDOM:-0} % 255 + 1))} \
        gtester $(GTESTER_OPTIONS) -m=$(SPEED) $*,"GTESTER","$*")
    $(if $(CONFIG_GCOV),@for f in $(gcov-files-$(subst tests/,,$*)-y) $(gcov-files-generic-y); do \
      echo Gcov report for $$f:;\
      $(GCOV) $(GCOV_OPTIONS) $$f -o `dirname $$f`; \
    done,)

So it used quiet-command (but I'm okay with changing this) and it set
MALLOC_PERTURB_ (I think this would still be nice to have). Not sure how
relevant the gcov stuff is, I never used this.

I think the minimal change for this commit would have been to just
remove 'gtester $(GTESTER_OPTIONS) -m=$(SPEED)' and leave everything
else in place.

Kevin

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

[Paolo Bonzini]

On 08/02/19 17:00, Kevin Wolf wrote:
> Am 08.02.2019 um 14:46 hat Paolo Bonzini geschrieben:
>> On 08/02/19 13:48, Kevin Wolf wrote:
>>> I just wanted to work on a unit test and found that a simple 'make
>>> check-tests/test-bdrv-drain' (which used to build and run the test with
>>> a single command) doesn't work any more.
>>>
>>> git bisect points to this commit.
>>
>> What's wrong with "tests/test-bdrv-drain"?  (Before there were some
>> differences in test environment, but not now).
> 
> It runs the old version without building the source I just modified.

Oh.  Since it wasn't documented, it seemed to be just an implementation
detail rather than something that would be used by people...  So
basically something like this?

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 048cf5639c..6ac3f2c7eb 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -861,6 +861,9 @@ check-unit: $(check-unit-y)
 check-speed: $(check-speed-y)
 	$(call do_test_human, $^)

+$(patsubst %, check-%, $(check-unit-y) $(check-speed-y): check-%: %
+        $<
+
 # gtester tests with TAP output

 $(patsubst %, check-report-qtest-%.tap, $(QTEST_TARGETS)):
check-report-qtest-%.tap: $(check-qtest-y)


Paolo

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

[Kevin Wolf]

Am 08.02.2019 um 14:46 hat Paolo Bonzini geschrieben:
> On 08/02/19 13:48, Kevin Wolf wrote:
> > I just wanted to work on a unit test and found that a simple 'make
> > check-tests/test-bdrv-drain' (which used to build and run the test with
> > a single command) doesn't work any more.
> > 
> > git bisect points to this commit.
> 
> What's wrong with "tests/test-bdrv-drain"?  (Before there were some
> differences in test environment, but not now).

It runs the old version without building the source I just modified.

Kevin

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

[Paolo Bonzini]

On 08/02/19 13:48, Kevin Wolf wrote:
> I just wanted to work on a unit test and found that a simple 'make
> check-tests/test-bdrv-drain' (which used to build and run the test with
> a single command) doesn't work any more.
> 
> git bisect points to this commit.

What's wrong with "tests/test-bdrv-drain"?  (Before there were some
differences in test environment, but not now).

Paolo

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

[Kevin Wolf]

Am 06.12.2018 um 22:50 hat Paolo Bonzini geschrieben:
> 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 <pbonzini@redhat.com>

I just wanted to work on a unit test and found that a simple 'make
check-tests/test-bdrv-drain' (which used to build and run the test with
a single command) doesn't work any more.

git bisect points to this commit.

Kevin

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

[Thomas Huth]

On 2018-12-06 22:50, 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 <pbonzini@redhat.com>
> ---
> 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

I gave it a quick try, and it seems to work fine. And I guess it's also
the best self-contained option that we currently have, so:

Acked-by: Thomas Huth <thuth@redhat.com>
Tested-by: Thomas Huth <thuth@redhat.com>

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

[Paolo Bonzini]

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 <pbonzini@redhat.com>
---
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

 scripts/gtester-cat                          |  26 --
 scripts/tap-driver.pl                        | 378 +++++++++++++++++++
 scripts/tap-merge.pl                         | 110 ++++++
 tests/Makefile.include                       |  54 +--
 tests/docker/dockerfiles/centos7.docker      |   1 +
 tests/docker/dockerfiles/debian-amd64.docker |   1 +
 tests/docker/dockerfiles/debian-ports.docker |   1 +
 tests/docker/dockerfiles/debian-sid.docker   |   1 +
 tests/docker/dockerfiles/debian8.docker      |   1 +
 tests/docker/dockerfiles/debian9.docker      |   1 +
 tests/docker/dockerfiles/fedora.docker       |   1 +
 tests/docker/dockerfiles/ubuntu.docker       |   1 +
 12 files changed, 529 insertions(+), 47 deletions(-)
 delete mode 100755 scripts/gtester-cat
 create mode 100755 scripts/tap-driver.pl
 create mode 100755 scripts/tap-merge.pl

diff --git a/scripts/gtester-cat b/scripts/gtester-cat
deleted file mode 100755
index 061a952cad..0000000000
--- a/scripts/gtester-cat
+++ /dev/null
@@ -1,26 +0,0 @@
-#!/bin/sh
-#
-# Copyright IBM, Corp. 2012
-#
-# Authors:
-#  Anthony Liguori <aliguori@us.ibm.com>
-#
-# This work is licensed under the terms of the GNU GPLv2 or later.
-# See the COPYING file in the top-level directory.
-
-cat <<EOF
-<?xml version="1.0"?>
-<gtester>
- <info>
-  <package>qemu</package>
-  <version>0.0</version>
-  <revision>rev</revision>
- </info>
-EOF
-
-sed \
-  -e '/<?xml/d' \
-  -e '/^<gtester>$/d' \
-  -e '/<info>/,/<\/info>/d' \
-  -e '$b' \
-  -e '/^<\/gtester>$/d' "$@"
diff --git a/scripts/tap-driver.pl b/scripts/tap-driver.pl
new file mode 100755
index 0000000000..5e59b5db49
--- /dev/null
+++ b/scripts/tap-driver.pl
@@ -0,0 +1,378 @@
+#! /usr/bin/env perl
+# Copyright (C) 2011-2013 Free Software Foundation, Inc.
+# Copyright (C) 2018 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <https://www.gnu.org/licenses/>.
+
+# As a special exception to the GNU General Public License, if you
+# distribute this file as part of a program that contains a
+# configuration script generated by Autoconf, you may include it under
+# the same distribution terms that you use for the rest of that program.
+
+# ---------------------------------- #
+#  Imports, static data, and setup.  #
+# ---------------------------------- #
+
+use warnings FATAL => 'all';
+use strict;
+use Getopt::Long ();
+use TAP::Parser;
+use Term::ANSIColor qw(:constants);
+
+my $ME = "tap-driver.pl";
+my $VERSION = "2018-11-30";
+
+my $USAGE = <<'END';
+Usage:
+  tap-driver [--test-name=TEST] [--color={always|never|auto}]
+             [--verbose] [--show-failures-only]
+END
+
+my $HELP = "$ME: TAP-aware test driver for QEMU testsuite harness." .
+           "\n" . $USAGE;
+
+# It's important that NO_PLAN evaluates "false" as a boolean.
+use constant NO_PLAN => 0;
+use constant EARLY_PLAN => 1;
+use constant LATE_PLAN => 2;
+
+use constant DIAG_STRING => "#";
+
+# ------------------- #
+#  Global variables.  #
+# ------------------- #
+
+my $testno = 0;     # Number of test results seen so far.
+my $bailed_out = 0; # Whether a "Bail out!" directive has been seen.
+my $failed = 0;     # Final exit code
+
+# Whether the TAP plan has been seen or not, and if yes, which kind
+# it is ("early" is seen before any test result, "late" otherwise).
+my $plan_seen = NO_PLAN;
+
+# ----------------- #
+#  Option parsing.  #
+# ----------------- #
+
+my %cfg = (
+  "color" => 0,
+  "verbose" => 0,
+  "show-failures-only" => 0,
+);
+
+my $color = "auto";
+my $test_name = undef;
+
+# Perl's Getopt::Long allows options to take optional arguments after a space.
+# Prevent --color by itself from consuming other arguments
+foreach (@ARGV) {
+  if ($_ eq "--color" || $_ eq "-color") {
+    $_ = "--color=$color";
+  }
+}
+
+Getopt::Long::GetOptions
+  (
+    'help' => sub { print $HELP; exit 0; },
+    'version' => sub { print "$ME $VERSION\n"; exit 0; },
+    'test-name=s' => \$test_name,
+    'color=s'  => \$color,
+    'show-failures-only' => sub { $cfg{"show-failures-only"} = 1; },
+    'verbose' => sub { $cfg{"verbose"} = 1; },
+  ) or exit 1;
+
+if ($color =~ /^always$/i) {
+  $cfg{'color'} = 1;
+} elsif ($color =~ /^never$/i) {
+  $cfg{'color'} = 0;
+} elsif ($color =~ /^auto$/i) {
+  $cfg{'color'} = (-t STDOUT);
+} else {
+  die "Invalid color mode: $color\n";
+}
+
+# ------------- #
+#  Prototypes.  #
+# ------------- #
+
+sub colored ($$);
+sub decorate_result ($);
+sub extract_tap_comment ($);
+sub handle_tap_bailout ($);
+sub handle_tap_plan ($);
+sub handle_tap_result ($);
+sub is_null_string ($);
+sub main ();
+sub report ($;$);
+sub stringify_result_obj ($);
+sub testsuite_error ($);
+
+# -------------- #
+#  Subroutines.  #
+# -------------- #
+
+# If the given string is undefined or empty, return true, otherwise
+# return false.  This function is useful to avoid pitfalls like:
+#   if ($message) { print "$message\n"; }
+# which wouldn't print anything if $message is the literal "0".
+sub is_null_string ($)
+{
+  my $str = shift;
+  return ! (defined $str and length $str);
+}
+
+sub stringify_result_obj ($)
+{
+  my $result_obj = shift;
+  if ($result_obj->is_unplanned || $result_obj->number != $testno)
+    {
+      return "ERROR";
+    }
+  elsif ($plan_seen == LATE_PLAN)
+    {
+      return "ERROR";
+    }
+  elsif (!$result_obj->directive)
+    {
+      return $result_obj->is_ok ? "PASS" : "FAIL";
+    }
+  elsif ($result_obj->has_todo)
+    {
+      return $result_obj->is_actual_ok ? "XPASS" : "XFAIL";
+    }
+  elsif ($result_obj->has_skip)
+    {
+      return $result_obj->is_ok ? "SKIP" : "FAIL";
+    }
+  die "$ME: INTERNAL ERROR"; # NOTREACHED
+}
+
+sub colored ($$)
+{
+  my ($color_string, $text) = @_;
+  return $color_string . $text . RESET;
+}
+
+sub decorate_result ($)
+{
+  my $result = shift;
+  return $result unless $cfg{"color"};
+  my %color_for_result =
+    (
+      "ERROR" => BOLD.MAGENTA,
+      "PASS"  => GREEN,
+      "XPASS" => BOLD.YELLOW,
+      "FAIL"  => BOLD.RED,
+      "XFAIL" => YELLOW,
+      "SKIP"  => BLUE,
+    );
+  if (my $color = $color_for_result{$result})
+    {
+      return colored ($color, $result);
+    }
+  else
+    {
+      return $result; # Don't colorize unknown stuff.
+    }
+}
+
+sub report ($;$)
+{
+  my ($msg, $result, $explanation) = (undef, @_);
+  if ($result =~ /^(?:X?(?:PASS|FAIL)|SKIP|ERROR)/)
+    {
+      # Output on console might be colorized.
+      $msg = decorate_result($result);
+      if ($result =~ /^(?:PASS|XFAIL|SKIP)/)
+        {
+          return if $cfg{"show-failures-only"};
+        }
+      else
+        {
+          $failed = 1;
+        }
+    }
+  elsif ($result eq "#")
+    {
+      $msg = "  ";
+    }
+  else
+    {
+      die "$ME: INTERNAL ERROR"; # NOTREACHED
+    }
+  $msg .= " $explanation" if defined $explanation;
+  print $msg . "\n";
+}
+
+sub testsuite_error ($)
+{
+  report "ERROR", "- $_[0]";
+}
+
+sub handle_tap_result ($)
+{
+  $testno++;
+  my $result_obj = shift;
+
+  my $test_result = stringify_result_obj $result_obj;
+  my $string = $result_obj->number;
+
+  my $description = $result_obj->description;
+  $string .= " $test_name" unless is_null_string $test_name;
+  $string .= " $description" unless is_null_string $description;
+
+  if ($plan_seen == LATE_PLAN)
+    {
+      $string .= " # AFTER LATE PLAN";
+    }
+  elsif ($result_obj->is_unplanned)
+    {
+      $string .= " # UNPLANNED";
+    }
+  elsif ($result_obj->number != $testno)
+    {
+      $string .= " # OUT-OF-ORDER (expecting $testno)";
+    }
+  elsif (my $directive = $result_obj->directive)
+    {
+      $string .= " # $directive";
+      my $explanation = $result_obj->explanation;
+      $string .= " $explanation"
+        unless is_null_string $explanation;
+    }
+
+  report $test_result, $string;
+}
+
+sub handle_tap_plan ($)
+{
+  my $plan = shift;
+  if ($plan_seen)
+    {
+      # Error, only one plan per stream is acceptable.
+      testsuite_error "multiple test plans";
+      return;
+    }
+  # The TAP plan can come before or after *all* the TAP results; we speak
+  # respectively of an "early" or a "late" plan.  If we see the plan line
+  # after at least one TAP result has been seen, assume we have a late
+  # plan; in this case, any further test result seen after the plan will
+  # be flagged as an error.
+  $plan_seen = ($testno >= 1 ? LATE_PLAN : EARLY_PLAN);
+  # If $testno > 0, we have an error ("too many tests run") that will be
+  # automatically dealt with later, so don't worry about it here.  If
+  # $plan_seen is true, we have an error due to a repeated plan, and that
+  # has already been dealt with above.  Otherwise, we have a valid "plan
+  # with SKIP" specification, and should report it as a particular kind
+  # of SKIP result.
+  if ($plan->directive && $testno == 0)
+    {
+      my $explanation = is_null_string ($plan->explanation) ?
+                        undef : "- " . $plan->explanation;
+      report "SKIP", $explanation;
+    }
+}
+
+sub handle_tap_bailout ($)
+{
+  my ($bailout, $msg) = ($_[0], "Bail out!");
+  $bailed_out = 1;
+  $msg .= " " . $bailout->explanation
+    unless is_null_string $bailout->explanation;
+  testsuite_error $msg;
+}
+
+sub extract_tap_comment ($)
+{
+  my $line = shift;
+  if (index ($line, DIAG_STRING) == 0)
+    {
+      # Strip leading `DIAG_STRING' from `$line'.
+      $line = substr ($line, length (DIAG_STRING));
+      # And strip any leading and trailing whitespace left.
+      $line =~ s/(?:^\s*|\s*$)//g;
+      # Return what is left (if any).
+      return $line;
+    }
+  return "";
+}
+
+sub main ()
+{
+  my $iterator = TAP::Parser::Iterator::Stream->new(\*STDIN);
+  my $parser = TAP::Parser->new ({iterator => $iterator });
+
+  while (defined (my $cur = $parser->next))
+    {
+      # Parsing of TAP input should stop after a "Bail out!" directive.
+      next if $bailed_out;
+
+      if ($cur->is_plan)
+        {
+          handle_tap_plan ($cur);
+        }
+      elsif ($cur->is_test)
+        {
+          handle_tap_result ($cur);
+        }
+      elsif ($cur->is_bailout)
+        {
+          handle_tap_bailout ($cur);
+        }
+      elsif ($cfg{"verbose"})
+        {
+          my $comment = extract_tap_comment ($cur->raw);
+          report "#", "$comment" if length $comment;
+       }
+    }
+  # A "Bail out!" directive should cause us to ignore any following TAP
+  # error.
+  if (!$bailed_out)
+    {
+      if (!$plan_seen)
+        {
+          testsuite_error "missing test plan";
+        }
+      elsif ($parser->tests_planned != $parser->tests_run)
+        {
+          my ($planned, $run) = ($parser->tests_planned, $parser->tests_run);
+          my $bad_amount = $run > $planned ? "many" : "few";
+          testsuite_error (sprintf "too %s tests run (expected %d, got %d)",
+                                   $bad_amount, $planned, $run);
+        }
+    }
+}
+
+# ----------- #
+#  Main code. #
+# ----------- #
+
+main;
+exit($failed);
+
+# Local Variables:
+# perl-indent-level: 2
+# perl-continued-statement-offset: 2
+# perl-continued-brace-offset: 0
+# perl-brace-offset: 0
+# perl-brace-imaginary-offset: 0
+# perl-label-offset: -2
+# cperl-indent-level: 2
+# cperl-brace-offset: 0
+# cperl-continued-brace-offset: 0
+# cperl-label-offset: -2
+# cperl-extra-newline-before-brace: t
+# cperl-merge-trailing-else: nil
+# cperl-continued-statement-offset: 2
+# End:
diff --git a/scripts/tap-merge.pl b/scripts/tap-merge.pl
new file mode 100755
index 0000000000..59e3fa5007
--- /dev/null
+++ b/scripts/tap-merge.pl
@@ -0,0 +1,110 @@
+#! /usr/bin/env perl
+# Copyright (C) 2018 Red Hat, Inc.
+#
+# Author: Paolo Bonzini <pbonzini@redhat.com>
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <https://www.gnu.org/licenses/>.
+
+# ---------------------------------- #
+#  Imports, static data, and setup.  #
+# ---------------------------------- #
+
+use warnings FATAL => 'all';
+use strict;
+use Getopt::Long ();
+use TAP::Parser;
+
+my $ME = "tap-merge.pl";
+my $VERSION = "2018-11-30";
+
+my $HELP = "$ME: merge multiple TAP inputs from stdin.";
+
+use constant DIAG_STRING => "#";
+
+# ----------------- #
+#  Option parsing.  #
+# ----------------- #
+
+Getopt::Long::GetOptions
+  (
+    'help' => sub { print $HELP; exit 0; },
+    'version' => sub { print "$ME $VERSION\n"; exit 0; },
+  );
+
+# -------------- #
+#  Subroutines.  #
+# -------------- #
+
+sub main ()
+{
+  my $iterator = TAP::Parser::Iterator::Stream->new(\*STDIN);
+  my $parser = TAP::Parser->new ({iterator => $iterator });
+  my $testno = 0;     # Number of test results seen so far.
+  my $bailed_out = 0; # Whether a "Bail out!" directive has been seen.
+
+  while (defined (my $cur = $parser->next))
+    {
+      if ($cur->is_bailout)
+        {
+          $bailed_out = 1;
+          print DIAG_STRING . " " . $cur->as_string . "\n";
+          next;
+        }
+      elsif ($cur->is_plan)
+        {
+          $bailed_out = 0;
+          next;
+        }
+      elsif ($cur->is_test)
+        {
+          $bailed_out = 0 if $cur->number == 1;
+          $testno++;
+          $cur = TAP::Parser::Result::Test->new({
+                          ok => $cur->ok,
+                          test_num => $testno,
+                          directive => $cur->directive,
+                          explanation => $cur->explanation,
+                          description => $cur->description
+                  });
+        }
+      elsif ($cur->is_version)
+        {
+          next if $testno > 0;
+        }
+      print $cur->as_string . "\n" unless $bailed_out;
+    }
+  print "1..$testno\n";
+}
+
+# ----------- #
+#  Main code. #
+# ----------- #
+
+main;
+
+# Local Variables:
+# perl-indent-level: 2
+# perl-continued-statement-offset: 2
+# perl-continued-brace-offset: 0
+# perl-brace-offset: 0
+# perl-brace-imaginary-offset: 0
+# perl-label-offset: -2
+# cperl-indent-level: 2
+# cperl-brace-offset: 0
+# cperl-continued-brace-offset: 0
+# cperl-label-offset: -2
+# cperl-extra-newline-before-brace: t
+# cperl-merge-trailing-else: nil
+# cperl-continued-statement-offset: 2
+# End:
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))
 
 .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 \
-- 
2.19.2