* [PATCH] Revert "check-block: replace -makecheck with TAP output"
@ 2022-02-08 12:44 Paolo Bonzini
2022-02-08 14:40 ` Peter Maydell
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Paolo Bonzini @ 2022-02-08 12:44 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, thuth
This reverts commit d316859f4e28c74ab8b618895d2a5e0a865d3cf1. The
TAP output is inferior in that it does not include the diff for
failed tests.
Reported-by: Peter Maydell <peter.maydell@linaro.org>
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 | 49 +++++++++++++++-----------------
5 files changed, 44 insertions(+), 48 deletions(-)
diff --git a/tests/check-block.sh b/tests/check-block.sh
index 720a46bc36..88e02453d2 100755
--- a/tests/check-block.sh
+++ b/tests/check-block.sh
@@ -14,8 +14,8 @@ else
fi
skip() {
- echo "1..0 #SKIP $*"
- exit 0
+ echo "$*"
+ exit 77
}
if grep -q "CONFIG_GPROF=y" config-host.mak 2>/dev/null ; then
@@ -79,7 +79,7 @@ JOBS=$(echo "$MAKEFLAGS" | sed -n 's/\(^\|.* \)-j\([0-9]\+\)\( .*\|$\)/-j \2/p')
ret=0
for fmt in $format_list ; do
- ${PYTHON} ./check $JOBS -tap -$fmt $group || ret=1
+ ${PYTHON} ./check $JOBS -makecheck -$fmt $group || ret=1
done
exit $ret
diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 75de1b4691..0c27721a41 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -32,6 +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('-j', dest='jobs', type=int, default=1,
help='run tests in multiple parallel jobs')
@@ -51,8 +53,6 @@ def make_argparser() -> argparse.ArgumentParser:
p.add_argument('--color', choices=['on', 'off', 'auto'],
default='auto', help="use terminal colors. The default "
"'auto' value means use colors if terminal stdout detected")
- p.add_argument('-tap', action='store_true',
- help='produce TAP output')
g_env = p.add_argument_group('test environment options')
mg = g_env.add_mutually_exclusive_group()
@@ -164,7 +164,7 @@ if __name__ == '__main__':
if args.dry_run:
print('\n'.join(tests))
else:
- with TestRunner(env, tap=args.tap,
+ with TestRunner(env, makecheck=args.makecheck,
color=args.color) as tr:
paths = [os.path.join(env.source_iotests, t) for t in tests]
ok = tr.run_tests(paths, args.jobs)
diff --git a/tests/qemu-iotests/meson.build b/tests/qemu-iotests/meson.build
index 5be3c74127..3a9425d15c 100644
--- a/tests/qemu-iotests/meson.build
+++ b/tests/qemu-iotests/meson.build
@@ -22,7 +22,6 @@ if have_tools and targetos != 'windows'
endif
test('qemu-iotests ' + format, sh, args: [files('../check-block.sh'), format],
depends: qemu_iotests_binaries, env: qemu_iotests_env,
- protocol: 'tap',
suite: suites,
timeout: 0,
is_parallel: false)
diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
index 0f32897fe8..c33454fa68 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, prefix: str = '') -> None:
+ def print_env(self) -> None:
template = """\
-{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}"""
+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}
+"""
args = collections.defaultdict(str, self.get_env())
@@ -310,5 +310,5 @@ def print_env(self, prefix: str = '') -> 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 0eace147b8..15788f919e 100644
--- a/tests/qemu-iotests/testrunner.py
+++ b/tests/qemu-iotests/testrunner.py
@@ -152,10 +152,10 @@ def run_tests_pool(self, tests: List[str],
return results
- def __init__(self, env: TestEnv, tap: bool = False,
+ def __init__(self, env: TestEnv, makecheck: bool = False,
color: str = 'auto') -> None:
self.env = env
- self.tap = tap
+ self.makecheck = makecheck
self.last_elapsed = LastElapsedTime('.last-elapsed-cache', env)
assert color in ('auto', 'on', 'off')
@@ -185,16 +185,13 @@ def test_print_one_line(self, test: str,
""" Print short test info before/after test run """
test = os.path.basename(test)
- 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 = ''
- if self.tap:
- if status == 'pass':
- print(f'ok {self.env.imgfmt} {test}')
- elif status == 'fail':
- print(f'not ok {self.env.imgfmt} {test}')
- elif status == 'not run':
- print(f'ok {self.env.imgfmt} {test} # SKIP')
+ print(f' TEST iotest-{self.env.imgfmt}: {test}{status}')
return
if lasttime:
@@ -346,7 +343,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.tap:
+ if not self.makecheck:
self.test_print_one_line(test=test,
test_field_width=test_field_width,
status = 'started' if mp else '...',
@@ -375,9 +372,7 @@ def run_tests(self, tests: List[str], jobs: int = 1) -> bool:
notrun = []
casenotrun = []
- if self.tap:
- self.env.print_env('# ')
- else:
+ if not self.makecheck:
self.env.print_env()
test_field_width = max(len(os.path.basename(t)) for t in tests) + 2
@@ -403,6 +398,8 @@ def run_tests(self, tests: List[str], jobs: int = 1) -> 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':
@@ -415,16 +412,16 @@ def run_tests(self, tests: List[str], jobs: int = 1) -> bool:
if res.interrupted:
break
- if not self.tap:
- if notrun:
- print('Not run:', ' '.join(notrun))
+ 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')
- else:
- print(f'Passed all {n_run} iotests')
- return not failed
+ 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
--
2.31.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] Revert "check-block: replace -makecheck with TAP output"
2022-02-08 12:44 [PATCH] Revert "check-block: replace -makecheck with TAP output" Paolo Bonzini
@ 2022-02-08 14:40 ` Peter Maydell
2022-02-08 14:51 ` Thomas Huth
2022-03-04 16:57 ` Daniel P. Berrangé
2 siblings, 0 replies; 7+ messages in thread
From: Peter Maydell @ 2022-02-08 14:40 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: thuth, qemu-devel
On Tue, 8 Feb 2022 at 12:44, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> This reverts commit d316859f4e28c74ab8b618895d2a5e0a865d3cf1. The
> TAP output is inferior in that it does not include the diff for
> failed tests.
>
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Thanks; I tested and confirmed that failing iotests now
print the diff output again.
Tested-by: Peter Maydell <peter.maydell@linaro.org>
-- PMM
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Revert "check-block: replace -makecheck with TAP output"
2022-02-08 12:44 [PATCH] Revert "check-block: replace -makecheck with TAP output" Paolo Bonzini
2022-02-08 14:40 ` Peter Maydell
@ 2022-02-08 14:51 ` Thomas Huth
2022-02-08 16:21 ` Paolo Bonzini
2022-03-04 16:57 ` Daniel P. Berrangé
2 siblings, 1 reply; 7+ messages in thread
From: Thomas Huth @ 2022-02-08 14:51 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: Peter Maydell
On 08/02/2022 13.44, Paolo Bonzini wrote:
> This reverts commit d316859f4e28c74ab8b618895d2a5e0a865d3cf1. The
> TAP output is inferior in that it does not include the diff for
> failed tests.
What about dumping the output of meson-logs/testlog.txt instead? The diff
also shows up there!
Thomas
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Revert "check-block: replace -makecheck with TAP output"
2022-02-08 14:51 ` Thomas Huth
@ 2022-02-08 16:21 ` Paolo Bonzini
2022-02-09 7:32 ` Thomas Huth
0 siblings, 1 reply; 7+ messages in thread
From: Paolo Bonzini @ 2022-02-08 16:21 UTC (permalink / raw)
To: Thomas Huth, qemu-devel; +Cc: Peter Maydell
On 2/8/22 15:51, Thomas Huth wrote:
> On 08/02/2022 13.44, Paolo Bonzini wrote:
>> This reverts commit d316859f4e28c74ab8b618895d2a5e0a865d3cf1. The
>> TAP output is inferior in that it does not include the diff for
>> failed tests.
>
> What about dumping the output of meson-logs/testlog.txt instead? The
> diff also shows up there!
It does, but sort of by accident because it's not valid TAP (and in fact
it continues with a TAP parsing error).
Paolo
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Revert "check-block: replace -makecheck with TAP output"
2022-02-08 16:21 ` Paolo Bonzini
@ 2022-02-09 7:32 ` Thomas Huth
0 siblings, 0 replies; 7+ messages in thread
From: Thomas Huth @ 2022-02-09 7:32 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: Peter Maydell
On 08/02/2022 17.21, Paolo Bonzini wrote:
> On 2/8/22 15:51, Thomas Huth wrote:
>> On 08/02/2022 13.44, Paolo Bonzini wrote:
>>> This reverts commit d316859f4e28c74ab8b618895d2a5e0a865d3cf1. The
>>> TAP output is inferior in that it does not include the diff for
>>> failed tests.
>>
>> What about dumping the output of meson-logs/testlog.txt instead? The diff
>> also shows up there!
>
> It does, but sort of by accident because it's not valid TAP (and in fact it
> continues with a TAP parsing error).
Ok, then let's try to go with my series (I'm working on a v2 right now) and
the additional patch that prints out the diff to stderr instead of stdout,
please ... I hope that should be good enough, and certainly looks way nicer
than reverting to the old way of running the iotests.
Thomas
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Revert "check-block: replace -makecheck with TAP output"
2022-02-08 12:44 [PATCH] Revert "check-block: replace -makecheck with TAP output" Paolo Bonzini
2022-02-08 14:40 ` Peter Maydell
2022-02-08 14:51 ` Thomas Huth
@ 2022-03-04 16:57 ` Daniel P. Berrangé
2022-03-07 8:13 ` Thomas Huth
2 siblings, 1 reply; 7+ messages in thread
From: Daniel P. Berrangé @ 2022-03-04 16:57 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Peter Maydell, thuth, qemu-devel
On Tue, Feb 08, 2022 at 07:44:32AM -0500, Paolo Bonzini wrote:
> This reverts commit d316859f4e28c74ab8b618895d2a5e0a865d3cf1. The
> TAP output is inferior in that it does not include the diff for
> failed tests.
>
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> 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 | 49 +++++++++++++++-----------------
> 5 files changed, 44 insertions(+), 48 deletions(-)
Peter mentioned that he's seeing the CentOS 8 CI job fail
continuously: eg
https://gitlab.com/qemu-project/qemu/-/jobs/2163919781
[quote]
1/1 qemu:block / qemu-iotests qcow2 ERROR 163.04s 120 subtests passed
Summary of Failures:
1/1 qemu:block / qemu-iotests qcow2 ERROR 163.04s 120 subtests passed
Ok: 0
Expected Fail: 0
Fail: 1
Unexpected Pass: 0
Skipped: 0
Timeout: 0
Full log written to /home/gitlab-runner/builds/Jpwtyaz7/0/qemu-project/qemu/build/meson-logs/iotestslog.txt
make: *** [/home/gitlab-runner/builds/Jpwtyaz7/0/qemu-project/qemu/tests/Makefile.include:156: check-block] Error 1
[/quote]
None of the individual tests are showing ERROR though, only the final
status is error.
We also don't have access to iotestslog.txt which is very annoying.
When i run it locally I got the iotests.log and found that it
contained an error message saying the TAP outout was malformed
and couldn't be parsed.
When I ran the iotests manually I confirmed this, it prints:
ok qcow2 037
ok qcow2 038
ok qcow2 039
ok qcow2 040
ok qcow2 041
[case not run] test_after_a_quorum_snapshot (__main__.TestRepairQuorum): formats ['quorum'] are not whitelisted
[case not run] test_cancel (__main__.TestRepairQuorum): formats ['quorum'] are not whitelisted
[case not run] test_cancel_after_ready (__main__.TestRepairQuorum): formats ['quorum'] are not whitelisted
[case not run] test_complete (__main__.TestRepairQuorum): formats ['quorum'] are not whitelisted
[case not run] test_device_not_found (__main__.TestRepairQuorum): formats ['quorum'] are not whitelisted
[case not run] test_image_not_found (__main__.TestRepairQuorum): formats ['quorum'] are not whitelisted
[case not run] test_medium_not_found (__main__.TestRepairQuorum): formats ['quorum'] are not whitelisted
[case not run] test_no_node_name (__main__.TestRepairQuorum): formats ['quorum'] are not whitelisted
[case not run] test_nonexistent_replaces (__main__.TestRepairQuorum): formats ['quorum'] are not whitelisted
[case not run] test_pause (__main__.TestRepairQuorum): formats ['quorum'] are not whitelisted
[case not run] test_with_other_parent (__main__.TestRepairQuorum): formats ['quorum'] are not whitelisted
[case not run] test_with_other_parents_after_mirror_start (__main__.TestRepairQuorum): formats ['quorum'] are not whitelisted
[case not run] test_wrong_sync_mode (__main__.TestRepairQuorum): formats ['quorum'] are not whitelisted
ok qcow2 042
ok qcow2 043
ok qcow2 046
All those '[case not run]' lines are invalid tap output. The
need to start with a leading '#' to be treated as a diagnostic
message. I briefly considered sending a patch, until I remember
this change to stop using TAP.
Anyway I'd say the commit message here needs updating. It is
not merely improving diff output. It is adressing the fact that
the I/O tests runner is not producing valid TAP output, and so
causing bogus failures.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Revert "check-block: replace -makecheck with TAP output"
2022-03-04 16:57 ` Daniel P. Berrangé
@ 2022-03-07 8:13 ` Thomas Huth
0 siblings, 0 replies; 7+ messages in thread
From: Thomas Huth @ 2022-03-07 8:13 UTC (permalink / raw)
To: Daniel P. Berrangé, Paolo Bonzini
Cc: Peter Maydell, Hanna Reitz, qemu-devel, Qemu-block
On 04/03/2022 17.57, Daniel P. Berrangé wrote:
> On Tue, Feb 08, 2022 at 07:44:32AM -0500, Paolo Bonzini wrote:
>> This reverts commit d316859f4e28c74ab8b618895d2a5e0a865d3cf1. The
>> TAP output is inferior in that it does not include the diff for
>> failed tests.
>>
>> Reported-by: Peter Maydell <peter.maydell@linaro.org>
>> 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 | 49 +++++++++++++++-----------------
>> 5 files changed, 44 insertions(+), 48 deletions(-)
>
> Peter mentioned that he's seeing the CentOS 8 CI job fail
> continuously: eg
>
> https://gitlab.com/qemu-project/qemu/-/jobs/2163919781
>
> [quote]
> 1/1 qemu:block / qemu-iotests qcow2 ERROR 163.04s 120 subtests passed
> Summary of Failures:
> 1/1 qemu:block / qemu-iotests qcow2 ERROR 163.04s 120 subtests passed
> Ok: 0
> Expected Fail: 0
> Fail: 1
> Unexpected Pass: 0
> Skipped: 0
> Timeout: 0
> Full log written to /home/gitlab-runner/builds/Jpwtyaz7/0/qemu-project/qemu/build/meson-logs/iotestslog.txt
> make: *** [/home/gitlab-runner/builds/Jpwtyaz7/0/qemu-project/qemu/tests/Makefile.include:156: check-block] Error 1
>
> [/quote]
>
> None of the individual tests are showing ERROR though, only the final
> status is error.
>
> We also don't have access to iotestslog.txt which is very annoying.
>
> When i run it locally I got the iotests.log and found that it
> contained an error message saying the TAP outout was malformed
> and couldn't be parsed.
>
> When I ran the iotests manually I confirmed this, it prints:
>
> ok qcow2 037
> ok qcow2 038
> ok qcow2 039
> ok qcow2 040
> ok qcow2 041
> [case not run] test_after_a_quorum_snapshot (__main__.TestRepairQuorum): formats ['quorum'] are not whitelisted
> [case not run] test_cancel (__main__.TestRepairQuorum): formats ['quorum'] are not whitelisted
> [case not run] test_cancel_after_ready (__main__.TestRepairQuorum): formats ['quorum'] are not whitelisted
> [case not run] test_complete (__main__.TestRepairQuorum): formats ['quorum'] are not whitelisted
> [case not run] test_device_not_found (__main__.TestRepairQuorum): formats ['quorum'] are not whitelisted
> [case not run] test_image_not_found (__main__.TestRepairQuorum): formats ['quorum'] are not whitelisted
> [case not run] test_medium_not_found (__main__.TestRepairQuorum): formats ['quorum'] are not whitelisted
> [case not run] test_no_node_name (__main__.TestRepairQuorum): formats ['quorum'] are not whitelisted
> [case not run] test_nonexistent_replaces (__main__.TestRepairQuorum): formats ['quorum'] are not whitelisted
> [case not run] test_pause (__main__.TestRepairQuorum): formats ['quorum'] are not whitelisted
> [case not run] test_with_other_parent (__main__.TestRepairQuorum): formats ['quorum'] are not whitelisted
> [case not run] test_with_other_parents_after_mirror_start (__main__.TestRepairQuorum): formats ['quorum'] are not whitelisted
> [case not run] test_wrong_sync_mode (__main__.TestRepairQuorum): formats ['quorum'] are not whitelisted
>
> ok qcow2 042
> ok qcow2 043
> ok qcow2 046
>
>
> All those '[case not run]' lines are invalid tap output. The
> need to start with a leading '#' to be treated as a diagnostic
> message. I briefly considered sending a patch, until I remember
> this change to stop using TAP.
A patch for fixing the TAP problem is already on the list and has been
queued by Hanna:
https://lists.gnu.org/archive/html/qemu-devel/2022-02/msg05074.html
So that should hopefully fix the problem with the CentOS 8 CI job once it
gets merged.
Thomas
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-03-07 8:15 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-02-08 12:44 [PATCH] Revert "check-block: replace -makecheck with TAP output" Paolo Bonzini
2022-02-08 14:40 ` Peter Maydell
2022-02-08 14:51 ` Thomas Huth
2022-02-08 16:21 ` Paolo Bonzini
2022-02-09 7:32 ` Thomas Huth
2022-03-04 16:57 ` Daniel P. Berrangé
2022-03-07 8:13 ` Thomas Huth
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).