On 2025-10-10 Fr 1:52 AM, Sadhuprasad Patro wrote:
Hi all,
I've noticed that many TAP tests in the codebase make sub-optimal use
of the "|ok()"| function. Specifically, |ok()| is often used for
expressions involving comparison operators or regex matches, which is
not ideal because other Test::More functions provide much clearer
diagnostic messages when tests fail.
For example, instead of writing:
ok($var =~ /foo/, "found foo")
it’s better to write:
like($var, /foo/, "found foo")
I experimented by modifying a TAP test in |src/bin/pg_dump| to
deliberately fail using |ok()|. The failure output was quite minimal
and didn’t give much detail:
# +++ tap check in src/bin/pg_dump +++
t/005_pg_dump_filterfile.pl <http://005_pg_dump_filterfile.pl> .. 1/?
# Failed test 'table one dumped'
# at t/005_pg_dump_filterfile.pl <http://005_pg_dump_filterfile.pl>
line 103.
t/005_pg_dump_filterfile.pl <http://005_pg_dump_filterfile.pl> .. 57/?
# Looks like you failed 1 test of 108.
t/005_pg_dump_filterfile.pl <http://005_pg_dump_filterfile.pl> ..
Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/108 subtests
Test Summary Report
-------------------
t/005_pg_dump_filterfile.pl <http://005_pg_dump_filterfile.pl> (Wstat:
256 (exited 1) Tests: 108 Failed: 1)
Failed test: 2
Non-zero exit status: 1
Then I changed the same test to use |like()| instead of |ok()|, which
produced much more informative diagnostics:
# +++ tap check in src/bin/pg_dump +++
t/005_pg_dump_filterfile.pl <http://005_pg_dump_filterfile.pl> .. 1/?
# Failed test 'table one dumped'
# at t/005_pg_dump_filterfile.pl <http://005_pg_dump_filterfile.pl>
line 103.
#
# '--
# '
*/# doesn't match '(?^m:^CREATE TABLE public1\.table_one)'/*
t/005_pg_dump_filterfile.pl <http://005_pg_dump_filterfile.pl> .. 41/?
# Looks like you failed 1 test of 108.
t/005_pg_dump_filterfile.pl <http://005_pg_dump_filterfile.pl> ..
Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/108 subtests
Test Summary Report
-------------------
t/005_pg_dump_filterfile.pl <http://005_pg_dump_filterfile.pl> (Wstat:
256 (exited 1) Tests: 108 Failed: 1)
Failed test: 2
Non-zero exit status: 1
Based on this, I’ve replaced all such uses of |ok()| with the more
appropriate |is()|, |isnt()|, |like()|, |unlike()|, and |cmp_ok()|
functions, depending on the test case.
Please find the attached patch implementing these improvements...
'
Great, I think this is a definite improvement. I saw someone recently
complaining about this overuse of ok(), so thanks for doing the work to
improve it.
cheers
andrew
--
Andrew Dunstan
EDB:https://www.enterprisedb.com
