Make PostgresNode::append_conf append a newline automatically.

Although the documentation for append_conf said clearly that it didn't
add a newline, many test authors seem to have forgotten that ... or maybe
they just consulted the example at the top of the POD documentation,
which clearly shows adding a config entry without bothering to add a
trailing newline.  The worst part of that is that it works, as long as
you don't do it more than once, since the backend isn't picky about
whether config files end with newlines.  So there's not a strong forcing
function reminding test authors not to do it like that.  Upshot is that
this is a terribly fragile way to go about things, and there's at least
one existing test case that is demonstrably broken and not testing what
it thinks it is.

Let's just make append_conf append a newline, instead; that is clearly
way safer than the old definition.

I also cleaned up a few call sites that were unnecessarily ugly.
(I left things alone in places where it's plausible that additional
config lines would need to be added someday.)

Back-patch the change in append_conf itself to 9.6 where it was added,
as having a definitional inconsistency between branches would obviously
be pretty hazardous for back-patching TAP tests.  The other changes are
just cosmetic and don't need to be back-patched.

Discussion: https://postgr.es/m/19751.1492892376@sss.pgh.pa.us

diff --git a/src/test/modules/commit_ts/t/004_restart.pl b/src/test/modules/commit_ts/t/004_restart.pl
index a0a24971d2073e989a0c88f14fb0104423091ad2..b686925a7e0f0850df0912c2325b11738d48f377 100644 (file)
--- a/src/test/modules/commit_ts/t/004_restart.pl
+++ b/src/test/modules/commit_ts/t/004_restart.pl
@@ -7,10 +7,7 @@ use Test::More tests => 16;
 
 my $node_master = get_new_node('master');
 $node_master->init(allows_streaming => 1);
-$node_master->append_conf(
-   'postgresql.conf', qq(
-track_commit_timestamp = on
-));
+$node_master->append_conf('postgresql.conf', 'track_commit_timestamp = on');
 $node_master->start;
 
 my ($ret, $stdout, $stderr);
@@ -75,10 +72,7 @@ is($after_restart_ts, $before_restart_ts,
 
 # Now disable commit timestamps
 
-$node_master->append_conf(
-   'postgresql.conf', qq(
-track_commit_timestamp = off
-));
+$node_master->append_conf('postgresql.conf', 'track_commit_timestamp = off');
 
 $node_master->stop('fast');
 $node_master->start;
@@ -110,10 +104,7 @@ like(
    'expected error from disabled tx when committs disabled');
 
 # Re-enable, restart and ensure we can still get the old timestamps
-$node_master->append_conf(
-   'postgresql.conf', qq(
-track_commit_timestamp = on
-));
+$node_master->append_conf('postgresql.conf', 'track_commit_timestamp = on');
 
 $node_master->stop('fast');
 $node_master->start;

diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
index cb84f1f2c6dea4b52be669d66b813967b675ed64..6ee412ed1dc311dfe5fb1e9b2f525e7abb38482f 100644 (file)
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
@@ -455,7 +455,7 @@ A shortcut method to append to files like pg_hba.conf and postgresql.conf.
 Does no validation or sanity checking. Does not reload the configuration
 after writing.
 
-A newline is NOT automatically appended to the string.
+A newline is automatically appended to the string.
 
 =cut
 
@@ -465,7 +465,7 @@ sub append_conf
 
    my $conffile = $self->data_dir . '/' . $filename;
 
-   TestLib::append_to_file($conffile, $str);
+   TestLib::append_to_file($conffile, $str . "\n");
 }
 
 =pod

diff --git a/src/test/recovery/t/001_stream_rep.pl b/src/test/recovery/t/001_stream_rep.pl
index ccd59433dab98f91903644ed5265c5ba531c6bb1..0ebe366a016708cfabaaaa909320cec0d743fd04 100644 (file)
--- a/src/test/recovery/t/001_stream_rep.pl
+++ b/src/test/recovery/t/001_stream_rep.pl
@@ -113,16 +113,16 @@ note "switching to physical replication slot";
 # standbys. Since we're going to be testing things that affect the slot state,
 # also increase the standby feedback interval to ensure timely updates.
 my ($slotname_1, $slotname_2) = ('standby_1', 'standby_2');
-$node_master->append_conf('postgresql.conf', "max_replication_slots = 4\n");
+$node_master->append_conf('postgresql.conf', "max_replication_slots = 4");
 $node_master->restart;
 is($node_master->psql('postgres', qq[SELECT pg_create_physical_replication_slot('$slotname_1');]), 0, 'physical slot created on master');
-$node_standby_1->append_conf('recovery.conf', "primary_slot_name = $slotname_1\n");
-$node_standby_1->append_conf('postgresql.conf', "wal_receiver_status_interval = 1\n");
-$node_standby_1->append_conf('postgresql.conf', "max_replication_slots = 4\n");
+$node_standby_1->append_conf('recovery.conf', "primary_slot_name = $slotname_1");
+$node_standby_1->append_conf('postgresql.conf', "wal_receiver_status_interval = 1");
+$node_standby_1->append_conf('postgresql.conf', "max_replication_slots = 4");
 $node_standby_1->restart;
 is($node_standby_1->psql('postgres', qq[SELECT pg_create_physical_replication_slot('$slotname_2');]), 0, 'physical slot created on intermediate replica');
-$node_standby_2->append_conf('recovery.conf', "primary_slot_name = $slotname_2\n");
-$node_standby_2->append_conf('postgresql.conf', "wal_receiver_status_interval = 1\n");
+$node_standby_2->append_conf('recovery.conf', "primary_slot_name = $slotname_2");
+$node_standby_2->append_conf('postgresql.conf', "wal_receiver_status_interval = 1");
 $node_standby_2->restart;
 
 sub get_slot_xmins

diff --git a/src/test/recovery/t/002_archiving.pl b/src/test/recovery/t/002_archiving.pl
index 83b43bf84d41fc4428ef3500cbdf1f0bde102fcb..e4a643d82d2ecb7bd6d5ccd69e8f6969bc737b6c 100644 (file)
--- a/src/test/recovery/t/002_archiving.pl
+++ b/src/test/recovery/t/002_archiving.pl
@@ -23,10 +23,8 @@ $node_master->backup($backup_name);
 my $node_standby = get_new_node('standby');
 $node_standby->init_from_backup($node_master, $backup_name,
    has_restoring => 1);
-$node_standby->append_conf(
-   'postgresql.conf', qq(
-wal_retrieve_retry_interval = '100ms'
-));
+$node_standby->append_conf('postgresql.conf',
+   "wal_retrieve_retry_interval = '100ms'");
 $node_standby->start;
 
 # Create some content on master

diff --git a/src/test/recovery/t/003_recovery_targets.pl b/src/test/recovery/t/003_recovery_targets.pl
index b7b0caae68724478cd3abc7959ebd8ec12fc684d..203a46419e00789a8482bc658fc177d1a280bb66 100644 (file)
--- a/src/test/recovery/t/003_recovery_targets.pl
+++ b/src/test/recovery/t/003_recovery_targets.pl
@@ -23,9 +23,7 @@ sub test_recovery_standby
    foreach my $param_item (@$recovery_params)
    {
        $node_standby->append_conf(
-           'recovery.conf',
-           qq($param_item
-));
+           'recovery.conf', qq($param_item));
    }
 
    $node_standby->start;