From ce1b0f9da03e1cebc293f60b378079b22bd7cc0f Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Wed, 22 Jan 2025 14:47:13 +0900 Subject: Improve grammar of options for command arrays in TAP tests This commit rewrites a good chunk of the command arrays in TAP tests with a grammar based on the following rules: - Fat commas are used between option names and their values, making it clear to both humans and perltidy that values and names are bound together. This is particularly useful for the readability of multi-line command arrays, and there are plenty of them in the TAP tests. Most of the test code is updated to use this style. Some commands used parenthesis to show the link, or attached values and options in a single string. These are updated to use fat commas instead. - Option names are switched to use their long names, making them more self-documented. Based on a suggestion by Andrew Dunstan. - Add some trailing commas after the last item in multi-line arrays, which is a common perl style. Not all the places are taken care of, but this covers a very good chunk of them. Author: Dagfinn Ilmari Mannsåker Reviewed-by: Michael Paquier, Peter Smith, Euler Taveira Discussion: https://postgr.es/m/87jzc46d8u.fsf@wibble.ilmari.org --- src/test/recovery/t/001_stream_rep.pl | 7 ++++-- src/test/recovery/t/003_recovery_targets.pl | 14 +++++++----- src/test/recovery/t/017_shm.pl | 14 ++++++------ src/test/recovery/t/024_archive_recovery.pl | 7 +++--- src/test/recovery/t/027_stream_regress.pl | 34 ++++++++++++++++------------- src/test/ssl/t/001_ssltests.pl | 32 +++++++++++++-------------- 6 files changed, 60 insertions(+), 48 deletions(-) (limited to 'src/test') diff --git a/src/test/recovery/t/001_stream_rep.pl b/src/test/recovery/t/001_stream_rep.pl index c97adf17185..ee57d234c86 100644 --- a/src/test/recovery/t/001_stream_rep.pl +++ b/src/test/recovery/t/001_stream_rep.pl @@ -566,8 +566,11 @@ my $connstr = $node_primary->connstr('postgres') . " replication=database"; # a replication command and a SQL command. $node_primary->command_fails_like( [ - 'psql', '-X', '-c', "SELECT pg_backup_start('backup', true)", - '-c', 'BASE_BACKUP', '-d', $connstr + 'psql', + '--no-psqlrc', + '--command' => "SELECT pg_backup_start('backup', true)", + '--command' => 'BASE_BACKUP', + '--dbname' => $connstr ], qr/a backup is already in progress in this session/, 'BASE_BACKUP cannot run in session already running backup'); diff --git a/src/test/recovery/t/003_recovery_targets.pl b/src/test/recovery/t/003_recovery_targets.pl index a0e8ffdea40..0ae2e982727 100644 --- a/src/test/recovery/t/003_recovery_targets.pl +++ b/src/test/recovery/t/003_recovery_targets.pl @@ -57,7 +57,7 @@ $node_primary->init(has_archiving => 1, allows_streaming => 1); # Bump the transaction ID epoch. This is useful to stress the portability # of recovery_target_xid parsing. -system_or_bail('pg_resetwal', '--epoch', '1', $node_primary->data_dir); +system_or_bail('pg_resetwal', '--epoch' => '1', $node_primary->data_dir); # Start it $node_primary->start; @@ -147,8 +147,10 @@ recovery_target_time = '$recovery_time'"); my $res = run_log( [ - 'pg_ctl', '-D', $node_standby->data_dir, '-l', - $node_standby->logfile, 'start' + 'pg_ctl', + '--pgdata' => $node_standby->data_dir, + '--log' => $node_standby->logfile, + 'start', ]); ok(!$res, 'invalid recovery startup fails'); @@ -168,8 +170,10 @@ $node_standby->append_conf('postgresql.conf', run_log( [ - 'pg_ctl', '-D', $node_standby->data_dir, '-l', - $node_standby->logfile, 'start' + 'pg_ctl', + '--pgdata' => $node_standby->data_dir, + '--log' => $node_standby->logfile, + 'start', ]); # wait for postgres to terminate diff --git a/src/test/recovery/t/017_shm.pl b/src/test/recovery/t/017_shm.pl index fa712fcb4b8..e2e85d471fe 100644 --- a/src/test/recovery/t/017_shm.pl +++ b/src/test/recovery/t/017_shm.pl @@ -160,13 +160,13 @@ my $pre_existing_msg = qr/pre-existing shared memory block/; like(slurp_file($gnat->logfile), $pre_existing_msg, 'detected live backend via shared memory'); # Reject single-user startup. -my $single_stderr; -ok( !run_log( - [ 'postgres', '--single', '-D', $gnat->data_dir, 'template1' ], - '<', \undef, '2>', \$single_stderr), - 'live query blocks --single'); -print STDERR $single_stderr; -like($single_stderr, $pre_existing_msg, +command_fails_like( + [ + 'postgres', '--single', + '-D' => $gnat->data_dir, + 'template1' + ], + $pre_existing_msg, 'single-user mode detected live backend via shared memory'); log_ipcs(); diff --git a/src/test/recovery/t/024_archive_recovery.pl b/src/test/recovery/t/024_archive_recovery.pl index 5f8054b5376..b4527ec0843 100644 --- a/src/test/recovery/t/024_archive_recovery.pl +++ b/src/test/recovery/t/024_archive_recovery.pl @@ -76,9 +76,10 @@ sub test_recovery_wal_level_minimal # that the server ends with an error during recovery. run_log( [ - 'pg_ctl', '-D', - $recovery_node->data_dir, '-l', - $recovery_node->logfile, 'start' + 'pg_ctl', + '--pgdata' => $recovery_node->data_dir, + '--log' => $recovery_node->logfile, + 'start', ]); # wait for postgres to terminate diff --git a/src/test/recovery/t/027_stream_regress.pl b/src/test/recovery/t/027_stream_regress.pl index 467113b1379..bab7b28084b 100644 --- a/src/test/recovery/t/027_stream_regress.pl +++ b/src/test/recovery/t/027_stream_regress.pl @@ -105,19 +105,23 @@ $node_primary->wait_for_replay_catchup($node_standby_1); # Perform a logical dump of primary and standby, and check that they match command_ok( [ - 'pg_dumpall', '-f', $outputdir . '/primary.dump', - '--no-sync', '-p', $node_primary->port, - '--no-unlogged-table-data' # if unlogged, standby has schema only + 'pg_dumpall', + '--file' => $outputdir . '/primary.dump', + '--no-sync', + '--port' => $node_primary->port, + '--no-unlogged-table-data', # if unlogged, standby has schema only ], 'dump primary server'); command_ok( [ - 'pg_dumpall', '-f', $outputdir . '/standby.dump', - '--no-sync', '-p', $node_standby_1->port + 'pg_dumpall', + '--file' => $outputdir . '/standby.dump', + '--no-sync', + '--port' => $node_standby_1->port, ], 'dump standby server'); command_ok( - [ 'diff', $outputdir . '/primary.dump', $outputdir . '/standby.dump' ], + [ 'diff', $outputdir . '/primary.dump', $outputdir . '/standby.dump', ], 'compare primary and standby dumps'); # Likewise for the catalogs of the regression database, after disabling @@ -128,29 +132,29 @@ $node_primary->wait_for_replay_catchup($node_standby_1); command_ok( [ 'pg_dump', - ('--schema', 'pg_catalog'), - ('-f', $outputdir . '/catalogs_primary.dump'), + '--schema' => 'pg_catalog', + '--file' => $outputdir . '/catalogs_primary.dump', '--no-sync', - ('-p', $node_primary->port), + '--port', $node_primary->port, '--no-unlogged-table-data', - 'regression' + 'regression', ], 'dump catalogs of primary server'); command_ok( [ 'pg_dump', - ('--schema', 'pg_catalog'), - ('-f', $outputdir . '/catalogs_standby.dump'), + '--schema' => 'pg_catalog', + '--file' => $outputdir . '/catalogs_standby.dump', '--no-sync', - ('-p', $node_standby_1->port), - 'regression' + '--port' => $node_standby_1->port, + 'regression', ], 'dump catalogs of standby server'); command_ok( [ 'diff', $outputdir . '/catalogs_primary.dump', - $outputdir . '/catalogs_standby.dump' + $outputdir . '/catalogs_standby.dump', ], 'compare primary and standby catalog dumps'); diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl index c4a8dbb0015..5422511d4ab 100644 --- a/src/test/ssl/t/001_ssltests.pl +++ b/src/test/ssl/t/001_ssltests.pl @@ -543,12 +543,14 @@ $node->connect_fails( # pg_stat_ssl command_like( [ - 'psql', '-X', - '-A', '-F', - ',', '-P', - 'null=_null_', '-d', - "$common_connstr sslrootcert=invalid", '-c', - "SELECT * FROM pg_stat_ssl WHERE pid = pg_backend_pid()" + 'psql', + '--no-psqlrc', + '--no-align', + '--field-separator' => ',', + '--pset', => 'null=_null_', + '--dbname' => "$common_connstr sslrootcert=invalid", + '--command' => + "SELECT * FROM pg_stat_ssl WHERE pid = pg_backend_pid()" ], qr{^pid,ssl,version,cipher,bits,client_dn,client_serial,issuer_dn\r?\n ^\d+,t,TLSv[\d.]+,[\w-]+,\d+,_null_,_null_,_null_\r?$}mx, @@ -742,17 +744,15 @@ else command_like( [ 'psql', - '-X', - '-A', - '-F', - ',', - '-P', - 'null=_null_', - '-d', - "$common_connstr user=ssltestuser sslcert=ssl/client.crt " + '--no-psqlrc', + '--no-align', + '--field-separator' => ',', + '--pset' => 'null=_null_', + '--dbname' => + "$common_connstr user=ssltestuser sslcert=ssl/client.crt " . sslkey('client.key'), - '-c', - "SELECT * FROM pg_stat_ssl WHERE pid = pg_backend_pid()" + '--command' => + "SELECT * FROM pg_stat_ssl WHERE pid = pg_backend_pid()" ], qr{^pid,ssl,version,cipher,bits,client_dn,client_serial,issuer_dn\r?\n ^\d+,t,TLSv[\d.]+,[\w-]+,\d+,/?CN=ssltestuser,$serialno,/?\QCN=Test CA for PostgreSQL SSL regression test client certs\E\r?$}mx, -- cgit v1.2.3