From: | Peter Smith <smithpb2250(at)gmail(dot)com> |
---|---|
To: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | pg_createsubscriber TAP test wrapping makes command options hard to read. |
Date: | 2024-12-12 01:27:54 |
Message-ID: | CAHut+PvfaUZQR3rEnj-Dcg_6713Wu4yRdd-yZkkC9NVsARht8A@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
While reviewing a patch for another pg_createsubscriber thread I found
that multiple TAP tests have a terrible wrapping where the command
options and their associated oparg are separated on different lines
instead of paired together nicely. This makes it unnecessarily
difficult to see what the test is doing.
Please find the attached patch that changes the wrapping to improve
the command option readability. For example,
BEFORE
command_ok(
[
'pg_createsubscriber', '--verbose',
'--recovery-timeout', "$PostgreSQL::Test::Utils::timeout_default",
'--dry-run', '--pgdata',
$node_s->data_dir, '--publisher-server',
$node_p->connstr($db1), '--socketdir',
$node_s->host, '--subscriber-port',
$node_s->port, '--publication',
'pub1', '--publication',
'pub2', '--subscription',
'sub1', '--subscription',
'sub2', '--database',
$db1, '--database',
$db2
],
'run pg_createsubscriber --dry-run on node S');
AFTER
command_ok(
[
'pg_createsubscriber', '--verbose',
'--recovery-timeout', "$PostgreSQL::Test::Utils::timeout_default",
'--dry-run',
'--pgdata', $node_s->data_dir,
'--publisher-server', $node_p->connstr($db1),
'--socketdir', $node_s->host,
'--subscriber-port', $node_s->port,
'--publication', 'pub1',
'--publication', 'pub2',
'--subscription', 'sub1',
'--subscription', 'sub2',
'--database', $db1,
'--database', $db2
],
'run pg_createsubscriber --dry-run on node S');
~~~
BTW, now that they are more readable I can see that there are some anomalies:
1. There is a test (the last one in the patch) that has multiple
'--verbose' switches. There is no comment to say it was deliberate, so
it looks suspiciously accidental.
2. The same test names a publication as 'Pub2' instead of 'pub2'.
Again, there is no comment to say it was deliberate so was that case
change also accidental?
======
Kind Regards,
Peter Smith.
Fujitsu Australia
Attachment | Content-Type | Size |
---|---|---|
v1-0001-Modify-wrapping-to-make-command-options-easier-to.patch | application/octet-stream | 5.8 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2024-12-12 01:34:28 | Re: Add Postgres module info |
Previous Message | Andrei Lepikhov | 2024-12-12 00:49:32 | Re: Add Postgres module info |