Re: Improved TAP tests by replacing sub-optimal uses of ok() with better Test::More functions

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