Re: pg_waldump: add test for coverage

From: Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>
To: Dong Wook Lee <sh95119(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: pg_waldump: add test for coverage
Date: 2022-09-06 05:57:57
Message-ID: 9062096a-f079-6218-b80b-f292675d2700@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 23.08.22 03:50, Dong Wook Lee wrote:
> Hi Hackers,
> I wrote a test for coverage.
> Unfortunately, it seems to take quite a while to run the test.
> I want to improve these execution times, but I don't know exactly what to do.
> Therefore, I want to hear feedback from many people.

I don't find these tests to be particularly slow. How long do they take
for you to run?

A couple of tips:

- You should give each test a name. That's why each test function has a
(usually) last argument that takes a string.

- You could use command_like() to run a command and check that it exits
successfully and check its standard out. For example, instead of

# test pg_waldump with -F (main)
IPC::Run::run [ 'pg_waldump', "$wal_dump_path", '-F', 'main' ], '>',
\$stdout, '2>', \$stderr;
isnt($stdout, '', "");

it is better to write

command_like([ 'pg_waldump', "$wal_dump_path", '-F', 'main' ],
qr/TODO/, 'test -F (main)');

- It would be useful to test the actual output (that is, fill in the
TODO above). I don't know what the best way to do that is -- that is
part of designing these tests.

Also,

- Your patch introduces a spurious blank line at the end of the test file.

- For portability, options must be before non-option arguments. So
instead of

[ 'pg_waldump', "$wal_dump_path", '-F', 'main' ]

it should be

[ 'pg_waldump', '-F', 'main', "$wal_dump_path" ]

I think having some more test coverage for pg_waldump would be good, so
I encourage you to continue working on this.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2022-09-06 05:59:58 Re: TRAP: FailedAssertion("prev_first_lsn < cur_txn->first_lsn", File: "reorderbuffer.c", Line: 927, PID: 568639)
Previous Message Tom Lane 2022-09-06 05:57:53 Re: [PATCH] Renumber confusing value for GUC_UNIT_BYTE