Explicitly skip TAP tests under Meson if disabled
Under Meson, it is not very easy to see if TAP tests have been enabled
or disabled, if you rely on the default auto setting. You either need
to carefully study the meson setup output, or you notice, what a minute,
didn't there use to be like 250 tests, not only 80?
I think it would be better if we still registered the TAP tests in Meson
even if the tap_tests option is disabled, but with a dummy command that
registers them as skipped. That way you get a more informative output like
Ok: 78
Expected Fail: 0
Fail: 0
Unexpected Pass: 0
Skipped: 187
Timeout: 0
which is really a more accurate representation of what the test run
actually accomplished than "everything Ok".
See attached patch for a possible implementation. (This uses perl as a
hard build requirement. We are planning to do that anyway, but
obviously other implementations, such as using python, would also be
possible.)
Attachments:
0001-Explicitly-skip-TAP-tests-under-Meson-if-disabled.patchtext/plain; charset=UTF-8; name=0001-Explicitly-skip-TAP-tests-under-Meson-if-disabled.patchDownload
From 19e78e4c5a16337c0ac4e661beb4729505736016 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Mon, 30 Oct 2023 05:32:45 -0400
Subject: [PATCH] Explicitly skip TAP tests under Meson if disabled
If the tap_tests option is disabled under Meson, the TAP tests are
currently not registered at all. But this makes it harder to see what
is going one, why suddently there are fewer tests than before.
Instead, register the tests anyway but with a dummy command that marks
them as skipped. That way, the total list and count of tests is
constant whether the option is enabled or not.
---
meson.build | 31 +++++++++++++++++--------------
1 file changed, 17 insertions(+), 14 deletions(-)
diff --git a/meson.build b/meson.build
index 2d516c8f372..1795dd8159e 100644
--- a/meson.build
+++ b/meson.build
@@ -3248,10 +3248,6 @@ foreach test_dir : tests
testport += 1
elif kind == 'tap'
- if not tap_tests_enabled
- continue
- endif
-
test_command = [
perl.path(),
'-I', meson.source_root() / 'src/test/perl',
@@ -3286,16 +3282,23 @@ foreach test_dir : tests
onetap_p = fs.stem(onetap_p)
endif
- test(test_dir['name'] / onetap_p,
- python,
- kwargs: test_kwargs,
- args: testwrap_base + [
- '--testgroup', test_dir['name'],
- '--testname', onetap_p,
- '--', test_command,
- test_dir['sd'] / onetap,
- ],
- )
+ if tap_tests_enabled
+ test(test_dir['name'] / onetap_p,
+ python,
+ kwargs: test_kwargs,
+ args: testwrap_base + [
+ '--testgroup', test_dir['name'],
+ '--testname', onetap_p,
+ '--', test_command,
+ test_dir['sd'] / onetap,
+ ],
+ )
+ else
+ test(test_dir['name'] / onetap_p,
+ perl,
+ args: ['-e', 'print "1..0 # Skipped: TAP tests not enabled"'],
+ kwargs: test_kwargs)
+ endif
endforeach
install_suites += test_group
else
--
2.42.0
Hi,
Under Meson, it is not very easy to see if TAP tests have been enabled
or disabled, if you rely on the default auto setting. You either need
to carefully study the meson setup output, or you notice, what a minute,
didn't there use to be like 250 tests, not only 80?I think it would be better if we still registered the TAP tests in Meson
even if the tap_tests option is disabled, but with a dummy command that
registers them as skipped. That way you get a more informative output likeOk: 78
Expected Fail: 0
Fail: 0
Unexpected Pass: 0
Skipped: 187
Timeout: 0which is really a more accurate representation of what the test run
actually accomplished than "everything Ok".See attached patch for a possible implementation. (This uses perl as a
hard build requirement. We are planning to do that anyway, but
obviously other implementations, such as using python, would also be
possible.)
I tested the patch and it works as intended.
Personally I like the change. It makes the output more explicit. In my
use cases not running TAP tests typically is not something I want . So
I would appreciate being warned with a long list of bright yellow
"SKIP" messages. If I really want to skip TAP tests these messages are
just informative and don't bother me.
+1
--
Best regards,
Aleksander Alekseev
Aleksander Alekseev <aleksander@timescale.com> writes:
Personally I like the change. It makes the output more explicit. In my
use cases not running TAP tests typically is not something I want . So
I would appreciate being warned with a long list of bright yellow
"SKIP" messages. If I really want to skip TAP tests these messages are
just informative and don't bother me.
+1 for counting such tests as "skipped" in the summary. -1 for
emitting a message per skipped test. If I'm intentionally not
running those tests, that would be very annoying noise, and
potentially would obscure messages I actually need to see.
(And about -100 for emitting such messages in yellow. Doesn't
anybody who codes this stuff have a clue about vision problems?)
regards, tom lane
Hi Peter,
You may find value in this Meson PR[0]https://github.com/mesonbuild/meson/pull/12362 adding a skip keyword argument to
Meson's test() function. From what I understand of the PR and your
issue, they seem related. If you could provide a comment describing why
this is valuable to you, it would be good to help the Meson
maintainers understand the use case better.
Thanks!
[0]: https://github.com/mesonbuild/meson/pull/12362
--
Tristan Partin
Neon (https://neon.tech)
On 30.10.23 10:12, Tom Lane wrote:
+1 for counting such tests as "skipped" in the summary. -1 for
emitting a message per skipped test. If I'm intentionally not
running those tests, that would be very annoying noise, and
potentially would obscure messages I actually need to see.
In my usage, those messages only show up in the logs, not during a
normal test run. This is similar to other skip messages, like "skipped
on Windows" or "skipped because LDAP not enabled" etc.
Hi,
On 2023-10-30 05:45:52 -0400, Peter Eisentraut wrote:
Under Meson, it is not very easy to see if TAP tests have been enabled or
disabled, if you rely on the default auto setting. You either need to
carefully study the meson setup output, or you notice, what a minute, didn't
there use to be like 250 tests, not only 80?I think it would be better if we still registered the TAP tests in Meson
even if the tap_tests option is disabled, but with a dummy command that
registers them as skipped. That way you get a more informative output like
Hm, ok. I've never felt I needed this, but I can see the point.
See attached patch for a possible implementation. (This uses perl as a hard
build requirement. We are planning to do that anyway, but obviously other
implementations, such as using python, would also be possible.)
There's already other hard dependencies on perl in the meson build (generating
kwlist etc). We certainly error out if it's not available.
- test(test_dir['name'] / onetap_p, - python, - kwargs: test_kwargs, - args: testwrap_base + [ - '--testgroup', test_dir['name'], - '--testname', onetap_p, - '--', test_command, - test_dir['sd'] / onetap, - ], - ) + if tap_tests_enabled + test(test_dir['name'] / onetap_p, + python, + kwargs: test_kwargs, + args: testwrap_base + [ + '--testgroup', test_dir['name'], + '--testname', onetap_p, + '--', test_command, + test_dir['sd'] / onetap, + ], + ) + else + test(test_dir['name'] / onetap_p, + perl, + args: ['-e', 'print "1..0 # Skipped: TAP tests not enabled"'], + kwargs: test_kwargs) + endif
I'd just use a single test() invocation here, and add an argument to testwrap
indicating that it should print out the skipped message. That way we a) don't
need two test() invocations, b) could still see the test name etc in the test
invocation.
Greetings,
Andres Freund
On 04.11.23 01:51, Andres Freund wrote:
I'd just use a single test() invocation here, and add an argument to testwrap
indicating that it should print out the skipped message. That way we a) don't
need two test() invocations, b) could still see the test name etc in the test
invocation.
Is testwrap only meant to be used with the tap protocol mode of meson's
test()? Otherwise, this skip option would have produce different output
for different protocols.
Hi,
On 2023-11-06 17:46:23 +0100, Peter Eisentraut wrote:
On 04.11.23 01:51, Andres Freund wrote:
I'd just use a single test() invocation here, and add an argument to testwrap
indicating that it should print out the skipped message. That way we a) don't
need two test() invocations, b) could still see the test name etc in the test
invocation.Is testwrap only meant to be used with the tap protocol mode of meson's
test()? Otherwise, this skip option would have produce different output for
different protocols.
Since Daniel added tap support to pg_regress it's only used with tap. If we
add something else, we can add a format parameter?
Greetings,
Andres Freund
On 04.11.23 01:51, Andres Freund wrote:
I'd just use a single test() invocation here, and add an argument to testwrap
indicating that it should print out the skipped message. That way we a) don't
need two test() invocations, b) could still see the test name etc in the test
invocation.
Here is a patch that does it that way.
Attachments:
v2-0001-Explicitly-skip-TAP-tests-under-Meson-if-disabled.patchtext/plain; charset=UTF-8; name=v2-0001-Explicitly-skip-TAP-tests-under-Meson-if-disabled.patchDownload
From d58e65a71d2fab40ab22f047999efe06d96d8688 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Wed, 15 Nov 2023 11:00:49 +0100
Subject: [PATCH v2] Explicitly skip TAP tests under Meson if disabled
If the tap_tests option is disabled under Meson, the TAP tests are
currently not registered at all. But this makes it harder to see what
is going one, why suddently there are fewer tests than before.
Instead, run testwrap with an option that marks the test as skipped.
That way, the total list and count of tests is constant whether the
option is enabled or not.
---
meson.build | 5 +++--
src/tools/testwrap | 5 +++++
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/meson.build b/meson.build
index 47c8fcdc53..286d7e4269 100644
--- a/meson.build
+++ b/meson.build
@@ -3252,8 +3252,9 @@ foreach test_dir : tests
testport += 1
elif kind == 'tap'
+ testwrap_tap = testwrap_base
if not tap_tests_enabled
- continue
+ testwrap_tap += ['--skip', 'TAP tests not enabled']
endif
test_command = [
@@ -3293,7 +3294,7 @@ foreach test_dir : tests
test(test_dir['name'] / onetap_p,
python,
kwargs: test_kwargs,
- args: testwrap_base + [
+ args: testwrap_tap + [
'--testgroup', test_dir['name'],
'--testname', onetap_p,
'--', test_command,
diff --git a/src/tools/testwrap b/src/tools/testwrap
index 7a64fe76a2..d01e61051c 100755
--- a/src/tools/testwrap
+++ b/src/tools/testwrap
@@ -12,6 +12,7 @@ parser.add_argument('--srcdir', help='source directory of test', type=str)
parser.add_argument('--basedir', help='base directory of test', type=str)
parser.add_argument('--testgroup', help='test group', type=str)
parser.add_argument('--testname', help='test name', type=str)
+parser.add_argument('--skip', help='skip test (with reason)', type=str)
parser.add_argument('test_command', nargs='*')
args = parser.parse_args()
@@ -23,6 +24,10 @@ print('# executing test in {} group {} test {}'.format(
testdir, args.testgroup, args.testname))
sys.stdout.flush()
+if args.skip is not None:
+ print('1..0 # Skipped: ' + args.skip)
+ sys.exit(0)
+
if os.path.exists(testdir) and os.path.isdir(testdir):
shutil.rmtree(testdir)
os.makedirs(testdir)
--
2.42.1
On 2023-11-15 11:02:19 +0100, Peter Eisentraut wrote:
On 04.11.23 01:51, Andres Freund wrote:
I'd just use a single test() invocation here, and add an argument to testwrap
indicating that it should print out the skipped message. That way we a) don't
need two test() invocations, b) could still see the test name etc in the test
invocation.Here is a patch that does it that way.
WFM!