All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/4] Replace custom test harness with "meson test"
@ 2021-10-15 10:07 Paolo Bonzini
  2021-10-15 10:07 ` [PATCH 1/4] build: use "meson test" as the test harness Paolo Bonzini
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Paolo Bonzini @ 2021-10-15 10:07 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, thuth, vsementsov, qemu-block, hreitz, marcandre.lureau

Hi all,

Starting with Meson 0.57, "meson test" has all features of QEMU's
makefile-based harness and more.  In particular, some features that
were added to reach feature parity are:

* print reproducer command line for each failed test right below the test

* keep the output of multiple (non-TAP) tests together in verbose mode,
  similar to "make --output-sync target"

* report on TAP subtests as they are encountered

It also includes nicer handling of test interruption, logging of the run
in the meson-logs/ subdirectory, and a progress report/spinner.  For
these reasons it would be nice to adopt it and remove the Perl scripts
that we have to present TAP output nicely.  While at it, this series
also changes qemu-iotests to be described in tests/qemu-iotests/meson.build
and it replaces the current "-makecheck" output style with TAP.

This is an RFC just to let people try it out and because I still haven't
sorted out timeouts (e.g.  OpenBSD is insanely slow on some tests).
But in theory, if you want to try it out, the only things you need to
know are:

* while "make check" does not timeout tests like it doesn't right now,
  "meson test" does respect timeouts, so you could get failures because
  of that.  That can and probably will change from RFC to final.

* CTRL+C will only interrupt the longest running test.  Pressing
  CTRL+C repeatedly three times (which you would likely do anyway,
  that's how things work) interrupts the whole run

* Right now "make check-block" only does a single test run just like
  "../tests/check-block.sh", but it would be possible to add the thorough
  suite to "meson test --suite block" as well.

* If you were using make check-report.tap and similar, they are replaced
  by targets like make check-report.junit.xml.  This is because Gitlab
  is able to parse the resulting XML and include on the website a report
  of which tests failed.

An example of non-verbose "make check" output is available at
https://gitlab.com/bonzini/qemu/-/jobs/1680980620.  A verbose run
instead is like https://asciinema.org/a/e5irnEszSnAheOHM30exbo3F6
(does not include check-block).

Paolo

Paolo Bonzini (4):
  build: use "meson test" as the test harness
  build: make check-block a meson test
  check-block: replace -makecheck with TAP output
  configure: remove dead EXESUF variable

 .gitlab-ci.d/crossbuild-template.yml |   2 +-
 Makefile                             |   3 +-
 configure                            |   3 -
 meson.build                          |   5 +-
 scripts/mtest2make.py                | 111 +++-----
 scripts/tap-driver.pl                | 379 ---------------------------
 scripts/tap-merge.pl                 | 111 --------
 tests/Makefile.include               |  15 +-
 tests/check-block.sh                 |  28 +-
 tests/fp/meson.build                 |   2 +-
 tests/meson.build                    |   1 +
 tests/qemu-iotests/check             |   6 +-
 tests/qemu-iotests/meson.build       |  14 +
 tests/qemu-iotests/testenv.py        |  30 +--
 tests/qemu-iotests/testrunner.py     |  48 ++--
 15 files changed, 121 insertions(+), 637 deletions(-)
 delete mode 100755 scripts/tap-driver.pl
 delete mode 100755 scripts/tap-merge.pl
 create mode 100644 tests/qemu-iotests/meson.build

-- 
2.31.1



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

* [PATCH 1/4] build: use "meson test" as the test harness
  2021-10-15 10:07 [RFC PATCH 0/4] Replace custom test harness with "meson test" Paolo Bonzini
@ 2021-10-15 10:07 ` Paolo Bonzini
  2021-10-15 10:07 ` [PATCH 2/4] build: make check-block a meson test Paolo Bonzini
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2021-10-15 10:07 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, thuth, vsementsov, qemu-block, hreitz, marcandre.lureau

"meson test" starting with version 0.57 is just as capable and easy to
use as QEMU's own TAP driver.  All existing options for "make check"
work.  The only required code change involves how to mark "slow" tests.

scripts/mtest2make.py is changed to invoke "meson test" using a trick
similar to how the Makefile already invokes ninja, i.e. turning the
MAKECMDGOALS into command line options.  Tests and their dependencies
are still rebuilt correctly before "meson test" is run.

The rules for .tap output are replaced by JUnit XML; GitLab is able to
parse that output and present it in the CI pipeline report.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 Makefile              |   3 +-
 meson.build           |   5 +-
 scripts/mtest2make.py | 105 ++++--------
 scripts/tap-driver.pl | 379 ------------------------------------------
 scripts/tap-merge.pl  | 111 -------------
 tests/fp/meson.build  |   2 +-
 6 files changed, 42 insertions(+), 563 deletions(-)
 delete mode 100755 scripts/tap-driver.pl
 delete mode 100755 scripts/tap-merge.pl

diff --git a/Makefile b/Makefile
index fe9415ac64..c680b5e176 100644
--- a/Makefile
+++ b/Makefile
@@ -145,7 +145,8 @@ NINJAFLAGS = $(if $V,-v) $(if $(MAKE.n), -n) $(if $(MAKE.k), -k0) \
         $(filter-out -j, $(lastword -j1 $(filter -l% -j%, $(MAKEFLAGS)))) \
 
 ninja-cmd-goals = $(or $(MAKECMDGOALS), all)
-ninja-cmd-goals += $(foreach t, $(.tests), $(.test.deps.$t))
+ninja-cmd-goals += $(foreach t, $(.check.build-suites) $(.check.mtest-suites), $(.check-$t.deps))
+ninja-cmd-goals += $(foreach t, $(.bench.build-suites) $(.check.mtest-suites), $(.bench-$t.deps))
 
 makefile-targets := build.ninja ctags TAGS cscope dist clean uninstall
 # "ninja -t targets" also lists all prerequisites.  If build system
diff --git a/meson.build b/meson.build
index 6b7487b725..c7af557afb 100644
--- a/meson.build
+++ b/meson.build
@@ -1,8 +1,11 @@
 project('qemu', ['c'], meson_version: '>=0.58.2',
         default_options: ['warning_level=1', 'c_std=gnu11', 'cpp_std=gnu++11', 'b_colorout=auto',
-                          'b_staticpic=false'],
+                          'b_staticpic=false', 'stdsplit=false'],
         version: files('VERSION'))
 
+add_test_setup('quick', exclude_suites: 'slow', is_default: true)
+add_test_setup('slow', env: ['G_TEST_SLOW=1'])
+
 not_found = dependency('', required: false)
 keyval = import('keyval')
 ss = import('sourceset')
diff --git a/scripts/mtest2make.py b/scripts/mtest2make.py
index 02c0453e67..b33c1d48df 100644
--- a/scripts/mtest2make.py
+++ b/scripts/mtest2make.py
@@ -13,101 +13,68 @@
 
 class Suite(object):
     def __init__(self):
-        self.tests = list()
-        self.slow_tests = list()
-        self.executables = set()
+        self.deps = set()
 
 print('''
 SPEED = quick
 
-# $1 = environment, $2 = test command, $3 = test name, $4 = dir
-.test-human-tap = $1 $(if $4,(cd $4 && $2),$2) -m $(SPEED) < /dev/null | ./scripts/tap-driver.pl --test-name="$3" $(if $(V),,--show-failures-only)
-.test-human-exitcode = $1 $(PYTHON) scripts/test-driver.py $(if $4,-C$4) $(if $(V),--verbose) -- $2 < /dev/null
-.test-tap-tap = $1 $(if $4,(cd $4 && $2),$2) < /dev/null | sed "s/^[a-z][a-z]* [0-9]*/& $3/" || true
-.test-tap-exitcode = printf "%s\\n" 1..1 "`$1 $(if $4,(cd $4 && $2),$2) < /dev/null > /dev/null || echo "not "`ok 1 $3"
-.test.human-print = echo $(if $(V),'$1 $2','Running test $3') &&
-.test.env = MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$(( $${RANDOM:-0} % 255 + 1))}
+.mtestargs = --no-rebuild -t 0
+.mtestargs += $(subst -j,--num-processes , $(filter-out -j, $(lastword -j1 $(filter -j%, $(MAKEFLAGS)))))
+ifneq ($(SPEED), quick)
+.mtestargs += --setup $(SPEED)
+endif
 
-# $1 = test name, $2 = test target (human or tap)
-.test.run = $(call .test.$2-print,$(.test.env.$1),$(.test.cmd.$1),$(.test.name.$1)) $(call .test-$2-$(.test.driver.$1),$(.test.env.$1),$(.test.cmd.$1),$(.test.name.$1),$(.test.dir.$1))
-
-.test.output-format = human
+.check.mtestargs = $(MTESTARGS) $(.mtestargs) $(if $(V),--verbose,--print-errorlogs)
+.bench.mtestargs = $(MTESTARGS) $(.mtestargs) --benchmark --verbose
 ''')
 
 introspect = json.load(sys.stdin)
-i = 0
 
 def process_tests(test, targets, suites):
-    global i
-    env = ' '.join(('%s=%s' % (shlex.quote(k), shlex.quote(v))
-                    for k, v in test['env'].items()))
     executable = test['cmd'][0]
     try:
         executable = os.path.relpath(executable)
     except:
         pass
-    if test['workdir'] is not None:
-        try:
-            test['cmd'][0] = os.path.relpath(executable, test['workdir'])
-        except:
-            test['cmd'][0] = executable
-    else:
-        test['cmd'][0] = executable
-    cmd = ' '.join((shlex.quote(x) for x in test['cmd']))
-    driver = test['protocol'] if 'protocol' in test else 'exitcode'
-
-    i += 1
-    if test['workdir'] is not None:
-        print('.test.dir.%d := %s' % (i, shlex.quote(test['workdir'])))
 
     deps = (targets.get(x, []) for x in test['depends'])
     deps = itertools.chain.from_iterable(deps)
-
-    print('.test.name.%d := %s' % (i, test['name']))
-    print('.test.driver.%d := %s' % (i, driver))
-    print('.test.env.%d := $(.test.env) %s' % (i, env))
-    print('.test.cmd.%d := %s' % (i, cmd))
-    print('.test.deps.%d := %s' % (i, ' '.join(deps)))
-    print('.PHONY: run-test-%d' % (i,))
-    print('run-test-%d: $(.test.deps.%d)' % (i,i))
-    print('\t@$(call .test.run,%d,$(.test.output-format))' % (i,))
+    deps = list(deps)
 
     test_suites = test['suite'] or ['default']
-    is_slow = any(s.endswith('-slow') for s in test_suites)
     for s in test_suites:
         # The suite name in the introspection info is "PROJECT:SUITE"
         s = s.split(':')[1]
-        if s.endswith('-slow'):
-            s = s[:-5]
-        if is_slow:
-            suites[s].slow_tests.append(i)
-        else:
-            suites[s].tests.append(i)
-        suites[s].executables.add(executable)
+        if s == 'slow':
+            continue
+        suites[s].deps.update(deps)
 
 def emit_prolog(suites, prefix):
-    all_tap = ' '.join(('%s-report-%s.tap' % (prefix, k) for k in suites.keys()))
-    print('.PHONY: %s %s-report.tap %s' % (prefix, prefix, all_tap))
-    print('%s: run-tests' % (prefix,))
-    print('%s-report.tap %s: %s-report%%.tap: all' % (prefix, all_tap, prefix))
-    print('''\t$(MAKE) .test.output-format=tap --quiet -Otarget V=1 %s$* | ./scripts/tap-merge.pl | tee "$@" \\
-              | ./scripts/tap-driver.pl $(if $(V),, --show-failures-only)''' % (prefix, ))
+    all_targets = ' '.join((f'{prefix}-{k}' for k in suites.keys()))
+    all_xml = ' '.join((f'{prefix}-report-{k}.junit.xml' for k in suites.keys()))
+    print(f'all-{prefix}-targets = {all_targets}')
+    print(f'all-{prefix}-xml = {all_xml}')
+    print(f'.PHONY: {prefix} do-meson-{prefix} {prefix}-report.junit.xml $(all-{prefix}-targets) $(all-{prefix}-xml)')
+    print(f'ifeq ($(filter {prefix}, $(MAKECMDGOALS)),)')
+    print(f'.{prefix}.mtestargs += $(foreach s,$(sort $(.{prefix}.mtest-suites)), --suite $s)')
+    print(f'endif')
+    print(f'{prefix}-build: run-ninja')
+    print(f'{prefix} $(all-{prefix}-targets): do-meson-{prefix}')
+    print(f'do-meson-{prefix}: run-ninja; $(if $(MAKE.n),,+)$(MESON) test $(.{prefix}.mtestargs)')
+    print(f'{prefix}-report.junit.xml $(all-{prefix}-xml): {prefix}-report%.junit.xml: run-ninja')
+    print(f'\t$(MAKE) {prefix}$* MTESTARGS="$(MTESTARGS) --logbase {prefix}-report$*" && ln -f meson-logs/$@ .')
 
 def emit_suite(name, suite, prefix):
-    executables = ' '.join(suite.executables)
-    slow_test_numbers = ' '.join((str(x) for x in suite.slow_tests))
-    test_numbers = ' '.join((str(x) for x in suite.tests))
-    target = '%s-%s' % (prefix, name)
-    print('.test.quick.%s := %s' % (target, test_numbers))
-    print('.test.slow.%s := $(.test.quick.%s) %s' % (target, target, slow_test_numbers))
-    print('%s-build: %s' % (prefix, executables))
-    print('.PHONY: %s' % (target, ))
-    print('.PHONY: %s-report-%s.tap' % (prefix, name))
-    print('%s: run-tests' % (target, ))
-    print('ifneq ($(filter %s %s, $(MAKECMDGOALS)),)' % (target, prefix))
-    print('.tests += $(.test.$(SPEED).%s)' % (target, ))
-    print('endif')
-    print('all-%s-targets += %s' % (prefix, target))
+    deps = ' '.join(suite.deps)
+    print(f'.{prefix}-{name}.deps = {deps}')
+    print(f'ifneq ($(filter {prefix}-build, $(MAKECMDGOALS)),)')
+    print(f'.{prefix}.build-suites += {name}')
+    print(f'endif')
+
+    targets = f'{prefix}-{name} {prefix}-report-{name}.junit.xml {prefix} {prefix}-report.junit.xml'
+    print(f'ifneq ($(filter {targets}, $(MAKECMDGOALS)),)')
+    print(f'.{prefix}.mtest-suites += {name}')
+    print(f'endif')
 
 targets = {t['id']: [os.path.relpath(f) for f in t['filename']]
            for t in introspect['targets']}
@@ -125,5 +92,3 @@ def emit_suite(name, suite, prefix):
 emit_prolog(benchsuites, 'bench')
 for name, suite in benchsuites.items():
     emit_suite(name, suite, 'bench')
-
-print('run-tests: $(patsubst %, run-test-%, $(.tests))')
diff --git a/scripts/tap-driver.pl b/scripts/tap-driver.pl
deleted file mode 100755
index b1d3880c50..0000000000
--- a/scripts/tap-driver.pl
+++ /dev/null
@@ -1,379 +0,0 @@
-#! /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", "$test_name - $_[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
deleted file mode 100755
index 10ccf57bb2..0000000000
--- a/scripts/tap-merge.pl
+++ /dev/null
@@ -1,111 +0,0 @@
-#! /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/fp/meson.build b/tests/fp/meson.build
index 07e2cdc8d2..32d57031fc 100644
--- a/tests/fp/meson.build
+++ b/tests/fp/meson.build
@@ -622,7 +622,7 @@ test('fp-test-mulAdd', fptest,
      # no fptest_rounding_args
      args: fptest_args +
            ['f16_mulAdd', 'f32_mulAdd', 'f64_mulAdd', 'f128_mulAdd'],
-     suite: ['softfloat-slow', 'softfloat-ops-slow'], timeout: 90)
+     suite: ['softfloat', 'softfloat-ops', 'slow'], timeout: 90)
 
 fpbench = executable(
   'fp-bench',
-- 
2.31.1




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

* [PATCH 2/4] build: make check-block a meson test
  2021-10-15 10:07 [RFC PATCH 0/4] Replace custom test harness with "meson test" Paolo Bonzini
  2021-10-15 10:07 ` [PATCH 1/4] build: use "meson test" as the test harness Paolo Bonzini
@ 2021-10-15 10:07 ` Paolo Bonzini
  2021-10-15 10:07 ` [PATCH 3/4] check-block: replace -makecheck with TAP output Paolo Bonzini
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2021-10-15 10:07 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, thuth, vsementsov, qemu-block, hreitz, marcandre.lureau

"meson test" supports can be asked to run tests verbosely.  This
makes it usable also for qemu-iotests's own harness.

This lets "make check-block" reuse mtest2make.py's infrastructure to
find and build test dependencies.  In the future, it will also enable
producing TAP output, for consistency with all other "make check"
tests.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 meson.build                    |  4 ++--
 scripts/mtest2make.py          |  8 +++++++-
 tests/Makefile.include         | 15 ++-------------
 tests/check-block.sh           | 26 ++++++++++++--------------
 tests/meson.build              |  1 +
 tests/qemu-iotests/meson.build | 13 +++++++++++++
 6 files changed, 37 insertions(+), 30 deletions(-)
 create mode 100644 tests/qemu-iotests/meson.build

diff --git a/meson.build b/meson.build
index c7af557afb..540d367cfc 100644
--- a/meson.build
+++ b/meson.build
@@ -3,8 +3,8 @@ project('qemu', ['c'], meson_version: '>=0.58.2',
                           'b_staticpic=false', 'stdsplit=false'],
         version: files('VERSION'))
 
-add_test_setup('quick', exclude_suites: 'slow', is_default: true)
-add_test_setup('slow', env: ['G_TEST_SLOW=1'])
+add_test_setup('quick', exclude_suites: ['block', 'slow'], is_default: true)
+add_test_setup('slow', exclude_suites: ['block'], env: ['G_TEST_SLOW=1'])
 
 not_found = dependency('', required: false)
 keyval = import('keyval')
diff --git a/scripts/mtest2make.py b/scripts/mtest2make.py
index b33c1d48df..7e03532bbb 100644
--- a/scripts/mtest2make.py
+++ b/scripts/mtest2make.py
@@ -64,13 +64,15 @@ def emit_prolog(suites, prefix):
     print(f'{prefix}-report.junit.xml $(all-{prefix}-xml): {prefix}-report%.junit.xml: run-ninja')
     print(f'\t$(MAKE) {prefix}$* MTESTARGS="$(MTESTARGS) --logbase {prefix}-report$*" && ln -f meson-logs/$@ .')
 
-def emit_suite(name, suite, prefix):
+def emit_suite_deps(name, suite, prefix):
     deps = ' '.join(suite.deps)
     print(f'.{prefix}-{name}.deps = {deps}')
     print(f'ifneq ($(filter {prefix}-build, $(MAKECMDGOALS)),)')
     print(f'.{prefix}.build-suites += {name}')
     print(f'endif')
 
+def emit_suite(name, suite, prefix):
+    emit_suite_deps(name, suite, prefix)
     targets = f'{prefix}-{name} {prefix}-report-{name}.junit.xml {prefix} {prefix}-report.junit.xml'
     print(f'ifneq ($(filter {targets}, $(MAKECMDGOALS)),)')
     print(f'.{prefix}.mtest-suites += {name}')
@@ -82,6 +84,10 @@ def emit_suite(name, suite, prefix):
 testsuites = defaultdict(Suite)
 for test in introspect['tests']:
     process_tests(test, targets, testsuites)
+# HACK: check-block is a separate target so that it runs with --verbose;
+# only write the dependencies
+emit_suite_deps('block', testsuites['block'], 'check')
+del testsuites['block']
 emit_prolog(testsuites, 'check')
 for name, suite in testsuites.items():
     emit_suite(name, suite, 'check')
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 8434a33fe6..00a1696bde 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -149,19 +149,8 @@ check:
 
 ifeq ($(CONFIG_TOOLS)$(CONFIG_POSIX),yy)
 check: check-block
-export PYTHON
-
-ifneq ($(filter check check-block check-build, $(MAKECMDGOALS)),)
-ninja-cmd-goals += \
-	qemu-img$(EXESUF) \
-	qemu-io$(EXESUF) \
-	qemu-nbd$(EXESUF) \
-	storage-daemon/qemu-storage-daemon$(EXESUF) \
-	$(filter qemu-system-%, $(ninja-targets))
-endif
-
-check-block: $(SRC_PATH)/tests/check-block.sh run-ninja
-	@$<
+check-block: run-ninja
+	$(MESON) test --no-rebuild --verbose --num-processes 1 --suite block
 endif
 
 check-build: run-ninja
diff --git a/tests/check-block.sh b/tests/check-block.sh
index f86cb863de..8895163625 100755
--- a/tests/check-block.sh
+++ b/tests/check-block.sh
@@ -16,9 +16,13 @@ if [ "$#" -ne 0 ]; then
     format_list="$@"
 fi
 
+skip() {
+    echo "$*"
+    exit 77
+}
+
 if grep -q "CONFIG_GPROF=y" config-host.mak 2>/dev/null ; then
-    echo "GPROF is enabled ==> Not running the qemu-iotests."
-    exit 0
+    skip "GPROF is enabled ==> Not running the qemu-iotests."
 fi
 
 # Disable tests with any sanitizer except for specific ones
@@ -36,36 +40,30 @@ for j in ${ALLOWED_SANITIZE_FLAGS}; do
 done
 if echo ${SANITIZE_FLAGS} | grep -q "\-fsanitize" 2>/dev/null; then
     # Have a sanitize flag that is not allowed, stop
-    echo "Sanitizers are enabled ==> Not running the qemu-iotests."
-    exit 0
+    skip "Sanitizers are enabled ==> Not running the qemu-iotests."
 fi
 
 if [ -z "$(find . -name 'qemu-system-*' -print)" ]; then
-    echo "No qemu-system binary available ==> Not running the qemu-iotests."
-    exit 0
+    skip "No qemu-system binary available ==> Not running the qemu-iotests."
 fi
 
 if ! command -v bash >/dev/null 2>&1 ; then
-    echo "bash not available ==> Not running the qemu-iotests."
-    exit 0
+    skip "bash not available ==> Not running the qemu-iotests."
 fi
 
 if LANG=C bash --version | grep -q 'GNU bash, version [123]' ; then
-    echo "bash version too old ==> Not running the qemu-iotests."
-    exit 0
+    skip "bash version too old ==> Not running the qemu-iotests."
 fi
 
 if ! (sed --version | grep 'GNU sed') > /dev/null 2>&1 ; then
     if ! command -v gsed >/dev/null 2>&1; then
-        echo "GNU sed not available ==> Not running the qemu-iotests."
-        exit 0
+        skip "GNU sed not available ==> Not running the qemu-iotests."
     fi
 else
     # Double-check that we're not using BusyBox' sed which says
     # that "This is not GNU sed version 4.0" ...
     if sed --version | grep -q 'not GNU sed' ; then
-        echo "BusyBox sed not supported ==> Not running the qemu-iotests."
-        exit 0
+        skip "BusyBox sed not supported ==> Not running the qemu-iotests."
     fi
 fi
 
diff --git a/tests/meson.build b/tests/meson.build
index 3f3882748a..d5e168d714 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -1,6 +1,7 @@
 py3 = import('python').find_installation()
 
 subdir('bench')
+subdir('qemu-iotests')
 
 test_qapi_outputs = [
   'qapi-builtin-types.c',
diff --git a/tests/qemu-iotests/meson.build b/tests/qemu-iotests/meson.build
new file mode 100644
index 0000000000..c59c17a9a9
--- /dev/null
+++ b/tests/qemu-iotests/meson.build
@@ -0,0 +1,13 @@
+if have_tools
+  qemu_iotests_binaries = [qemu_img, qemu_io, qemu_nbd, qsd]
+  foreach k, v : emulators
+    if k.startswith('qemu-system-')
+      qemu_iotests_binaries += v
+    endif
+  endforeach
+  test('qemu-iotests', sh, args: [files('../check-block.sh')],
+       depends: qemu_iotests_binaries,
+       suite: 'block',
+       timeout: 0,
+       is_parallel: false)
+endif
-- 
2.31.1




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

* [PATCH 3/4] check-block: replace -makecheck with TAP output
  2021-10-15 10:07 [RFC PATCH 0/4] Replace custom test harness with "meson test" Paolo Bonzini
  2021-10-15 10:07 ` [PATCH 1/4] build: use "meson test" as the test harness Paolo Bonzini
  2021-10-15 10:07 ` [PATCH 2/4] build: make check-block a meson test Paolo Bonzini
@ 2021-10-15 10:07 ` Paolo Bonzini
  2021-10-15 10:07 ` [PATCH 4/4] configure: remove dead EXESUF variable Paolo Bonzini
  2021-10-18  9:51 ` [RFC PATCH 0/4] Replace custom test harness with "meson test" Thomas Huth
  4 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2021-10-15 10:07 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, thuth, vsementsov, qemu-block, hreitz, marcandre.lureau

Let "meson test" take care of showing the results of the individual tests,
consistently with other output from "make check V=1".  Use TAP comments
so that the environment is included in the logs.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/check-block.sh             |  6 ++--
 tests/qemu-iotests/check         |  6 ++--
 tests/qemu-iotests/meson.build   |  1 +
 tests/qemu-iotests/testenv.py    | 30 ++++++++++----------
 tests/qemu-iotests/testrunner.py | 48 ++++++++++++++++----------------
 5 files changed, 46 insertions(+), 45 deletions(-)

diff --git a/tests/check-block.sh b/tests/check-block.sh
index 8895163625..159a040037 100755
--- a/tests/check-block.sh
+++ b/tests/check-block.sh
@@ -17,8 +17,8 @@ if [ "$#" -ne 0 ]; then
 fi
 
 skip() {
-    echo "$*"
-    exit 77
+    echo "1..0 #SKIP $*"
+    exit 0
 }
 
 if grep -q "CONFIG_GPROF=y" config-host.mak 2>/dev/null ; then
@@ -75,7 +75,7 @@ export PYTHONUTF8=1
 
 ret=0
 for fmt in $format_list ; do
-    ${PYTHON} ./check -makecheck -$fmt $group || ret=1
+    ${PYTHON} ./check -tap -$fmt $group || ret=1
 done
 
 exit $ret
diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index da1bfb839e..a8a26fcaaf 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -32,8 +32,8 @@ def make_argparser() -> argparse.ArgumentParser:
 
     p.add_argument('-n', '--dry-run', action='store_true',
                    help='show me, do not run tests')
-    p.add_argument('-makecheck', action='store_true',
-                   help='pretty print output for make check')
+    p.add_argument('-tap', action='store_true',
+                   help='produce TAP output')
 
     p.add_argument('-d', dest='debug', action='store_true', help='debug')
     p.add_argument('-p', dest='print', action='store_true',
@@ -161,7 +161,7 @@ if __name__ == '__main__':
     if args.dry_run:
         print('\n'.join(tests))
     else:
-        with TestRunner(env, makecheck=args.makecheck,
+        with TestRunner(env, tap=args.tap,
                         color=args.color) as tr:
             paths = [os.path.join(env.source_iotests, t) for t in tests]
             ok = tr.run_tests(paths)
diff --git a/tests/qemu-iotests/meson.build b/tests/qemu-iotests/meson.build
index c59c17a9a9..d93d378b11 100644
--- a/tests/qemu-iotests/meson.build
+++ b/tests/qemu-iotests/meson.build
@@ -7,6 +7,7 @@ if have_tools
   endforeach
   test('qemu-iotests', sh, args: [files('../check-block.sh')],
        depends: qemu_iotests_binaries,
+       protocol: 'tap',
        suite: 'block',
        timeout: 0,
        is_parallel: false)
diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
index c33454fa68..6f8ae4197d 100644
--- a/tests/qemu-iotests/testenv.py
+++ b/tests/qemu-iotests/testenv.py
@@ -287,21 +287,21 @@ def __enter__(self) -> 'TestEnv':
     def __exit__(self, exc_type: Any, exc_value: Any, traceback: Any) -> None:
         self.close()
 
-    def print_env(self) -> None:
+    def print_env(self, prefix='') -> None:
         template = """\
-QEMU          -- "{QEMU_PROG}" {QEMU_OPTIONS}
-QEMU_IMG      -- "{QEMU_IMG_PROG}" {QEMU_IMG_OPTIONS}
-QEMU_IO       -- "{QEMU_IO_PROG}" {QEMU_IO_OPTIONS}
-QEMU_NBD      -- "{QEMU_NBD_PROG}" {QEMU_NBD_OPTIONS}
-IMGFMT        -- {IMGFMT}{imgopts}
-IMGPROTO      -- {IMGPROTO}
-PLATFORM      -- {platform}
-TEST_DIR      -- {TEST_DIR}
-SOCK_DIR      -- {SOCK_DIR}
-GDB_OPTIONS   -- {GDB_OPTIONS}
-VALGRIND_QEMU -- {VALGRIND_QEMU}
-PRINT_QEMU_OUTPUT -- {PRINT_QEMU}
-"""
+{prefix}QEMU          -- "{QEMU_PROG}" {QEMU_OPTIONS}
+{prefix}QEMU_IMG      -- "{QEMU_IMG_PROG}" {QEMU_IMG_OPTIONS}
+{prefix}QEMU_IO       -- "{QEMU_IO_PROG}" {QEMU_IO_OPTIONS}
+{prefix}QEMU_NBD      -- "{QEMU_NBD_PROG}" {QEMU_NBD_OPTIONS}
+{prefix}IMGFMT        -- {IMGFMT}{imgopts}
+{prefix}IMGPROTO      -- {IMGPROTO}
+{prefix}PLATFORM      -- {platform}
+{prefix}TEST_DIR      -- {TEST_DIR}
+{prefix}SOCK_DIR      -- {SOCK_DIR}
+{prefix}GDB_OPTIONS   -- {GDB_OPTIONS}
+{prefix}VALGRIND_QEMU -- {VALGRIND_QEMU}
+{prefix}PRINT_QEMU_OUTPUT -- {PRINT_QEMU}
+{prefix}"""
 
         args = collections.defaultdict(str, self.get_env())
 
@@ -310,5 +310,5 @@ def print_env(self) -> None:
 
         u = os.uname()
         args['platform'] = f'{u.sysname}/{u.machine} {u.nodename} {u.release}'
-
+        args['prefix'] = prefix
         print(template.format_map(args))
diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py
index 0e29c2fddd..3ef14af1fa 100644
--- a/tests/qemu-iotests/testrunner.py
+++ b/tests/qemu-iotests/testrunner.py
@@ -126,10 +126,10 @@ def __init__(self, status: str, description: str = '',
 
 
 class TestRunner(ContextManager['TestRunner']):
-    def __init__(self, env: TestEnv, makecheck: bool = False,
+    def __init__(self, env: TestEnv, tap: bool = False,
                  color: str = 'auto') -> None:
         self.env = env
-        self.makecheck = makecheck
+        self.tap = tap
         self.last_elapsed = LastElapsedTime('.last-elapsed-cache', env)
 
         assert color in ('auto', 'on', 'off')
@@ -161,13 +161,13 @@ def test_print_one_line(self, test: str, starttime: str,
         if test_field_width is None:
             test_field_width = 8
 
-        if self.makecheck and status != '...':
-            if status and status != 'pass':
-                status = f' [{status}]'
-            else:
-                status = ''
-
-            print(f'  TEST   iotest-{self.env.imgfmt}: {test}{status}')
+        if self.tap:
+            if status == 'pass':
+                print(f'ok test {test}')
+            elif status == 'fail':
+                print(f'not ok test {test}')
+            elif status == 'not run':
+                print(f'ok test {test} # SKIP')
             return
 
         if lasttime:
@@ -290,7 +290,7 @@ def run_test(self, test: str,
         last_el = self.last_elapsed.get(test)
         start = datetime.datetime.now().strftime('%H:%M:%S')
 
-        if not self.makecheck:
+        if not self.tap:
             self.test_print_one_line(test=test, starttime=start,
                                      lasttime=last_el, end='\r',
                                      test_field_width=test_field_width)
@@ -315,7 +315,9 @@ def run_tests(self, tests: List[str]) -> bool:
         notrun = []
         casenotrun = []
 
-        if not self.makecheck:
+        if self.tap:
+            self.env.print_env('# ')
+        else:
             self.env.print_env()
 
         test_field_width = max(len(os.path.basename(t)) for t in tests) + 2
@@ -334,8 +336,6 @@ def run_tests(self, tests: List[str]) -> bool:
 
             if res.status == 'fail':
                 failed.append(name)
-                if self.makecheck:
-                    self.env.print_env()
                 if res.diff:
                     print('\n'.join(res.diff))
             elif res.status == 'not run':
@@ -345,16 +345,16 @@ def run_tests(self, tests: List[str]) -> bool:
             if res.interrupted:
                 break
 
-        if notrun:
-            print('Not run:', ' '.join(notrun))
+        if not self.tap:
+            if notrun:
+                print('Not run:', ' '.join(notrun))
 
-        if casenotrun:
-            print('Some cases not run in:', ' '.join(casenotrun))
+            if casenotrun:
+                print('Some cases not run in:', ' '.join(casenotrun))
 
-        if failed:
-            print('Failures:', ' '.join(failed))
-            print(f'Failed {len(failed)} of {n_run} iotests')
-            return False
-        else:
-            print(f'Passed all {n_run} iotests')
-            return True
+            if failed:
+                print('Failures:', ' '.join(failed))
+                print(f'Failed {len(failed)} of {n_run} iotests')
+            else:
+                print(f'Passed all {n_run} iotests')
+        return not failed
-- 
2.31.1




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

* [PATCH 4/4] configure: remove dead EXESUF variable
  2021-10-15 10:07 [RFC PATCH 0/4] Replace custom test harness with "meson test" Paolo Bonzini
                   ` (2 preceding siblings ...)
  2021-10-15 10:07 ` [PATCH 3/4] check-block: replace -makecheck with TAP output Paolo Bonzini
@ 2021-10-15 10:07 ` Paolo Bonzini
  2021-10-15 12:38   ` Philippe Mathieu-Daudé
  2021-10-18  9:51 ` [RFC PATCH 0/4] Replace custom test harness with "meson test" Thomas Huth
  4 siblings, 1 reply; 10+ messages in thread
From: Paolo Bonzini @ 2021-10-15 10:07 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, thuth, vsementsov, qemu-block, hreitz, marcandre.lureau

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 .gitlab-ci.d/crossbuild-template.yml | 2 +-
 configure                            | 3 ---
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/.gitlab-ci.d/crossbuild-template.yml b/.gitlab-ci.d/crossbuild-template.yml
index 10d22dcf6c..87cb8b7518 100644
--- a/.gitlab-ci.d/crossbuild-template.yml
+++ b/.gitlab-ci.d/crossbuild-template.yml
@@ -12,7 +12,7 @@
           mips64-softmmu ppc-softmmu riscv32-softmmu sh4-softmmu
           sparc-softmmu xtensa-softmmu $CROSS_SKIP_TARGETS"
     - make -j$(expr $(nproc) + 1) all check-build $MAKE_CHECK_ARGS
-    - if grep -q "EXESUF=.exe" config-host.mak;
+    - if test -f qemu-io.exe;
       then make installer;
       version="$(git describe --match v[0-9]*)";
       mv -v qemu-setup*.exe qemu-setup-${version}.exe;
diff --git a/configure b/configure
index 039467c04b..a4489bc23f 100755
--- a/configure
+++ b/configure
@@ -308,7 +308,6 @@ fortify_source="$default_feature"
 strip_opt="yes"
 mingw32="no"
 gcov="no"
-EXESUF=""
 modules="no"
 module_upgrades="no"
 prefix="/usr/local"
@@ -713,7 +712,6 @@ else
 fi
 
 if test "$mingw32" = "yes" ; then
-  EXESUF=".exe"
   # MinGW needs -mthreads for TLS and macro _MT.
   CONFIGURE_CFLAGS="-mthreads $CONFIGURE_CFLAGS"
   write_c_skeleton;
@@ -3814,7 +3812,6 @@ echo "GLIB_CFLAGS=$glib_cflags" >> $config_host_mak
 echo "GLIB_LIBS=$glib_libs" >> $config_host_mak
 echo "QEMU_LDFLAGS=$QEMU_LDFLAGS" >> $config_host_mak
 echo "LD_I386_EMULATION=$ld_i386_emulation" >> $config_host_mak
-echo "EXESUF=$EXESUF" >> $config_host_mak
 echo "LIBS_QGA=$libs_qga" >> $config_host_mak
 
 if test "$rng_none" = "yes"; then
-- 
2.31.1



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

* Re: [PATCH 4/4] configure: remove dead EXESUF variable
  2021-10-15 10:07 ` [PATCH 4/4] configure: remove dead EXESUF variable Paolo Bonzini
@ 2021-10-15 12:38   ` Philippe Mathieu-Daudé
  2021-10-15 13:36     ` Paolo Bonzini
  0 siblings, 1 reply; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-10-15 12:38 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel
  Cc: kwolf, thuth, vsementsov, qemu-block, hreitz, marcandre.lureau

On 10/15/21 12:07, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  .gitlab-ci.d/crossbuild-template.yml | 2 +-
>  configure                            | 3 ---
>  2 files changed, 1 insertion(+), 4 deletions(-)

Maybe squash in patch #2 or place as #3 mentioning
"the previous commit"?



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

* Re: [PATCH 4/4] configure: remove dead EXESUF variable
  2021-10-15 12:38   ` Philippe Mathieu-Daudé
@ 2021-10-15 13:36     ` Paolo Bonzini
  2021-10-15 14:23       ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 10+ messages in thread
From: Paolo Bonzini @ 2021-10-15 13:36 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: kwolf, thuth, vsementsov, qemu-block, hreitz, marcandre.lureau

On 15/10/21 14:38, Philippe Mathieu-Daudé wrote:
> On 10/15/21 12:07, Paolo Bonzini wrote:
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>   .gitlab-ci.d/crossbuild-template.yml | 2 +-
>>   configure                            | 3 ---
>>   2 files changed, 1 insertion(+), 4 deletions(-)
> 
> Maybe squash in patch #2 or place as #3 mentioning
> "the previous commit"?

Anywhere you place it it's wrong. :)  Squashing in #2 I dislike because 
of the functional change in .gilab-ci.d (in truth, the variable is 
*almost* dead!).  Having the change as #4 makes it survive one patch 
longer than it should, on the other hand having it as #3 separates 
similar changes to "check-block.sh".

What I will actually do in the final submission is not include the TAP 
patch and submit it separately.  Then for this one I can indeed add that 
it was used only in the implementation of check-block.

Paolo



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

* Re: [PATCH 4/4] configure: remove dead EXESUF variable
  2021-10-15 13:36     ` Paolo Bonzini
@ 2021-10-15 14:23       ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-10-15 14:23 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel
  Cc: kwolf, thuth, vsementsov, qemu-block, hreitz, marcandre.lureau

On 10/15/21 15:36, Paolo Bonzini wrote:
> On 15/10/21 14:38, Philippe Mathieu-Daudé wrote:
>> On 10/15/21 12:07, Paolo Bonzini wrote:
>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>> ---
>>>   .gitlab-ci.d/crossbuild-template.yml | 2 +-
>>>   configure                            | 3 ---
>>>   2 files changed, 1 insertion(+), 4 deletions(-)
>>
>> Maybe squash in patch #2 or place as #3 mentioning
>> "the previous commit"?
> 
> Anywhere you place it it's wrong. :)  Squashing in #2 I dislike because
> of the functional change in .gilab-ci.d (in truth, the variable is
> *almost* dead!).  Having the change as #4 makes it survive one patch
> longer than it should, on the other hand having it as #3 separates
> similar changes to "check-block.sh".

Indeed.

> What I will actually do in the final submission is not include the TAP
> patch and submit it separately.  Then for this one I can indeed add that
> it was used only in the implementation of check-block.

OK then,
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>



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

* Re: [RFC PATCH 0/4] Replace custom test harness with "meson test"
  2021-10-15 10:07 [RFC PATCH 0/4] Replace custom test harness with "meson test" Paolo Bonzini
                   ` (3 preceding siblings ...)
  2021-10-15 10:07 ` [PATCH 4/4] configure: remove dead EXESUF variable Paolo Bonzini
@ 2021-10-18  9:51 ` Thomas Huth
  2021-10-18 17:55   ` Paolo Bonzini
  4 siblings, 1 reply; 10+ messages in thread
From: Thomas Huth @ 2021-10-18  9:51 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel
  Cc: kwolf, marcandre.lureau, hreitz, vsementsov, qemu-block

On 15/10/2021 12.07, Paolo Bonzini wrote:
> Hi all,
> 
> Starting with Meson 0.57, "meson test" has all features of QEMU's
> makefile-based harness and more.

I just gave it a try, and basically I like this ... but I also encountered 
two issues:

> * CTRL+C will only interrupt the longest running test.  Pressing
>    CTRL+C repeatedly three times (which you would likely do anyway,
>    that's how things work) interrupts the whole run

I tried this, and while hitting CTRL-C multiple times brought me back to the 
shell prompt, the remaining tests kept getting started in the background 
instead of getting stopped ... something is still fishy here, I think.

> * Right now "make check-block" only does a single test run just like
>    "../tests/check-block.sh", but it would be possible to add the thorough
>    suite to "meson test --suite block" as well.

The output of the iotests is also not optimal yet... when running "make 
check SPEED=slow", the iotests are run multiple times with different target 
image types, but each run prints the same "▶ 1/1 test 001   OK" etc. to the 
console, so it's hard to say which target type  is currently exercised. 
Would it be possible to include the target image type here, e.g. something like:

▶ 1/1 test-qcow2 001                       OK

?

  Thomas



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

* Re: [RFC PATCH 0/4] Replace custom test harness with "meson test"
  2021-10-18  9:51 ` [RFC PATCH 0/4] Replace custom test harness with "meson test" Thomas Huth
@ 2021-10-18 17:55   ` Paolo Bonzini
  0 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2021-10-18 17:55 UTC (permalink / raw)
  To: Thomas Huth, qemu-devel
  Cc: kwolf, marcandre.lureau, hreitz, vsementsov, qemu-block

On 18/10/21 11:51, Thomas Huth wrote:
>> * CTRL+C will only interrupt the longest running test.  Pressing
>>    CTRL+C repeatedly three times (which you would likely do anyway,
>>    that's how things work) interrupts the whole run
> 
> I tried this, and while hitting CTRL-C multiple times brought me back to 
> the shell prompt, the remaining tests kept getting started in the 
> background instead of getting stopped ... something is still fishy here, 
> I think.

Ok, I checked that out.  Looks like CTRL+C magic and "make -j" are 
incompatible. :/  So this will have to wait a bit more, but in the 
meanwhile people can already use "meson test" if they want.

>> * Right now "make check-block" only does a single test run just like
>>    "../tests/check-block.sh", but it would be possible to add the 
>> thorough
>>    suite to "meson test --suite block" as well.
> 
> The output of the iotests is also not optimal yet... when running "make 
> check SPEED=slow", the iotests are run multiple times with different 
> target image types, but each run prints the same "▶ 1/1 test 001   OK" 
> etc. to the console, so it's hard to say which target type  is currently 
> exercised. Would it be possible to include the target image type here, 
> e.g. something like:

Yes, that's trivial:

diff --git a/tests/qemu-iotests/testrunner.py 
b/tests/qemu-iotests/testrunner.py
index 3ef14af1fa..45debc1928 100644
--- a/tests/qemu-iotests/testrunner.py
+++ b/tests/qemu-iotests/testrunner.py
@@ -163,11 +163,11 @@ def test_print_one_line(self, test: str, 
starttime: str,

          if self.tap:
              if status == 'pass':
-                print(f'ok test {test}')
+                print(f'ok {self.env.imgfmt} {test}')
              elif status == 'fail':
-                print(f'not ok test {test}')
+                print(f'not ok {self.env.imgfmt} {test}')
              elif status == 'not run':
-                print(f'ok test {test} # SKIP')
+                print(f'ok {self.env.imgfmt} {test} # SKIP')
              return

          if lasttime:

In fact, that's exactly what was printed in the non-TAP case.  Thanks 
for the feedback, even though it was bad! :)

Paolo



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

end of thread, other threads:[~2021-10-18 17:57 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-15 10:07 [RFC PATCH 0/4] Replace custom test harness with "meson test" Paolo Bonzini
2021-10-15 10:07 ` [PATCH 1/4] build: use "meson test" as the test harness Paolo Bonzini
2021-10-15 10:07 ` [PATCH 2/4] build: make check-block a meson test Paolo Bonzini
2021-10-15 10:07 ` [PATCH 3/4] check-block: replace -makecheck with TAP output Paolo Bonzini
2021-10-15 10:07 ` [PATCH 4/4] configure: remove dead EXESUF variable Paolo Bonzini
2021-10-15 12:38   ` Philippe Mathieu-Daudé
2021-10-15 13:36     ` Paolo Bonzini
2021-10-15 14:23       ` Philippe Mathieu-Daudé
2021-10-18  9:51 ` [RFC PATCH 0/4] Replace custom test harness with "meson test" Thomas Huth
2021-10-18 17:55   ` Paolo Bonzini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.