4:25 a.m.
meson's "test()" function provides a "should_fail: bool" argument
that
checks for a command to exit with a non-zero exit code instead of the usual
zero exit code to signal success. If the program under test does so, it is
recorded as "EXPECTEDFAIL" instead of "OK". While there is an argument
to be
made that the program under test failed as expected, the test in itself is
successful and should be recorded as such.
Before:
$ meson test
...
151/300 libvirt:bin / libvirtd fail with missing config
EXPECTEDFAIL 0.03s exit status 1
...
Ok: 299
Expected Fail: 1
Fail: 0
Unexpected Pass: 0
Skipped: 0
Timeout: 0
After:
$ meson test
...
151/300 libvirt:bin / libvirtd fail with missing config
OK 0.03s
...
Ok: 300
Expected Fail: 0
Fail: 0
Unexpected Pass: 0
Skipped: 0
Timeout: 0
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
tests/expected-fail | 13 +++++++++++++
tests/meson.build | 5 ++---
2 files changed, 15 insertions(+), 3 deletions(-)
create mode 100755 tests/expected-fail
diff --git a/tests/expected-fail b/tests/expected-fail
new file mode 100755
index 0000000000..85738c95dc
--- /dev/null
+++ b/tests/expected-fail
@@ -0,0 +1,13 @@
+#!/bin/sh
+
+# meson's "test()" function provides a "should_fail: bool"
argument that
+# checks for a command to exit with a non-zero exit code instead of the usual
+# zero exit code to signal success. If the program under test does so, it is
+# recorded as "EXPECTEDFAIL" instead of "OK". While there is an
argument to be
+# made that the program under test failed as expected, the test in itself is
+# successful and should be recorded as such.
+
+if "$@"
+then
+ exit 1
+fi
diff --git a/tests/meson.build b/tests/meson.build
index 0d76d37959..0ebcdc1496 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -725,9 +725,8 @@ endif
if conf.has('WITH_LIBVIRTD')
test('libvirtd fail with missing config',
- libvirtd_prog,
- args: [ '--config=no-such-conf', '--timeout=5' ],
- should_fail: true,
+ find_program('expected-fail'),
+ args: [ libvirtd_prog, '--config=no-such-conf', '--timeout=5' ],
suite: 'bin',
)
--
2.48.1
4:30 a.m.
New subject: [PATCH] tests: Record negative tests as pass instead of
expected-fail
On Mon, Feb 10, 2025 at 11:25:55 +0100, Tim Wiederhake wrote:
meson's "test()" function provides a "should_fail:
bool" argument that
checks for a command to exit with a non-zero exit code instead of the usual
zero exit code to signal success. If the program under test does so, it is
recorded as "EXPECTEDFAIL" instead of "OK". While there is an
argument to be
made that the program under test failed as expected, the test in itself is
successful and should be recorded as such.
Could you please elaborate what the problem is? The wording of the
Before:
$ meson test
...
151/300 libvirt:bin / libvirtd fail with missing config
EXPECTEDFAIL 0.03s exit status 1
... EXPECTEDFAIL case?
...
Ok: 299
Expected Fail: 1
Fail: 0
Unexpected Pass: 0
Skipped: 0
Timeout: 0
After:
$ meson test
...
151/300 libvirt:bin / libvirtd fail with missing config
OK 0.03s
...
Ok: 300
Expected Fail: 0
Fail: 0
Unexpected Pass: 0
Skipped: 0
Timeout: 0
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
tests/expected-fail | 13 +++++++++++++
tests/meson.build | 5 ++---
2 files changed, 15 insertions(+), 3 deletions(-)
create mode 100755 tests/expected-fail
diff --git a/tests/expected-fail b/tests/expected-fail
new file mode 100755
index 0000000000..85738c95dc
--- /dev/null
+++ b/tests/expected-fail
@@ -0,0 +1,13 @@
+#!/bin/sh
+
+# meson's "test()" function provides a "should_fail: bool"
argument that
+# checks for a command to exit with a non-zero exit code instead of the usual
+# zero exit code to signal success. If the program under test does so, it is
+# recorded as "EXPECTEDFAIL" instead of "OK". While there is an
argument to be
+# made that the program under test failed as expected, the test in itself is
+# successful and should be recorded as such.
+
+if "$@"
+then
+ exit 1
+fi
The idea was exactly to avoid having this extra script.
5:44 a.m.
New subject: [PATCH] tests: Record negative tests as pass instead of
expected-fail
On Mon, Feb 10, 2025 at 11:30:24 +0100, Peter Krempa wrote:
On Mon, Feb 10, 2025 at 11:25:55 +0100, Tim Wiederhake wrote:
> meson's "test()" function provides a "should_fail: bool"
argument that
> checks for a command to exit with a non-zero exit code instead of the usual
> zero exit code to signal success. If the program under test does so, it is
> recorded as "EXPECTEDFAIL" instead of "OK". While there is an
argument to be
> made that the program under test failed as expected, the test in itself is
> successful and should be recorded as such.
Could you please elaborate what the problem is? The wording of the
I've re-read this and it ineed seems that just the wording is the issue.
I'm not sold that we need a script to bypass the return value just for
meson to say "OK".
5:58 a.m.
New subject: [PATCH] tests: Record negative tests as pass instead of
expected-fail
On Mon, 2025-02-10 at 12:44 +0100, Peter Krempa wrote:
On Mon, Feb 10, 2025 at 11:30:24 +0100, Peter Krempa wrote:
> On Mon, Feb 10, 2025 at 11:25:55 +0100, Tim Wiederhake wrote:
> > meson's "test()" function provides a "should_fail:
bool" argument
> > that
> > checks for a command to exit with a non-zero exit code instead of
> > the usual
> > zero exit code to signal success. If the program under test does
> > so, it is
> > recorded as "EXPECTEDFAIL" instead of "OK". While there is
an
> > argument to be
> > made that the program under test failed as expected, the test in
> > itself is
> > successful and should be recorded as such.
>
> Could you please elaborate what the problem is? The wording of the
I've re-read this and it ineed seems that just the wording is the
issue.
I'm not sold that we need a script to bypass the return value just
for
meson to say "OK".
Expected failures or "xfail" results were used as a tool to write a
test for a bug before committing the fix. You'd write the test first,
mark it as "xfail" to make the issue visible without making the entire
test suite fail. The commit that fixes the issue would then remove the
"xfail" marker and demonstrate that it actually fixes the issue, i.e.
makes the test pass.
Using expected-fail is not the right tool to handle a test case that is
always expected to exit with a non-zero exit code. As I said, the test
is "program exits non-zero", and that test passes. There is no failure
in the test, expected or otherwise.
Tim
6:57 a.m.
New subject: [PATCH] tests: Record negative tests as pass instead of
expected-fail
On Mon, Feb 10, 2025 at 12:58:06 +0100, Tim Wiederhake wrote:
On Mon, 2025-02-10 at 12:44 +0100, Peter Krempa wrote:
> On Mon, Feb 10, 2025 at 11:30:24 +0100, Peter Krempa wrote:
> > On Mon, Feb 10, 2025 at 11:25:55 +0100, Tim Wiederhake wrote:
> > > meson's "test()" function provides a "should_fail:
bool" argument
> > > that
> > > checks for a command to exit with a non-zero exit code instead of
> > > the usual
> > > zero exit code to signal success. If the program under test does
> > > so, it is
> > > recorded as "EXPECTEDFAIL" instead of "OK". While
there is an
> > > argument to be
> > > made that the program under test failed as expected, the test in
> > > itself is
> > > successful and should be recorded as such.
> >
> > Could you please elaborate what the problem is? The wording of the
>
> I've re-read this and it ineed seems that just the wording is the
> issue.
>
> I'm not sold that we need a script to bypass the return value just
> for
> meson to say "OK".
>
Expected failures or "xfail" results were used as a tool to write a
test for a bug before committing the fix. You'd write the test first,
mark it as "xfail" to make the issue visible without making the entire
test suite fail. The commit that fixes the issue would then remove the
"xfail" marker and demonstrate that it actually fixes the issue, i.e.
makes the test pass.
Using expected-fail is not the right tool to handle a test case that is
always expected to exit with a non-zero exit code. As I said, the test
is "program exits non-zero", and that test passes. There is no failure
in the test, expected or otherwise.
Meson developers seem to think otherwise:
https://github.com/mesonbuild/meson/pull/4466#issuecomment-435688173
This is not actually true. Using expected failures to mark bugs is
an
antipattern and you should not do that. Tests that should fail are
for things like trying to run an executable with known invalid input.
It must return an error in this case rather than silently accept it.
The documentation also simply states the fact that it's for tests
returning non-zero:
should_fail bool
when true the test is considered passed if the executable returns a non-zero return
value (i.e. reports an error)
Which is why I chose to do this.
Based on the above I'm still not persuaded we want this. In case you
still want to go forward I'd sugest you revert e57ce7fb452 instead of
adding a workaround script.
7:03 a.m.
New subject: [PATCH] tests: Record negative tests as pass instead of
expected-fail
On Mon, Feb 10, 2025 at 01:57:51PM +0100, Peter Krempa wrote:
On Mon, Feb 10, 2025 at 12:58:06 +0100, Tim Wiederhake wrote:
> On Mon, 2025-02-10 at 12:44 +0100, Peter Krempa wrote:
> > On Mon, Feb 10, 2025 at 11:30:24 +0100, Peter Krempa wrote:
> > > On Mon, Feb 10, 2025 at 11:25:55 +0100, Tim Wiederhake wrote:
> > > > meson's "test()" function provides a "should_fail:
bool" argument
> > > > that
> > > > checks for a command to exit with a non-zero exit code instead of
> > > > the usual
> > > > zero exit code to signal success. If the program under test does
> > > > so, it is
> > > > recorded as "EXPECTEDFAIL" instead of "OK". While
there is an
> > > > argument to be
> > > > made that the program under test failed as expected, the test in
> > > > itself is
> > > > successful and should be recorded as such.
> > >
> > > Could you please elaborate what the problem is? The wording of the
> >
> > I've re-read this and it ineed seems that just the wording is the
> > issue.
> >
> > I'm not sold that we need a script to bypass the return value just
> > for
> > meson to say "OK".
> >
>
> Expected failures or "xfail" results were used as a tool to write a
> test for a bug before committing the fix. You'd write the test first,
> mark it as "xfail" to make the issue visible without making the entire
> test suite fail. The commit that fixes the issue would then remove the
> "xfail" marker and demonstrate that it actually fixes the issue, i.e.
> makes the test pass.
>
> Using expected-fail is not the right tool to handle a test case that is
> always expected to exit with a non-zero exit code. As I said, the test
> is "program exits non-zero", and that test passes. There is no failure
> in the test, expected or otherwise.
Meson developers seem to think otherwise:
https://github.com/mesonbuild/meson/pull/4466#issuecomment-435688173
> This is not actually true. Using expected failures to mark bugs is an
> antipattern and you should not do that. Tests that should fail are
> for things like trying to run an executable with known invalid input.
> It must return an error in this case rather than silently accept it.
The documentation also simply states the fact that it's for tests
returning non-zero:
should_fail bool
when true the test is considered passed if the executable returns a non-zero return
value (i.e. reports an error)
Which is why I chose to do this.
Based on the above I'm still not persuaded we want this. In case you
still want to go forward I'd sugest you revert e57ce7fb452 instead of
adding a workaround script.
IMHO based on the meson PR comment above, the current code is working as
Meson developers intended and so we don't need todo anything.
The only issue I see here is that the meson docs should be improved to
better explain their intentions around usage pattersn for the 'should_fail'
flag, as it is ambiguous/open to differing interpretations. They should
expand the docs per that PR comment explanation.
With 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 :|
172
days inactive
172
days old
5 comments
3 participants
participants (3)
-
Daniel P. Berrangé -
Peter Krempa -
Tim Wiederhake