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

From Andrew Dunstan
Subject Re: Improved TAP tests by replacing sub-optimal uses of ok() with better Test::More functions
Date
Msg-id f8e47e3e-549b-4cc9-9c20-dfbd49dad644@dunslane.net
Whole thread Raw
In response to Improved TAP tests by replacing sub-optimal uses of ok() with better Test::More functions  (Sadhuprasad Patro <b.sadhu@gmail.com>)
List pgsql-hackers


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 .. 1/?
#   Failed test 'table one dumped'
#   at t/005_pg_dump_filterfile.pl line 103.
t/005_pg_dump_filterfile.pl .. 57/? # Looks like you failed 1 test of 108.
t/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 (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 .. 1/?
#   Failed test 'table one dumped'
#   at t/005_pg_dump_filterfile.pl line 103.
#
            #                   '--
# '
#     doesn't match '(?^m:^CREATE TABLE public1\.table_one)'
t/005_pg_dump_filterfile.pl .. 41/? # Looks like you failed 1 test of 108.
t/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 (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

pgsql-hackers by date:

Previous
From: Robert Haas
Date:
Subject: Re: Thoughts on a "global" client configuration?
Next
From: Alvaro Herrera
Date:
Subject: Re: Adding REPACK [concurrently]