Horton

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

Andrew Dunstan <andrew@dunslane.net>

View thread
From: Andrew Dunstan <andrew@dunslane.net>
To: Sadhuprasad Patro <b.sadhu@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2025-10-10T13:33:10Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Improve TAP tests by replacing ok() with better Test::More functions

  2. Fix matching check in recovery test 042_low_level_backup

  3. pg_createsubscriber: Fix matching check in TAP test

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