qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PULL 23/35] test: replace gtester with a TAP driver
  2018-12-17 23:16 [Qemu-devel] [PULL 00/35] Misc patches for 2018-12-18 Paolo Bonzini
@ 2018-12-17 23:16 ` Paolo Bonzini
  0 siblings, 0 replies; 19+ messages in thread
From: Paolo Bonzini @ 2018-12-17 23:16 UTC (permalink / raw)
  To: qemu-devel

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>
Message-Id: <1543513531-1151-3-git-send-email-pbonzini@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 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..62915e4388 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.20.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PULL 23/35] test: replace gtester with a TAP driver
  2018-12-19 15:18 [Qemu-devel] [PULL v2 00/35] Misc patches for 2018-12-18 Paolo Bonzini
@ 2018-12-19 15:19 ` Paolo Bonzini
  0 siblings, 0 replies; 19+ messages in thread
From: Paolo Bonzini @ 2018-12-19 15:19 UTC (permalink / raw)
  To: qemu-devel

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>
Message-Id: <1543513531-1151-3-git-send-email-pbonzini@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 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 3f5a1d0c30..080c7637e2 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -808,41 +808,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))" $(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 $(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.20.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PULL v4 00/35] Misc patches for 2018-12-21
@ 2018-12-21 12:36 Paolo Bonzini
  2018-12-21 12:36 ` [Qemu-devel] [PULL 22/35] test: execute g_test_run when tests are skipped Paolo Bonzini
                   ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Paolo Bonzini @ 2018-12-21 12:36 UTC (permalink / raw)
  To: qemu-devel

The following changes since commit e85c577158a2e8e252414959da9ef15c12eec63d:

  Merge remote-tracking branch 'remotes/huth-gitlab/tags/pull-request-2018-12-17' into staging (2018-12-18 14:31:06 +0000)

are available in the git repository at:


  git://github.com/bonzini/qemu.git tags/for-upstream

for you to fetch changes up to 2cbc4ea2a9f9b81d5f4bfee8c412849755b842b0:

  avoid TABs in files that only contain a few (2018-12-21 12:58:28 +0100)

v3->v4: avoid extra long command lines and ease cut-and-paste from make check logs
----------------------------------------------------------------
* HAX support for Linux hosts (Alejandro)
* esp bugfixes (Guenter)
* Windows build cleanup (Marc-André)
* checkpatch logic improvements (Paolo)
* coalesced range bugfix (Paolo)
* switch testsuite to TAP (Paolo)
* QTAILQ rewrite (Paolo)
* block/iscsi.c cancellation fixes (Stefan)
* improve selection of the default accelerator (Thomas)

----------------------------------------------------------------
Alexandro Sanchez Bach (1):
      hax: Support for Linux hosts

Guenter Roeck (2):
      esp-pci: Fix status register write erase control
      scsi: esp: Defer command completion until previous interrupts have been handled

Marc-André Lureau (5):
      vhost-user-bridge: fix "unknown type name" compilation error
      build-sys: don't include windows.h, osdep.h does it
      build-sys: move windows defines in osdep.h header
      build-sys: build with Vista API by default
      qga: drop < Vista compatibility

Paolo Bonzini (21):
      checkpatch: fix premature exit when no input or --mailback
      checkpatch: check Signed-off-by in --mailback mode
      checkpatch: improve handling of multiple patches or files
      checkpatch: colorize output to terminal
      pam: wrap MemoryRegion initialization in a transaction
      memory: extract flat_range_coalesced_io_{del,add}
      memory: avoid unnecessary coalesced_io_del operations
      memory: update coalesced_range on transaction_commit
      test: execute g_test_run when tests are skipped
      test: replace gtester with a TAP driver
      qemu/queue.h: do not access tqe_prev directly
      vfio: make vfio_address_spaces static
      qemu/queue.h: leave head structs anonymous unless necessary
      qemu/queue.h: typedef QTAILQ heads
      qemu/queue.h: remove Q_TAILQ_{HEAD,ENTRY}
      qemu/queue.h: reimplement QTAILQ without pointer-to-pointers
      qemu/queue.h: simplify reverse access to QTAILQ
      checkpatch: warn about qemu/queue.h head structs that are not typedef-ed
      scripts: add script to convert multiline comments into 4-line format
      remove space-tab sequences
      avoid TABs in files that only contain a few

Peng Hao (1):
      hw/watchdog/wdt_i6300esb: remove a unnecessary comment

Stefan Hajnoczi (4):
      block/iscsi: drop unused IscsiAIOCB->buf field
      block/iscsi: take iscsilun->mutex in iscsi_timed_check_events()
      block/iscsi: fix ioctl cancel use-after-free
      block/iscsi: cancel libiscsi task when ABORT TASK TMF completes

Thomas Huth (1):
      accel: Improve selection of the default accelerator

 accel/accel.c                                |  18 +-
 accel/kvm/kvm-all.c                          |   4 +-
 accel/tcg/translate-all.c                    |   4 -
 block/bochs.c                                |  22 +-
 block/file-posix.c                           |   2 +-
 block/file-win32.c                           |   8 +-
 block/gluster.c                              |   2 +-
 block/iscsi.c                                |  47 +++-
 block/linux-aio.c                            |   4 +-
 block/mirror.c                               |   2 +-
 block/qcow2-bitmap.c                         |   4 +-
 block/qcow2-cluster.c                        |   2 +-
 block/qcow2.h                                |   5 +-
 block/sheepdog.c                             |   6 +-
 block/vhdx.h                                 |   2 +-
 block/vpc.c                                  |   2 +-
 blockdev.c                                   |   4 +-
 bsd-user/elfload.c                           |   2 +-
 bsd-user/x86_64/target_syscall.h             |   2 +-
 configure                                    |   3 -
 contrib/elf2dmp/main.c                       |   2 +-
 contrib/ivshmem-client/ivshmem-client.h      |   4 +-
 contrib/ivshmem-server/ivshmem-server.h      |   5 +-
 cpus-common.c                                |   2 +-
 crypto/aes.c                                 |  28 +-
 disas/alpha.c                                |   8 +-
 disas/arm.c                                  |   2 +-
 disas/i386.c                                 |   4 +-
 disas/m68k.c                                 |   4 +-
 dump.c                                       |   2 +-
 exec.c                                       |   5 +-
 fsdev/qemu-fsdev.c                           |   2 +-
 hw/alpha/typhoon.c                           |  12 +-
 hw/arm/stellaris.c                           |   2 +-
 hw/block/nvme.h                              |   8 +-
 hw/block/xen_disk.c                          |   6 +-
 hw/char/sh_serial.c                          |  18 +-
 hw/char/virtio-serial-bus.c                  |   2 +-
 hw/char/xen_console.c                        |  58 ++--
 hw/core/loader.c                             |  28 +-
 hw/core/qdev.c                               |   4 +-
 hw/core/reset.c                              |   2 +-
 hw/display/tc6393xb.c                        |   6 +-
 hw/display/vga.c                             |   8 +-
 hw/display/virtio-gpu-3d.c                   |   6 +-
 hw/dma/pxa2xx_dma.c                          |   4 +-
 hw/dma/soc_dma.c                             |   2 +-
 hw/gpio/max7310.c                            |   2 +-
 hw/i386/xen/xen-hvm.c                        |   4 +-
 hw/i386/xen/xen-mapcache.c                   |   2 +-
 hw/ide/core.c                                |  94 +++----
 hw/input/lm832x.c                            |   2 +-
 hw/input/pckbd.c                             |   2 +-
 hw/input/tsc210x.c                           |   2 +-
 hw/intc/apic.c                               |   2 +-
 hw/mips/gt64xxx_pci.c                        |   6 +-
 hw/mips/mips_r4k.c                           |   4 +-
 hw/misc/max111x.c                            |   6 +-
 hw/misc/omap_l4.c                            |   4 +-
 hw/net/mipsnet.c                             |  16 +-
 hw/net/ne2000.c                              |  44 ++--
 hw/net/rocker/rocker.c                       |   2 +-
 hw/net/virtio-net.c                          |   4 +-
 hw/net/vmxnet3.c                             |   6 +-
 hw/pci-host/pam.c                            |   2 +
 hw/pci/msix.c                                |   2 +-
 hw/pci/pci.c                                 |  44 ++--
 hw/pci/pci_bridge.c                          |   2 +-
 hw/ppc/ppc405_uc.c                           |   2 +-
 hw/ppc/prep.c                                |   4 +-
 hw/ppc/spapr_iommu.c                         |   2 +-
 hw/scsi/esp-pci.c                            |  10 +-
 hw/scsi/esp.c                                |  33 ++-
 hw/scsi/lsi53c895a.c                         |   6 +-
 hw/scsi/scsi-bus.c                           |   2 +-
 hw/scsi/trace-events                         |   1 +
 hw/sh4/r2d.c                                 |  16 +-
 hw/usb/ccid-card-emulated.c                  |   4 +-
 hw/usb/combined-packet.c                     |   2 +-
 hw/usb/dev-bluetooth.c                       |   2 +-
 hw/usb/dev-hid.c                             |   6 +-
 hw/usb/dev-hub.c                             |  14 +-
 hw/usb/dev-mtp.c                             |   4 +-
 hw/usb/dev-network.c                         |   2 +-
 hw/usb/hcd-ehci.c                            |   2 +-
 hw/usb/hcd-ehci.h                            |   2 +-
 hw/usb/hcd-uhci.c                            |   8 +-
 hw/usb/xen-usb.c                             |   6 +-
 hw/vfio/common.c                             |   4 +-
 hw/watchdog/watchdog.c                       |   2 +-
 hw/watchdog/wdt_i6300esb.c                   |   1 -
 hw/xen/xen_devconfig.c                       |   2 +-
 hw/xen/xen_pvdev.c                           |   4 +-
 hw/xenpv/xen_domainbuild.c                   |   8 +-
 include/elf.h                                |  10 +-
 include/exec/memory.h                        |   6 +-
 include/hw/acpi/acpi.h                       |  14 +-
 include/hw/elf_ops.h                         |   2 +-
 include/hw/ide/internal.h                    |   2 +-
 include/hw/qdev-core.h                       |   2 +-
 include/hw/scsi/esp.h                        |   2 +
 include/hw/sh4/sh_intc.h                     |  20 +-
 include/hw/usb.h                             |   2 +-
 include/hw/vfio/vfio-common.h                |   4 +-
 include/hw/vfio/vfio-platform.h              |   2 +-
 include/hw/xen/io/ring.h                     |   4 +-
 include/net/net.h                            |   2 +-
 include/qemu/acl.h                           |  14 +-
 include/qemu/iov.h                           |   2 +-
 include/qemu/option_int.h                    |   2 +-
 include/qemu/osdep.h                         |  17 ++
 include/qemu/queue.h                         | 153 +++++------
 include/qemu/rcu_queue.h                     |  45 ++--
 include/qom/cpu.h                            |   9 +-
 include/scsi/constants.h                     |   2 +-
 include/sysemu/accel.h                       |   2 +-
 include/sysemu/balloon.h                     |   2 +-
 include/sysemu/kvm.h                         |   2 -
 include/sysemu/memory_mapping.h              |   2 +-
 include/sysemu/rng.h                         |   2 +-
 linux-user/elfload.c                         |   2 +-
 linux-user/linuxload.c                       |  14 +-
 linux-user/main.c                            |   4 +-
 linux-user/mmap.c                            |  10 +-
 linux-user/qemu.h                            |   4 +-
 linux-user/signal.c                          |  16 +-
 linux-user/strace.c                          |   4 +-
 linux-user/syscall.c                         |   2 +-
 linux-user/syscall_defs.h                    |   4 +-
 linux-user/uaccess.c                         |   2 +-
 linux-user/vm86.c                            |   2 +-
 linux-user/x86_64/target_syscall.h           |   2 +-
 memory.c                                     |  97 ++++---
 memory_mapping.c                             |   2 +-
 migration/block-dirty-bitmap.c               |   2 +-
 migration/block.c                            |   4 +-
 migration/ram.c                              |   2 +-
 monitor.c                                    |   4 +-
 nbd/client.c                                 |   2 +-
 net/checksum.c                               |   2 +-
 net/filter.c                                 |   2 +-
 net/net.c                                    |   2 +-
 net/queue.c                                  |   2 +-
 net/slirp.c                                  |   2 +-
 qga/commands-posix.c                         |   2 +-
 qga/commands-win32.c                         |  70 +----
 qtest.c                                      |   4 +-
 rules.mak                                    |   4 +-
 scripts/checkpatch.pl                        |  95 +++++--
 scripts/cocci-macro-file.h                   |  24 +-
 scripts/fix-multiline-comments.sh            |  62 +++++
 scripts/gtester-cat                          |  26 --
 scripts/tap-driver.pl                        | 378 +++++++++++++++++++++++++++
 scripts/tap-merge.pl                         | 110 ++++++++
 slirp/ip_input.c                             |   4 +-
 slirp/slirp.c                                |   2 +-
 slirp/tcp_input.c                            |  10 +-
 slirp/tcp_output.c                           |   4 +-
 slirp/tcp_timer.c                            |   2 +-
 slirp/udp.c                                  |   2 +-
 target/alpha/translate.c                     |   2 +-
 target/arm/kvm.c                             |   2 +-
 target/cris/helper.c                         |   2 +-
 target/cris/mmu.h                            |  10 +-
 target/cris/translate_v10.inc.c              |   2 +-
 target/i386/Makefile.objs                    |   6 +-
 target/i386/hax-i386.h                       |   6 +-
 target/i386/hax-mem.c                        |   2 +-
 target/i386/{hax-darwin.c => hax-posix.c}    |   0
 target/i386/{hax-darwin.h => hax-posix.h}    |   0
 target/i386/translate.c                      |  12 +-
 target/mips/translate.c                      |   2 +-
 target/tilegx/translate.c                    |   2 +-
 tcg/i386/tcg-target.inc.c                    |   4 +-
 tcg/tcg.c                                    |   2 +-
 tcg/tcg.h                                    |   6 +-
 tests/Makefile.include                       |  74 ++++--
 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/ivshmem-test.c                         |   5 +-
 tests/libqos/malloc.c                        |   2 +-
 tests/migration-test.c                       |   8 +-
 tests/tcg/alpha/test-cond.c                  |   4 +-
 tests/tcg/arm/hello-arm.c                    |  20 +-
 tests/tcg/cris/check_glibc_kernelversion.c   |   8 +-
 tests/tcg/cris/check_mmap3.c                 |   2 +-
 tests/tcg/cris/check_openpf1.c               |   2 +-
 tests/tcg/cris/check_settls1.c               |   2 +-
 tests/tcg/i386/hello-i386.c                  |  14 +-
 tests/tcg/mips/hello-mips.c                  |  10 +-
 tests/tcg/multiarch/sha1.c                   |  12 +-
 tests/test-crypto-pbkdf.c                    |   3 +-
 tests/test-rcu-list.c                        |   2 +-
 tests/test-vmstate.c                         |   8 +-
 tests/vhost-user-bridge.c                    |   3 +-
 tests/vhost-user-test.c                      |   4 +-
 ui/console.c                                 |   4 +-
 ui/input.c                                   |  14 +-
 ui/keymaps.h                                 |   4 +-
 ui/qemu-pixman.c                             |   2 +-
 ui/vnc-enc-zywrle-template.c                 |   4 +-
 ui/vnc.c                                     |   4 +-
 util/bitops.c                                |   4 +-
 util/osdep.c                                 |   4 +-
 util/qemu-option.c                           |   4 +-
 util/qemu-sockets.c                          |   4 +-
 util/qemu-thread-win32.c                     |   4 -
 vl.c                                         |   4 +-
 215 files changed, 1505 insertions(+), 898 deletions(-)
 create mode 100755 scripts/fix-multiline-comments.sh
 delete mode 100755 scripts/gtester-cat
 create mode 100755 scripts/tap-driver.pl
 create mode 100755 scripts/tap-merge.pl
 rename target/i386/{hax-darwin.c => hax-posix.c} (100%)
 rename target/i386/{hax-darwin.h => hax-posix.h} (100%)
-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PULL 22/35] test: execute g_test_run when tests are skipped
  2018-12-21 12:36 [Qemu-devel] [PULL v4 00/35] Misc patches for 2018-12-21 Paolo Bonzini
@ 2018-12-21 12:36 ` Paolo Bonzini
  2018-12-21 12:36 ` [Qemu-devel] [PULL 23/35] test: replace gtester with a TAP driver Paolo Bonzini
  2018-12-21 21:09 ` [Qemu-devel] [PULL v4 00/35] Misc patches for 2018-12-21 Peter Maydell
  2 siblings, 0 replies; 19+ messages in thread
From: Paolo Bonzini @ 2018-12-21 12:36 UTC (permalink / raw)
  To: qemu-devel

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>
Message-Id: <1543513531-1151-2-git-send-email-pbonzini@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/cdrom-test.c        | 2 +-
 tests/ivshmem-test.c      | 5 ++---
 tests/migration-test.c    | 8 ++++----
 tests/test-crypto-pbkdf.c | 3 ++-
 4 files changed, 9 insertions(+), 9 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/ivshmem-test.c b/tests/ivshmem-test.c
index 089e268..fe5eb30 100644
--- a/tests/ivshmem-test.c
+++ b/tests/ivshmem-test.c
@@ -492,7 +492,7 @@ int main(int argc, char **argv)
     /* shm */
     tmpshm = mktempshm(TMPSHMSIZE, &fd);
     if (!tmpshm) {
-        return 0;
+        goto out;
     }
     tmpshmem = mmap(0, TMPSHMSIZE, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
     g_assert(tmpshmem != MAP_FAILED);
@@ -514,9 +514,8 @@ int main(int argc, char **argv)
         }
     }
 
+out:
     ret = g_test_run();
-
     cleanup();
-
     return ret;
 }
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
     }
 
diff --git a/tests/test-crypto-pbkdf.c b/tests/test-crypto-pbkdf.c
index d937aff..85ed1f9 100644
--- a/tests/test-crypto-pbkdf.c
+++ b/tests/test-crypto-pbkdf.c
@@ -440,6 +440,7 @@ int main(int argc, char **argv)
 #else
 int main(int argc, char **argv)
 {
-    return 0;
+    g_test_init(&argc, &argv, NULL);
+    return g_test_run();
 }
 #endif
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PULL 23/35] test: replace gtester with a TAP driver
  2018-12-21 12:36 [Qemu-devel] [PULL v4 00/35] Misc patches for 2018-12-21 Paolo Bonzini
  2018-12-21 12:36 ` [Qemu-devel] [PULL 22/35] test: execute g_test_run when tests are skipped Paolo Bonzini
@ 2018-12-21 12:36 ` Paolo Bonzini
  2018-12-21 21:09 ` [Qemu-devel] [PULL v4 00/35] Misc patches for 2018-12-21 Peter Maydell
  2 siblings, 0 replies; 19+ messages in thread
From: Paolo Bonzini @ 2018-12-21 12:36 UTC (permalink / raw)
  To: qemu-devel

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>
Message-Id: <1543513531-1151-3-git-send-email-pbonzini@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rules.mak                                    |   4 +-
 scripts/gtester-cat                          |  26 --
 scripts/tap-driver.pl                        | 378 +++++++++++++++++++++++++++
 scripts/tap-merge.pl                         | 110 ++++++++
 tests/Makefile.include                       |  74 ++++--
 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 +
 13 files changed, 548 insertions(+), 52 deletions(-)
 delete mode 100755 scripts/gtester-cat
 create mode 100755 scripts/tap-driver.pl
 create mode 100755 scripts/tap-merge.pl

diff --git a/rules.mak b/rules.mak
index bbb2667..86e033d 100644
--- a/rules.mak
+++ b/rules.mak
@@ -132,7 +132,9 @@ modules:
 #  otherwise print the 'quiet' output in the format "  NAME     args to print"
 # NAME should be a short name of the command, 7 letters or fewer.
 # If called with only a single argument, will print nothing in quiet mode.
-quiet-command = $(if $(V),$1,$(if $(2),@printf "  %-7s %s\n" $2 $3 && $1, @$1))
+quiet-command-run = $(if $(V),,$(if $2,printf "  %-7s %s\n" $2 $3 && ))$1
+quiet-@ = $(if $(V),,@)
+quiet-command = $(quiet-@)$(call quiet-command-run,$1,$2,$3)
 
 # cc-option
 # Usage: CFLAGS+=$(call cc-option, -falign-functions=0, -malign-functions=0)
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..5e59b5d
--- /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 0000000..59e3fa5
--- /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 3f5a1d0..aacd070 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -808,41 +808,67 @@ 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
+
+define do_test_human_k
+        $(quiet-@)rc=0; $(foreach COMMAND, $1, \
+          $(call quiet-command-run, \
+            export MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$(( $${RANDOM:-0} % 255 + 1))} $2; \
+              $(COMMAND) -m=$(SPEED) -k --tap \
+              | ./scripts/tap-driver.pl --test-name="$(notdir $(COMMAND))" $(if $(V),, --show-failures-only) \
+              || rc=$$?;, "TEST", "$@: $(COMMAND)")) exit $$rc
+endef
+define do_test_human_no_k
+        $(foreach COMMAND, $1, \
+          $(call quiet-command, \
+            MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$(( $${RANDOM:-0} % 255 + 1))} $2 \
+              $(COMMAND) -m=$(SPEED) -k --tap \
+              | ./scripts/tap-driver.pl --test-name="$(notdir $(COMMAND))" $(if $(V),, --show-failures-only), \
+              "TEST", "$@: $(COMMAND)")
+)
+endef
+do_test_human = \
+        $(if $(findstring k, $(MAKEFLAGS)), $(do_test_human_k), $(do_test_human_no_k))
+
+define do_test_tap
+	$(call quiet-command, \
+          { export MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$(( $${RANDOM:-0} % 255 + 1))} $2; \
+            $(foreach COMMAND, $1, \
+	      $(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 $(if $(V),, --show-failures-only), \
+	  "TAP","$@")
+endef
 
 .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","$*")
+check-unit: $(check-unit-y)
+	$(call do_test_human, $^)
 
-# gtester tests with XML output
+check-speed: $(check-speed-y)
+	$(call do_test_human, $^)
 
-$(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","$@")
+# gtester tests with TAP output
 
-check-report-unit.xml: $(check-unit-y)
-	$(call quiet-command,gtester -q $(GTESTER_OPTIONS) -o $@ -m=$(SPEED) $^,"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))
 
-# Reports and overall runs
+check-report-unit.tap: $(check-unit-y)
+	$(call do_test_tap,$^)
 
-check-report.xml: $(patsubst %,check-report-qtest-%.xml, $(QTEST_TARGETS)) check-report-unit.xml
-	$(call quiet-command,$(SRC_PATH)/scripts/gtester-cat $^ > $@,"GEN","$@")
+# Reports and overall runs
 
-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
 
@@ -957,8 +983,6 @@ check-acceptance: check-venv $(TESTS_RESULTS_DIR)
 .PHONY: check-qapi-schema check-qtest check-unit check check-clean
 check-qapi-schema: $(patsubst %,check-%, $(check-qapi-schema-y)) check-tests/qapi-schema/doc-good.texi
 check-qtest: $(patsubst %,check-qtest-%, $(QTEST_TARGETS))
-check-unit: $(patsubst %,check-%, $(check-unit-y))
-check-speed: $(patsubst %,check-%, $(check-speed-y))
 check-block: $(patsubst %,check-%, $(check-block-y))
 check: check-qapi-schema check-unit check-qtest check-decodetree
 check-clean:
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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PULL v4 00/35] Misc patches for 2018-12-21
  2018-12-21 12:36 [Qemu-devel] [PULL v4 00/35] Misc patches for 2018-12-21 Paolo Bonzini
  2018-12-21 12:36 ` [Qemu-devel] [PULL 22/35] test: execute g_test_run when tests are skipped Paolo Bonzini
  2018-12-21 12:36 ` [Qemu-devel] [PULL 23/35] test: replace gtester with a TAP driver Paolo Bonzini
@ 2018-12-21 21:09 ` Peter Maydell
  2018-12-22  8:41   ` Paolo Bonzini
  2 siblings, 1 reply; 19+ messages in thread
From: Peter Maydell @ 2018-12-21 21:09 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: QEMU Developers

On Fri, 21 Dec 2018 at 12:40, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> The following changes since commit e85c577158a2e8e252414959da9ef15c12eec63d:
>
>   Merge remote-tracking branch 'remotes/huth-gitlab/tags/pull-request-2018-12-17' into staging (2018-12-18 14:31:06 +0000)
>
> are available in the git repository at:
>
>
>   git://github.com/bonzini/qemu.git tags/for-upstream
>
> for you to fetch changes up to 2cbc4ea2a9f9b81d5f4bfee8c412849755b842b0:
>
>   avoid TABs in files that only contain a few (2018-12-21 12:58:28 +0100)
>
> v3->v4: avoid extra long command lines and ease cut-and-paste from make check logs
> ----------------------------------------------------------------
> * HAX support for Linux hosts (Alejandro)
> * esp bugfixes (Guenter)
> * Windows build cleanup (Marc-André)
> * checkpatch logic improvements (Paolo)
> * coalesced range bugfix (Paolo)
> * switch testsuite to TAP (Paolo)
> * QTAILQ rewrite (Paolo)
> * block/iscsi.c cancellation fixes (Stefan)
> * improve selection of the default accelerator (Thomas)
>
> ----------------------------------------------------------------

Thanks for improving the length of the command lines. Unfortunately
this does not seem to have fixed the 'write error' from make:

MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64
QTEST_QEMU_IMG=qemu-img test
s/boot-order-test -m=quick -k --tap | ./scripts/tap-driver.pl
--test-name="boot-order-test"
PASS 1 boot-order-test /x86_64/boot-order/pc
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64
QTEST_QEMU_IMG=qemu-img test
s/bios-tables-test -m=quick -k --tap | ./scripts/tap-driver.pl
--test-name="bios-tables-test"

Looking for expected file 'tests/data/acpi/pc/DSDT'
Using expected file 'tests/data/acpi/pc/DSDT'
Looking for expected file 'tests/data/acpi/pc/FACP'
Using expected file 'tests/data/acpi/pc/FACP'
[... omit a bunch of other uninteresting logging from the acpi test]
Looking for expected file 'tests/data/acpi/pc/NFIT.dimmpxm'
Using expected file 'tests/data/acpi/pc/NFIT.dimmpxm'

Looking for expected file 'tests/data/acpi/q35/DSDT.bridge'
Using expected file 'tests/data/acpi/q35/DSDT.bridge'
Looking for expected file 'tests/data/acpi/q35/FACP.bridge'
Looking for expected file 'tests/data/acpi/q35/FACP'
Using expected file 'tests/data/acpi/q35/FACP'
Looking for expected file 'tests/data/acpi/q35/APIC.bridge'
Looking for expected file 'tests/data/acpi/q35/APIC'
Using expected file 'tests/data/acpi/q35/APIC'
Looking for expectfwrite(): Resource temporarily unavailable
make: Leaving directory '/home/petmay01/linaro/qemu-for-merges/build/alldbg'
make: write error: stdout

I don't really understand what's going on here, or why
it only happens with this one system (my main x86-64
Linux Ubuntu 16.04.5 box) and not the various others I'm
running test builds on. But it does seem to be 100%
reliable with any of these pullreqs with the new test
driver in them :-(

thanks
-- PMM

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PULL v4 00/35] Misc patches for 2018-12-21
  2018-12-21 21:09 ` [Qemu-devel] [PULL v4 00/35] Misc patches for 2018-12-21 Peter Maydell
@ 2018-12-22  8:41   ` Paolo Bonzini
  2018-12-22 11:26     ` Peter Maydell
  2019-01-03 18:37     ` Peter Maydell
  0 siblings, 2 replies; 19+ messages in thread
From: Paolo Bonzini @ 2018-12-22  8:41 UTC (permalink / raw)
  To: Peter Maydell; +Cc: QEMU Developers

On 21/12/18 22:09, Peter Maydell wrote:
> I don't really understand what's going on here, or why
> it only happens with this one system (my main x86-64
> Linux Ubuntu 16.04.5 box) and not the various others I'm
> running test builds on. But it does seem to be 100%
> reliable with any of these pullreqs with the new test
> driver in them :-(

I'm afraid something in your setup is causing make's stdout to have
O_NONBLOCK set.  Make doesn't use O_NONBLOCK at all, so it must be
something above it.  I also checked Perl with strace and, at least here,
it doesn't set O_NONBLOCK.

So here are some ideas... First, can you try applying something like
this to reproduce?

--- a/Makefile
+++ b/Makefile
@@ -17,9 +17,13 @@ print-%:
 # All following code might depend on configuration variables
 ifneq ($(wildcard config-host.mak),)
 # Put the all: rule here so that config-host.mak can contain dependencies.
-all:
+all: lotsofoutput
 include config-host.mak

+.PHONY: lotsofoutput
+lotsofoutput:
+	yes 1234567890 | head -n 10000
+
 git-submodule-update:

 .PHONY: git-submodule-update


And please try applying this, which is a bit of a shot in the dark but
1) it is a good idea anyway; 2) it may help, if not alone, together with
the workarounds below:

diff --git a/scripts/tap-driver.pl b/scripts/tap-driver.pl
index 5e59b5db49..6621a5cd67 100755
--- a/scripts/tap-driver.pl
+++ b/scripts/tap-driver.pl
@@ -313,6 +313,7 @@ sub main ()
   my $iterator = TAP::Parser::Iterator::Stream->new(\*STDIN);
   my $parser = TAP::Parser->new ({iterator => $iterator });

+  STDOUT->autoflush(1);
   while (defined (my $cur = $parser->next))
     {
       # Parsing of TAP input should stop after a "Bail out!" directive.
diff --git a/scripts/tap-merge.pl b/scripts/tap-merge.pl
index 59e3fa5007..10ccf57bb2 100755
--- a/scripts/tap-merge.pl
+++ b/scripts/tap-merge.pl
@@ -53,6 +53,7 @@ sub main ()
   my $testno = 0;     # Number of test results seen so far.
   my $bailed_out = 0; # Whether a "Bail out!" directive has been seen.

+  STDOUT->autoflush(1);
   while (defined (my $cur = $parser->next))
     {
       if ($cur->is_bailout)

Possible workarounds include:

- using "make -Oline" or "make -Onone" (for -Oline, it may require the
above autoflush patch).

- running this Python script before invoking make

    import os
    from fcntl import *
    fcntl(1, F_SETFL, fcntl(1, F_GETFL) & ~os.O_NONBLOCK)

Paolo

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PULL v4 00/35] Misc patches for 2018-12-21
  2018-12-22  8:41   ` Paolo Bonzini
@ 2018-12-22 11:26     ` Peter Maydell
  2019-01-03 18:37     ` Peter Maydell
  1 sibling, 0 replies; 19+ messages in thread
From: Peter Maydell @ 2018-12-22 11:26 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: QEMU Developers

On Sat, 22 Dec 2018 at 08:41, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 21/12/18 22:09, Peter Maydell wrote:
> > I don't really understand what's going on here, or why
> > it only happens with this one system (my main x86-64
> > Linux Ubuntu 16.04.5 box) and not the various others I'm
> > running test builds on. But it does seem to be 100%
> > reliable with any of these pullreqs with the new test
> > driver in them :-(
>
> I'm afraid something in your setup is causing make's stdout to have
> O_NONBLOCK set.  Make doesn't use O_NONBLOCK at all, so it must be
> something above it.  I also checked Perl with strace and, at least here,
> it doesn't set O_NONBLOCK.
>
> So here are some ideas... First, can you try applying something like
> this to reproduce?

OK, I'll give these a go, but it'll have to be in January now.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PULL v4 00/35] Misc patches for 2018-12-21
  2018-12-22  8:41   ` Paolo Bonzini
  2018-12-22 11:26     ` Peter Maydell
@ 2019-01-03 18:37     ` Peter Maydell
  2019-01-04  7:59       ` Paolo Bonzini
  1 sibling, 1 reply; 19+ messages in thread
From: Peter Maydell @ 2019-01-03 18:37 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: QEMU Developers

On Sat, 22 Dec 2018 at 08:41, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 21/12/18 22:09, Peter Maydell wrote:
> > I don't really understand what's going on here, or why
> > it only happens with this one system (my main x86-64
> > Linux Ubuntu 16.04.5 box) and not the various others I'm
> > running test builds on. But it does seem to be 100%
> > reliable with any of these pullreqs with the new test
> > driver in them :-(
>
> I'm afraid something in your setup is causing make's stdout to have
> O_NONBLOCK set.  Make doesn't use O_NONBLOCK at all, so it must be
> something above it.  I also checked Perl with strace and, at least here,
> it doesn't set O_NONBLOCK.

Interestingly, I have today run into "make: write error: stdout"
with the existing make check infrastructure. It seems to
only happen if I run 'make check' in a loop like this:
  while make -C build/x86 -j8 V=1 check; do true; done
Adding strace to this shows that although the first
invocation of 'make' starts with stdout not O_NONBLOCK,
something in that run results in stdout being set O_NONBLOCK,
and then that persists into the second run of make,
which then may fail as a result. (This is make 4.1, which
will write output with fputs() unless a --sync-output option
is used, and thus doesn't retry on EINTR.) When the 'while'
loop finishes, something in the shell results in stdout
being set back to blocking again.

I presume that something in one of the tests we're running,
likely QEMU itself, ends up setting stdout to non-blocking.
This while rune *used* to be entirely reliable, so maybe
something recent has changed ?

thanks
-- PMM

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PULL v4 00/35] Misc patches for 2018-12-21
  2019-01-03 18:37     ` Peter Maydell
@ 2019-01-04  7:59       ` Paolo Bonzini
  2019-01-04 10:06         ` Peter Maydell
  2019-01-04 13:03         ` Peter Maydell
  0 siblings, 2 replies; 19+ messages in thread
From: Paolo Bonzini @ 2019-01-04  7:59 UTC (permalink / raw)
  To: Peter Maydell; +Cc: QEMU Developers

On 03/01/19 19:37, Peter Maydell wrote:
> On Sat, 22 Dec 2018 at 08:41, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>> On 21/12/18 22:09, Peter Maydell wrote:
>>> I don't really understand what's going on here, or why
>>> it only happens with this one system (my main x86-64
>>> Linux Ubuntu 16.04.5 box) and not the various others I'm
>>> running test builds on. But it does seem to be 100%
>>> reliable with any of these pullreqs with the new test
>>> driver in them :-(
>>
>> I'm afraid something in your setup is causing make's stdout to have
>> O_NONBLOCK set.  Make doesn't use O_NONBLOCK at all, so it must be
>> something above it.  I also checked Perl with strace and, at least here,
>> it doesn't set O_NONBLOCK.
> 
> Interestingly, I have today run into "make: write error: stdout"
> with the existing make check infrastructure. [...]
> I presume that something in one of the tests we're running,
> likely QEMU itself, ends up setting stdout to non-blocking.
> This while rune *used* to be entirely reliable, so maybe
> something recent has changed ?

I don't know... I tried running make check under "strace -e fcntl" and I
didn't find any occurrences of fcntl(1, O_SETFL, ...|O_NONBLOCK).
Perhaps you can add a check after every invocation of a test executable.

Are you going to apply the pull request since the bug is preexisting?

Thanks,

Polo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PULL v4 00/35] Misc patches for 2018-12-21
  2019-01-04  7:59       ` Paolo Bonzini
@ 2019-01-04 10:06         ` Peter Maydell
  2019-01-04 11:01           ` Paolo Bonzini
  2019-01-04 13:03         ` Peter Maydell
  1 sibling, 1 reply; 19+ messages in thread
From: Peter Maydell @ 2019-01-04 10:06 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: QEMU Developers

On Fri, 4 Jan 2019 at 07:59, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 03/01/19 19:37, Peter Maydell wrote:
> > On Sat, 22 Dec 2018 at 08:41, Paolo Bonzini <pbonzini@redhat.com> wrote:
> > Interestingly, I have today run into "make: write error: stdout"
> > with the existing make check infrastructure. [...]
> > I presume that something in one of the tests we're running,
> > likely QEMU itself, ends up setting stdout to non-blocking.
> > This while rune *used* to be entirely reliable, so maybe
> > something recent has changed ?
>
> I don't know... I tried running make check under "strace -e fcntl" and I
> didn't find any occurrences of fcntl(1, O_SETFL, ...|O_NONBLOCK).
> Perhaps you can add a check after every invocation of a test executable.

> Are you going to apply the pull request since the bug is preexisting?

The pull request takes the problem from "occasionally shows up
in by-hand running of make check in a while loop" to "reliably
causes my automated tests to fail every time". I can't apply
a pullreq that does that, so we need to find at least a workaround.
I'm not sure why your test harness makes it much more likely
that the problem manifests.

(The shell will typically clear the O_NONBLOCK flag on its terminal
before waiting for interactive input, which is why this is a
problem when running make check inside a while loop or in my
pull test script which runs make check for several trees in
succession, but not an issue for individual runs on the command line.)

thanks
-- PMM

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PULL v4 00/35] Misc patches for 2018-12-21
  2019-01-04 10:06         ` Peter Maydell
@ 2019-01-04 11:01           ` Paolo Bonzini
  2019-01-04 11:31             ` Peter Maydell
  0 siblings, 1 reply; 19+ messages in thread
From: Paolo Bonzini @ 2019-01-04 11:01 UTC (permalink / raw)
  To: Peter Maydell; +Cc: QEMU Developers

On 04/01/19 11:06, Peter Maydell wrote:
> On Fri, 4 Jan 2019 at 07:59, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>> On 03/01/19 19:37, Peter Maydell wrote:
>>> On Sat, 22 Dec 2018 at 08:41, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>> Interestingly, I have today run into "make: write error: stdout"
>>> with the existing make check infrastructure. [...]
>>> I presume that something in one of the tests we're running,
>>> likely QEMU itself, ends up setting stdout to non-blocking.
>>> This while rune *used* to be entirely reliable, so maybe
>>> something recent has changed ?
>>
>> I don't know... I tried running make check under "strace -e fcntl" and I
>> didn't find any occurrences of fcntl(1, O_SETFL, ...|O_NONBLOCK).
>> Perhaps you can add a check after every invocation of a test executable.
> 
>> Are you going to apply the pull request since the bug is preexisting?
> 
> The pull request takes the problem from "occasionally shows up
> in by-hand running of make check in a while loop" to "reliably
> causes my automated tests to fail every time". I can't apply
> a pullreq that does that, so we need to find at least a workaround.

The certain workaround is to clear O_NONBLOCK before invoking make,
possibly by placing a wrapper in ~/bin:

    import os
    from fcntl import *
    fcntl(1, F_SETFL, fcntl(1, F_GETFL) & ~os.O_NONBLOCK)

If you're using --output-sync, *not* using it might work, but not if the
write error occurs when writing the command (as opposed to the output).
 For what it's worth, I think the latest version of Make is still
susceptible when using --output-sync.  The loop that flushes the
temporary buffer to stdout or stderr is as follows:

  while (1)
    {
      int len;
      EINTRLOOP (len, read (from, buffer, sizeof (buffer)));
      if (len < 0)
        perror ("read()");
      if (len <= 0)
        break;
      if (fwrite (buffer, len, 1, to) < 1)
        {
          perror ("fwrite()");
          break;
        }
      fflush (to);
    }

Maybe it's one of the dependencies of QEMU that is causing it.


Paolo

> I'm not sure why your test harness makes it much more likely
> that the problem manifests.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PULL v4 00/35] Misc patches for 2018-12-21
  2019-01-04 11:01           ` Paolo Bonzini
@ 2019-01-04 11:31             ` Peter Maydell
  2019-01-04 12:59               ` Paolo Bonzini
  0 siblings, 1 reply; 19+ messages in thread
From: Peter Maydell @ 2019-01-04 11:31 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: QEMU Developers

On Fri, 4 Jan 2019 at 11:01, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 04/01/19 11:06, Peter Maydell wrote:
> > On Fri, 4 Jan 2019 at 07:59, Paolo Bonzini <pbonzini@redhat.com> wrote:
> >>
> >> On 03/01/19 19:37, Peter Maydell wrote:
> >>> On Sat, 22 Dec 2018 at 08:41, Paolo Bonzini <pbonzini@redhat.com> wrote:
> >>> Interestingly, I have today run into "make: write error: stdout"
> >>> with the existing make check infrastructure. [...]
> >>> I presume that something in one of the tests we're running,
> >>> likely QEMU itself, ends up setting stdout to non-blocking.
> >>> This while rune *used* to be entirely reliable, so maybe
> >>> something recent has changed ?
> >>
> >> I don't know... I tried running make check under "strace -e fcntl" and I
> >> didn't find any occurrences of fcntl(1, O_SETFL, ...|O_NONBLOCK).
> >> Perhaps you can add a check after every invocation of a test executable.
> >
> >> Are you going to apply the pull request since the bug is preexisting?
> >
> > The pull request takes the problem from "occasionally shows up
> > in by-hand running of make check in a while loop" to "reliably
> > causes my automated tests to fail every time". I can't apply
> > a pullreq that does that, so we need to find at least a workaround.
>
> The certain workaround is to clear O_NONBLOCK before invoking make,
> possibly by placing a wrapper in ~/bin:
>
>     import os
>     from fcntl import *
>     fcntl(1, F_SETFL, fcntl(1, F_GETFL) & ~os.O_NONBLOCK)

I'll give this a go, but I think this will not necessarily be
sufficient if some program invoked by make sets the O_NONBLOCK flag,
and then make later in the same run tries to output and gets EINTR.
Perhaps it would be better to put it in the test harness that
invokes test binaries, so as to clear the flag as soon
as possible? It could also then in theory print a warning
if a test case had put stdin/stdout into non-blocking mode.

(The semantics of O_NONBLOCK here seem to me to be completely
broken. But they are what they are...)

thanks
-- PMM

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PULL v4 00/35] Misc patches for 2018-12-21
  2019-01-04 11:31             ` Peter Maydell
@ 2019-01-04 12:59               ` Paolo Bonzini
  0 siblings, 0 replies; 19+ messages in thread
From: Paolo Bonzini @ 2019-01-04 12:59 UTC (permalink / raw)
  To: Peter Maydell; +Cc: QEMU Developers

On 04/01/19 12:31, Peter Maydell wrote:
> I'll give this a go, but I think this will not necessarily be
> sufficient if some program invoked by make sets the O_NONBLOCK flag,
> and then make later in the same run tries to output and gets EINTR.
> Perhaps it would be better to put it in the test harness that
> invokes test binaries, so as to clear the flag as soon
> as possible? It could also then in theory print a warning
> if a test case had put stdin/stdout into non-blocking mode.

Aha, you gave me the clue I needed. :)  gtester redirected stdin/out/err
to /dev/null.

In the new harness, stdout is piped into tap-driver.pl, but everything
else is left in place.  Leaving stderr should be safe, while stdin is
the most likely culprit.  Adding </dev/null to the test invocation
should fix it; I'll send a new pull request either today or next Monday.

> (The semantics of O_NONBLOCK here seem to me to be completely
> broken. But they are what they are...)

Yes, and F_SETFL is not the only "interesting" fcntl in this respect;
fortunately QEMU is not using F_SETOWN anymore!

Paolo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PULL v4 00/35] Misc patches for 2018-12-21
  2019-01-04  7:59       ` Paolo Bonzini
  2019-01-04 10:06         ` Peter Maydell
@ 2019-01-04 13:03         ` Peter Maydell
  2019-01-04 13:34           ` Paolo Bonzini
  2019-01-04 13:45           ` Peter Maydell
  1 sibling, 2 replies; 19+ messages in thread
From: Peter Maydell @ 2019-01-04 13:03 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: QEMU Developers

On Fri, 4 Jan 2019 at 07:59, Paolo Bonzini <pbonzini@redhat.com> wrote:
> I don't know... I tried running make check under "strace -e fcntl" and I
> didn't find any occurrences of fcntl(1, O_SETFL, ...|O_NONBLOCK).

I found this strace command worked to track down some possible
culprits:
strace -o /tmp/strace.log -e fcntl,execve,clone -e signal=none -f make
-C build/x86 V=1 check

Extracts from the logs:

29251 execve("tests/test-char", ["tests/test-char", "-q", "-p",
"/char/stdio/subprocess", "--GTestSubprocess", "--GTestLogFD", "4"],
[/* 66 vars */] <unfinished ...>
29251 <... execve resumed> )            = 0
29251 fcntl(3, F_GETFD)                 = 0
29251 fcntl(3, F_SETFD, FD_CLOEXEC)     = 0
29251 clone(child_stack=0x7f43946addf0,
flags=CLONE_VM|CLONE_FS|CLONE_FILES|CLONE_SIGHAND|CLONE_THREAD|CLONE_SYSVSEM|CLONE_SETTLS|CLONE_PARENT_SETTID|CLONE_CHILD_CLEARTID,
parent_tidptr=0x7f43946ae9d0, tls=0x7f43946ae700,
child_tidptr=0x7f43946ae9d0) = 29254
29251 fcntl(4, F_GETFD)                 = 0
29251 fcntl(4, F_SETFD, FD_CLOEXEC)     = 0
29251 fcntl(4, F_GETFL)                 = 0x2 (flags O_RDWR)
29251 fcntl(4, F_SETFL, O_RDWR|O_NONBLOCK) = 0
29251 fcntl(0, F_GETFL)                 = 0x8000 (flags O_RDONLY|O_LARGEFILE)
29251 fcntl(0, F_GETFL)                 = 0x8000 (flags O_RDONLY|O_LARGEFILE)
29251 fcntl(0, F_SETFL, O_RDONLY|O_NONBLOCK|O_LARGEFILE) = 0
29251 fcntl(1, F_GETFL)                 = 0x1 (flags O_WRONLY)
29251 fcntl(1, F_SETFL, O_WRONLY|O_NONBLOCK) = 0
29251 fcntl(0, F_SETFL, O_RDONLY|O_LARGEFILE) = 0
29251 fcntl(0, F_SETFL, O_RDONLY|O_LARGEFILE) = -1 EBADF (Bad file descriptor)
29254 +++ exited with 0 +++
29251 +++ exited with 0 +++

(the attempt to F_SETFL a closed stdin is also interesting).

and

30300 execve("tests/hexloader-test", ["tests/hexloader-test",
"--quiet", "--keep-going", "-m
=quick", "--GTestLogFD=6"], [/* 68 vars */]) = 0
30300 fcntl(3, F_GETFD)                 = 0
30300 fcntl(3, F_SETFD, FD_CLOEXEC)     = 0
30300 fcntl(4, F_GETFD)                 = 0
30300 fcntl(4, F_SETFD, FD_CLOEXEC)     = 0
30300 clone(child_stack=0,
flags=CLONE_CHILD_CLEARTID|CLONE_CHILD_SETTID|SIGCHLD,
child_tidptr=0x7f26bdc09a10) = 30301
30301 execve("/bin/sh", ["sh", "-c", "exec
arm-softmmu/qemu-system-arm"...], [/* 69 vars */]) = 0
30301 execve("arm-softmmu/qemu-system-arm",
["arm-softmmu/qemu-system-arm", "-qtest",
"unix:/tmp/qtest-30300.sock,nowai"..., "-qtest-log", "/dev/null",
"-chardev", "socket,path=/tmp/qtest-30300.qmp"..., "-mon",
"chardev=char0,mode=control", "-machine", "accel=qtest", "-display",
"none", "-M", "vexpress-a9", "-nographic", ...], [/* 69 vars */]) = 0
30301 fcntl(3, F_GETFD)                 = 0
30301 fcntl(3, F_SETFD, FD_CLOEXEC)     = 0
30301 clone(child_stack=0x7f84f7614730,
flags=CLONE_VM|CLONE_FS|CLONE_FILES|CLONE_SIGHAND|CLONE_THREAD|CLONE_SYSVSEM|CLONE_SETTLS|CLONE_PARENT_SETTID|CLONE_CHILD_CLEARTID,
parent_tidptr=0x7f84f76179d0, tls=0x7f84f7617700,
child_tidptr=0x7f84f76179d0) = 30302
30301 fcntl(7, F_GETFD)                 = 0
30301 fcntl(7, F_SETFD, FD_CLOEXEC)     = 0
30301 fcntl(7, F_GETFL)                 = 0x2 (flags O_RDWR)
30301 fcntl(7, F_SETFL, O_RDWR|O_NONBLOCK) = 0
30301 fcntl(11, F_GETFL)                = 0x2 (flags O_RDWR)
30301 fcntl(11, F_SETFL, O_RDWR|O_NONBLOCK) = 0
30301 fcntl(12, F_GETFL)                = 0x2 (flags O_RDWR)
30301 fcntl(12, F_SETFL, O_RDWR|O_NONBLOCK) = 0
30301 clone(child_stack=0x7f84f6e13730,
flags=CLONE_VM|CLONE_FS|CLONE_FILES|CLONE_SIGHAND|CLONE_THREAD|CLONE_SYSVSEM|CLONE_SETTLS|CLONE_PARENT_SETTID|CLONE_CHILD_CLEARTID,
parent_tidptr=0x7f84f6e169d0, tls=0x7f84f6e16700,
child_tidptr=0x7f84f6e169d0) = 30303
30301 fcntl(0, F_GETFL)                 = 0x8000 (flags O_RDONLY|O_LARGEFILE)
30301 fcntl(0, F_GETFL)                 = 0x8000 (flags O_RDONLY|O_LARGEFILE)
30301 fcntl(0, F_SETFL, O_RDONLY|O_NONBLOCK|O_LARGEFILE) = 0
30301 fcntl(1, F_GETFL)                 = 0x8402 (flags
O_RDWR|O_APPEND|O_LARGEFILE)
30301 fcntl(1, F_SETFL, O_RDWR|O_APPEND|O_NONBLOCK|O_LARGEFILE) = 0
30301 clone(child_stack=0x7f84f6612730,
flags=CLONE_VM|CLONE_FS|CLONE_FILES|CLONE_SIGHAND|CLONE_THREAD|CLONE_SYSVSEM|CLONE_SETTLS|CLONE_PARENT_SETTID|CLONE_CHILD_CLEARTID,
parent_tidptr=0x7f84f66159d0, tls=0x7f84f6615700,
child_tidptr=0x7f84f66159d0) = 30304
30303 +++ exited with 0 +++
30301 fcntl(0, F_SETFL, O_RDONLY|O_LARGEFILE) = 0
30301 fcntl(0, F_SETFL, O_RDONLY|O_LARGEFILE) = -1 EBADF (Bad file descriptor)
30302 +++ exited with 0 +++
30301 +++ exited with 0 +++

where tests/hexloader-test starts a QEMU process which
makes stdin/stdout non-blocking.

I didn't try to also track dup/dup2/dup3 use to see whether fd 0/1/2
were duped to other fds and then set O_NONBLOCK via those, or
open to see if they were reopened with other contents.

(Test run is still in-progress, I'll update if there are any more
interesting bits of logging.)

thanks
-- PMM

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PULL v4 00/35] Misc patches for 2018-12-21
  2019-01-04 13:03         ` Peter Maydell
@ 2019-01-04 13:34           ` Paolo Bonzini
  2019-01-04 13:45           ` Peter Maydell
  1 sibling, 0 replies; 19+ messages in thread
From: Paolo Bonzini @ 2019-01-04 13:34 UTC (permalink / raw)
  To: Peter Maydell; +Cc: QEMU Developers

On 04/01/19 14:03, Peter Maydell wrote:
> On Fri, 4 Jan 2019 at 07:59, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> I don't know... I tried running make check under "strace -e fcntl" and I
>> didn't find any occurrences of fcntl(1, O_SETFL, ...|O_NONBLOCK).
> 
> I found this strace command worked to track down some possible
> culprits:
> strace -o /tmp/strace.log -e fcntl,execve,clone -e signal=none -f make
> -C build/x86 V=1 check
> 
> Extracts from the logs:
> 
> 29251 execve("tests/test-char", ["tests/test-char", "-q", "-p",
> "/char/stdio/subprocess", "--GTestSubprocess", "--GTestLogFD", "4"],
> [/* 66 vars */] <unfinished ...>
> 29251 <... execve resumed> )            = 0
> 29251 fcntl(3, F_GETFD)                 = 0
> 29251 fcntl(3, F_SETFD, FD_CLOEXEC)     = 0
> 29251 clone(child_stack=0x7f43946addf0,
> flags=CLONE_VM|CLONE_FS|CLONE_FILES|CLONE_SIGHAND|CLONE_THREAD|CLONE_SYSVSEM|CLONE_SETTLS|CLONE_PARENT_SETTID|CLONE_CHILD_CLEARTID,
> parent_tidptr=0x7f43946ae9d0, tls=0x7f43946ae700,
> child_tidptr=0x7f43946ae9d0) = 29254
> 29251 fcntl(4, F_GETFD)                 = 0
> 29251 fcntl(4, F_SETFD, FD_CLOEXEC)     = 0
> 29251 fcntl(4, F_GETFL)                 = 0x2 (flags O_RDWR)
> 29251 fcntl(4, F_SETFL, O_RDWR|O_NONBLOCK) = 0
> 29251 fcntl(0, F_GETFL)                 = 0x8000 (flags O_RDONLY|O_LARGEFILE)
> 29251 fcntl(0, F_GETFL)                 = 0x8000 (flags O_RDONLY|O_LARGEFILE)
> 29251 fcntl(0, F_SETFL, O_RDONLY|O_NONBLOCK|O_LARGEFILE) = 0
> 29251 fcntl(1, F_GETFL)                 = 0x1 (flags O_WRONLY)
> 29251 fcntl(1, F_SETFL, O_WRONLY|O_NONBLOCK) = 0
> 29251 fcntl(0, F_SETFL, O_RDONLY|O_LARGEFILE) = 0
> 29251 fcntl(0, F_SETFL, O_RDONLY|O_LARGEFILE) = -1 EBADF (Bad file descriptor)
> 29254 +++ exited with 0 +++
> 29251 +++ exited with 0 +++

This one is okay, since the subprocess is using a pipe for stdin and stdout.

QEMU restores O_NONBLOCK at exit (see term_exit in
chardev/char-stdio.c); however, for parallel make and no --output-sync,
a small window does remain where make sees a nonblocking stdout.  Even
that wouldn't explain why the second make inside the while loop always
sees a nonblocking stdout, while the first never does.

Paolo

> (the attempt to F_SETFL a closed stdin is also interesting).
> 
> and
> 
> 30300 execve("tests/hexloader-test", ["tests/hexloader-test",
> "--quiet", "--keep-going", "-m
> =quick", "--GTestLogFD=6"], [/* 68 vars */]) = 0
> 30300 fcntl(3, F_GETFD)                 = 0
> 30300 fcntl(3, F_SETFD, FD_CLOEXEC)     = 0
> 30300 fcntl(4, F_GETFD)                 = 0
> 30300 fcntl(4, F_SETFD, FD_CLOEXEC)     = 0
> 30300 clone(child_stack=0,
> flags=CLONE_CHILD_CLEARTID|CLONE_CHILD_SETTID|SIGCHLD,
> child_tidptr=0x7f26bdc09a10) = 30301
> 30301 execve("/bin/sh", ["sh", "-c", "exec
> arm-softmmu/qemu-system-arm"...], [/* 69 vars */]) = 0
> 30301 execve("arm-softmmu/qemu-system-arm",
> ["arm-softmmu/qemu-system-arm", "-qtest",
> "unix:/tmp/qtest-30300.sock,nowai"..., "-qtest-log", "/dev/null",
> "-chardev", "socket,path=/tmp/qtest-30300.qmp"..., "-mon",
> "chardev=char0,mode=control", "-machine", "accel=qtest", "-display",
> "none", "-M", "vexpress-a9", "-nographic", ...], [/* 69 vars */]) = 0
> 30301 fcntl(3, F_GETFD)                 = 0
> 30301 fcntl(3, F_SETFD, FD_CLOEXEC)     = 0
> 30301 clone(child_stack=0x7f84f7614730,
> flags=CLONE_VM|CLONE_FS|CLONE_FILES|CLONE_SIGHAND|CLONE_THREAD|CLONE_SYSVSEM|CLONE_SETTLS|CLONE_PARENT_SETTID|CLONE_CHILD_CLEARTID,
> parent_tidptr=0x7f84f76179d0, tls=0x7f84f7617700,
> child_tidptr=0x7f84f76179d0) = 30302
> 30301 fcntl(7, F_GETFD)                 = 0
> 30301 fcntl(7, F_SETFD, FD_CLOEXEC)     = 0
> 30301 fcntl(7, F_GETFL)                 = 0x2 (flags O_RDWR)
> 30301 fcntl(7, F_SETFL, O_RDWR|O_NONBLOCK) = 0
> 30301 fcntl(11, F_GETFL)                = 0x2 (flags O_RDWR)
> 30301 fcntl(11, F_SETFL, O_RDWR|O_NONBLOCK) = 0
> 30301 fcntl(12, F_GETFL)                = 0x2 (flags O_RDWR)
> 30301 fcntl(12, F_SETFL, O_RDWR|O_NONBLOCK) = 0
> 30301 clone(child_stack=0x7f84f6e13730,
> flags=CLONE_VM|CLONE_FS|CLONE_FILES|CLONE_SIGHAND|CLONE_THREAD|CLONE_SYSVSEM|CLONE_SETTLS|CLONE_PARENT_SETTID|CLONE_CHILD_CLEARTID,
> parent_tidptr=0x7f84f6e169d0, tls=0x7f84f6e16700,
> child_tidptr=0x7f84f6e169d0) = 30303
> 30301 fcntl(0, F_GETFL)                 = 0x8000 (flags O_RDONLY|O_LARGEFILE)
> 30301 fcntl(0, F_GETFL)                 = 0x8000 (flags O_RDONLY|O_LARGEFILE)
> 30301 fcntl(0, F_SETFL, O_RDONLY|O_NONBLOCK|O_LARGEFILE) = 0
> 30301 fcntl(1, F_GETFL)                 = 0x8402 (flags
> O_RDWR|O_APPEND|O_LARGEFILE)
> 30301 fcntl(1, F_SETFL, O_RDWR|O_APPEND|O_NONBLOCK|O_LARGEFILE) = 0
> 30301 clone(child_stack=0x7f84f6612730,
> flags=CLONE_VM|CLONE_FS|CLONE_FILES|CLONE_SIGHAND|CLONE_THREAD|CLONE_SYSVSEM|CLONE_SETTLS|CLONE_PARENT_SETTID|CLONE_CHILD_CLEARTID,
> parent_tidptr=0x7f84f66159d0, tls=0x7f84f6615700,
> child_tidptr=0x7f84f66159d0) = 30304
> 30303 +++ exited with 0 +++
> 30301 fcntl(0, F_SETFL, O_RDONLY|O_LARGEFILE) = 0
> 30301 fcntl(0, F_SETFL, O_RDONLY|O_LARGEFILE) = -1 EBADF (Bad file descriptor)
> 30302 +++ exited with 0 +++
> 30301 +++ exited with 0 +++
> 
> where tests/hexloader-test starts a QEMU process which
> makes stdin/stdout non-blocking.
> 
> I didn't try to also track dup/dup2/dup3 use to see whether fd 0/1/2
> were duped to other fds and then set O_NONBLOCK via those, or
> open to see if they were reopened with other contents.
> 
> (Test run is still in-progress, I'll update if there are any more
> interesting bits of logging.)
> 
> thanks
> -- PMM
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PULL v4 00/35] Misc patches for 2018-12-21
  2019-01-04 13:03         ` Peter Maydell
  2019-01-04 13:34           ` Paolo Bonzini
@ 2019-01-04 13:45           ` Peter Maydell
  1 sibling, 0 replies; 19+ messages in thread
From: Peter Maydell @ 2019-01-04 13:45 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: QEMU Developers

On Fri, 4 Jan 2019 at 13:03, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Fri, 4 Jan 2019 at 07:59, Paolo Bonzini <pbonzini@redhat.com> wrote:
> > I don't know... I tried running make check under "strace -e fcntl" and I
> > didn't find any occurrences of fcntl(1, O_SETFL, ...|O_NONBLOCK).
>
> I found this strace command worked to track down some possible
> culprits:
> strace -o /tmp/strace.log -e fcntl,execve,clone -e signal=none -f make
> -C build/x86 V=1 check
>
> Extracts from the logs:
>
> 29251 execve("tests/test-char", ["tests/test-char", "-q", "-p",
> "/char/stdio/subprocess", "--GTestSubprocess", "--GTestLogFD", "4"],
> [/* 66 vars */] <unfinished ...>

This one seems to be ok, because the call to g_test_trap_subprocess()
does not pass any flags arguments, which means the child gets
its stdin/stdout/stderr all set to /dev/null, and the change
to O_NONBLOCK is thus harmless.

> 30300 execve("tests/hexloader-test", ["tests/hexloader-test",
> "--quiet", "--keep-going", "-m
> =quick", "--GTestLogFD=6"], [/* 68 vars */]) = 0

This one on the other hand does seem like it might be an issue.
Removing the "-nographic" from the qtest_initf() command
causes it no longer to mess with the O_NONBLOCK state
of its stdout. I'm just doing a 'while loop' test run to
see if it fixes the problems I was seeing with that.

It seems a touch brittle if a harmless-looking command
line option to QEMU can break test runs, though -- is
there a way we can make this more robust, eg by having
the QEMU process i/o always redirected ?

thanks
-- PMM

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PULL 23/35] test: replace gtester with a TAP driver
  2019-01-07 22:31 [Qemu-devel] [PULL v4 00/35] Misc patches for 2018-12-18 Paolo Bonzini
@ 2019-01-07 22:31 ` Paolo Bonzini
  0 siblings, 0 replies; 19+ messages in thread
From: Paolo Bonzini @ 2019-01-07 22:31 UTC (permalink / raw)
  To: qemu-devel

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>
Message-Id: <1543513531-1151-3-git-send-email-pbonzini@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rules.mak                                    |   4 +-
 scripts/gtester-cat                          |  26 --
 scripts/tap-driver.pl                        | 379 +++++++++++++++++++
 scripts/tap-merge.pl                         | 111 ++++++
 tests/Makefile.include                       |  75 ++--
 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 +
 13 files changed, 551 insertions(+), 52 deletions(-)
 delete mode 100755 scripts/gtester-cat
 create mode 100755 scripts/tap-driver.pl
 create mode 100755 scripts/tap-merge.pl

diff --git a/rules.mak b/rules.mak
index bbb2667928..86e033d815 100644
--- a/rules.mak
+++ b/rules.mak
@@ -132,7 +132,9 @@ modules:
 #  otherwise print the 'quiet' output in the format "  NAME     args to print"
 # NAME should be a short name of the command, 7 letters or fewer.
 # If called with only a single argument, will print nothing in quiet mode.
-quiet-command = $(if $(V),$1,$(if $(2),@printf "  %-7s %s\n" $2 $3 && $1, @$1))
+quiet-command-run = $(if $(V),,$(if $2,printf "  %-7s %s\n" $2 $3 && ))$1
+quiet-@ = $(if $(V),,@)
+quiet-command = $(quiet-@)$(call quiet-command-run,$1,$2,$3)
 
 # cc-option
 # Usage: CFLAGS+=$(call cc-option, -falign-functions=0, -malign-functions=0)
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..6621a5cd67
--- /dev/null
+++ b/scripts/tap-driver.pl
@@ -0,0 +1,379 @@
+#! /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 });
+
+  STDOUT->autoflush(1);
+  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..10ccf57bb2
--- /dev/null
+++ b/scripts/tap-merge.pl
@@ -0,0 +1,111 @@
+#! /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.
+
+  STDOUT->autoflush(1);
+  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 3f5a1d0c30..51e9c16e98 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -808,41 +808,68 @@ 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
+
+define do_test_human_k
+        $(quiet-@)rc=0; $(foreach COMMAND, $1, \
+          $(call quiet-command-run, \
+            export MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$(( $${RANDOM:-0} % 255 + 1))} $2; \
+              $(COMMAND) -m=$(SPEED) -k --tap < /dev/null \
+              | ./scripts/tap-driver.pl --test-name="$(notdir $(COMMAND))" $(if $(V),, --show-failures-only) \
+              || rc=$$?;, "TEST", "$@: $(COMMAND)")) exit $$rc
+endef
+define do_test_human_no_k
+        $(foreach COMMAND, $1, \
+          $(call quiet-command, \
+            MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$(( $${RANDOM:-0} % 255 + 1))} $2 \
+              $(COMMAND) -m=$(SPEED) -k --tap < /dev/null \
+              | ./scripts/tap-driver.pl --test-name="$(notdir $(COMMAND))" $(if $(V),, --show-failures-only), \
+              "TEST", "$@: $(COMMAND)")
+)
+endef
+do_test_human = \
+        $(if $(findstring k, $(MAKEFLAGS)), $(do_test_human_k), $(do_test_human_no_k))
+
+define do_test_tap
+	$(call quiet-command, \
+          { export MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$(( $${RANDOM:-0} % 255 + 1))} $2; \
+            $(foreach COMMAND, $1, \
+	      $(COMMAND) -m=$(SPEED) -k --tap < /dev/null \
+              | sed "s/^[a-z][a-z]* [0-9]* /&$(notdir $(COMMAND)) /" || true; ) } \
+	      | ./scripts/tap-merge.pl | tee "$@" \
+	      | ./scripts/tap-driver.pl $(if $(V),, --show-failures-only), \
+	  "TAP","$@")
+endef
 
 .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","$*")
+check-unit: $(check-unit-y)
+	$(call do_test_human, $^)
 
-# gtester tests with XML output
+check-speed: $(check-speed-y)
+	$(call do_test_human, $^)
 
-$(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","$@")
+# gtester tests with TAP output
 
-check-report-unit.xml: $(check-unit-y)
-	$(call quiet-command,gtester -q $(GTESTER_OPTIONS) -o $@ -m=$(SPEED) $^,"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))
 
-# Reports and overall runs
+check-report-unit.tap: $(check-unit-y)
+	$(call do_test_tap,$^)
 
-check-report.xml: $(patsubst %,check-report-qtest-%.xml, $(QTEST_TARGETS)) check-report-unit.xml
-	$(call quiet-command,$(SRC_PATH)/scripts/gtester-cat $^ > $@,"GEN","$@")
+# Reports and overall runs
 
-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
 
@@ -957,8 +984,6 @@ check-acceptance: check-venv $(TESTS_RESULTS_DIR)
 .PHONY: check-qapi-schema check-qtest check-unit check check-clean
 check-qapi-schema: $(patsubst %,check-%, $(check-qapi-schema-y)) check-tests/qapi-schema/doc-good.texi
 check-qtest: $(patsubst %,check-qtest-%, $(QTEST_TARGETS))
-check-unit: $(patsubst %,check-%, $(check-unit-y))
-check-speed: $(patsubst %,check-%, $(check-speed-y))
 check-block: $(patsubst %,check-%, $(check-block-y))
 check: check-qapi-schema check-unit check-qtest check-decodetree
 check-clean:
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.20.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PULL 23/35] test: replace gtester with a TAP driver
  2019-01-09 22:09 [Qemu-devel] [PULL v6 00/35] Misc patches for 2018-12-18 Paolo Bonzini
@ 2019-01-09 22:09 ` Paolo Bonzini
  0 siblings, 0 replies; 19+ messages in thread
From: Paolo Bonzini @ 2019-01-09 22:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: 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>
Message-Id: <1543513531-1151-3-git-send-email-pbonzini@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rules.mak                                    |   4 +-
 scripts/gtester-cat                          |  26 --
 scripts/tap-driver.pl                        | 378 +++++++++++++++++++
 scripts/tap-merge.pl                         | 110 ++++++
 tests/Makefile.include                       |  75 ++--
 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 +
 13 files changed, 549 insertions(+), 52 deletions(-)
 delete mode 100755 scripts/gtester-cat
 create mode 100755 scripts/tap-driver.pl
 create mode 100755 scripts/tap-merge.pl

diff --git a/rules.mak b/rules.mak
index bbb2667928..86e033d815 100644
--- a/rules.mak
+++ b/rules.mak
@@ -132,7 +132,9 @@ modules:
 #  otherwise print the 'quiet' output in the format "  NAME     args to print"
 # NAME should be a short name of the command, 7 letters or fewer.
 # If called with only a single argument, will print nothing in quiet mode.
-quiet-command = $(if $(V),$1,$(if $(2),@printf "  %-7s %s\n" $2 $3 && $1, @$1))
+quiet-command-run = $(if $(V),,$(if $2,printf "  %-7s %s\n" $2 $3 && ))$1
+quiet-@ = $(if $(V),,@)
+quiet-command = $(quiet-@)$(call quiet-command-run,$1,$2,$3)
 
 # cc-option
 # Usage: CFLAGS+=$(call cc-option, -falign-functions=0, -malign-functions=0)
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 9c84bbd829..c17f6d5dfa 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -810,41 +810,68 @@ 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
+
+define do_test_human_k
+        $(quiet-@)rc=0; $(foreach COMMAND, $1, \
+          $(call quiet-command-run, \
+            export MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$(( $${RANDOM:-0} % 255 + 1))} $2; \
+              $(COMMAND) -m=$(SPEED) -k --tap < /dev/null \
+              | ./scripts/tap-driver.pl --test-name="$(notdir $(COMMAND))" $(if $(V),, --show-failures-only) \
+              || rc=$$?;, "TEST", "$@: $(COMMAND)")) exit $$rc
+endef
+define do_test_human_no_k
+        $(foreach COMMAND, $1, \
+          $(call quiet-command, \
+            MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$(( $${RANDOM:-0} % 255 + 1))} $2 \
+              $(COMMAND) -m=$(SPEED) -k --tap < /dev/null \
+              | ./scripts/tap-driver.pl --test-name="$(notdir $(COMMAND))" $(if $(V),, --show-failures-only), \
+              "TEST", "$@: $(COMMAND)")
+)
+endef
+do_test_human = \
+        $(if $(findstring k, $(MAKEFLAGS)), $(do_test_human_k), $(do_test_human_no_k))
+
+define do_test_tap
+	$(call quiet-command, \
+          { export MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$(( $${RANDOM:-0} % 255 + 1))} $2; \
+            $(foreach COMMAND, $1, \
+	      $(COMMAND) -m=$(SPEED) -k --tap < /dev/null \
+	      | sed "s/^[a-z][a-z]* [0-9]* /&$(notdir $(COMMAND)) /" || true; ) } \
+	      | ./scripts/tap-merge.pl | tee "$@" \
+	      | ./scripts/tap-driver.pl $(if $(V),, --show-failures-only), \
+	  "TAP","$@")
+endef
 
 .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","$*")
+check-unit: $(check-unit-y)
+	$(call do_test_human, $^)
 
-# gtester tests with XML output
+check-speed: $(check-speed-y)
+	$(call do_test_human, $^)
 
-$(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","$@")
+# gtester tests with TAP output
 
-check-report-unit.xml: $(check-unit-y)
-	$(call quiet-command,gtester -q $(GTESTER_OPTIONS) -o $@ -m=$(SPEED) $^,"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))
 
-# Reports and overall runs
+check-report-unit.tap: $(check-unit-y)
+	$(call do_test_tap,$^)
 
-check-report.xml: $(patsubst %,check-report-qtest-%.xml, $(QTEST_TARGETS)) check-report-unit.xml
-	$(call quiet-command,$(SRC_PATH)/scripts/gtester-cat $^ > $@,"GEN","$@")
+# Reports and overall runs
 
-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
 
@@ -959,8 +986,6 @@ check-acceptance: check-venv $(TESTS_RESULTS_DIR)
 .PHONY: check-qapi-schema check-qtest check-unit check check-clean
 check-qapi-schema: $(patsubst %,check-%, $(check-qapi-schema-y)) check-tests/qapi-schema/doc-good.texi
 check-qtest: $(patsubst %,check-qtest-%, $(QTEST_TARGETS))
-check-unit: $(patsubst %,check-%, $(check-unit-y))
-check-speed: $(patsubst %,check-%, $(check-speed-y))
 check-block: $(patsubst %,check-%, $(check-block-y))
 check: check-qapi-schema check-unit check-qtest check-decodetree
 check-clean:
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.20.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2019-01-09 22:22 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-12-21 12:36 [Qemu-devel] [PULL v4 00/35] Misc patches for 2018-12-21 Paolo Bonzini
2018-12-21 12:36 ` [Qemu-devel] [PULL 22/35] test: execute g_test_run when tests are skipped Paolo Bonzini
2018-12-21 12:36 ` [Qemu-devel] [PULL 23/35] test: replace gtester with a TAP driver Paolo Bonzini
2018-12-21 21:09 ` [Qemu-devel] [PULL v4 00/35] Misc patches for 2018-12-21 Peter Maydell
2018-12-22  8:41   ` Paolo Bonzini
2018-12-22 11:26     ` Peter Maydell
2019-01-03 18:37     ` Peter Maydell
2019-01-04  7:59       ` Paolo Bonzini
2019-01-04 10:06         ` Peter Maydell
2019-01-04 11:01           ` Paolo Bonzini
2019-01-04 11:31             ` Peter Maydell
2019-01-04 12:59               ` Paolo Bonzini
2019-01-04 13:03         ` Peter Maydell
2019-01-04 13:34           ` Paolo Bonzini
2019-01-04 13:45           ` Peter Maydell
  -- strict thread matches above, loose matches on Subject: below --
2019-01-09 22:09 [Qemu-devel] [PULL v6 00/35] Misc patches for 2018-12-18 Paolo Bonzini
2019-01-09 22:09 ` [Qemu-devel] [PULL 23/35] test: replace gtester with a TAP driver Paolo Bonzini
2019-01-07 22:31 [Qemu-devel] [PULL v4 00/35] Misc patches for 2018-12-18 Paolo Bonzini
2019-01-07 22:31 ` [Qemu-devel] [PULL 23/35] test: replace gtester with a TAP driver Paolo Bonzini
2018-12-19 15:18 [Qemu-devel] [PULL v2 00/35] Misc patches for 2018-12-18 Paolo Bonzini
2018-12-19 15:19 ` [Qemu-devel] [PULL 23/35] test: replace gtester with a TAP driver Paolo Bonzini
2018-12-17 23:16 [Qemu-devel] [PULL 00/35] Misc patches for 2018-12-18 Paolo Bonzini
2018-12-17 23:16 ` [Qemu-devel] [PULL 23/35] test: replace gtester with a TAP driver Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).