Re: Re: In-core regression tests for replication, cascading, archiving, PITR, etc.

On Wed, Dec 02, 2015 at 06:59:09PM -0300, Alvaro Herrera wrote:
> It seemed better to me to have the import list be empty, i.e. "use
> TestLib ()" and then qualify the routine names inside PostgresNode,
> instead of having to list the names of the routines to import, so I
> pushed it that way after running the tests a few more times.

I inspected commit 1caef31:

> --- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
> +++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl

> -like($recovery_conf, qr/^standby_mode = 'on[']$/m, 'recovery.conf sets standby_mode');
> -like($recovery_conf, qr/^primary_conninfo = '.*port=$ENV{PGPORT}.*'$/m, 'recovery.conf sets primary_conninfo');
> -
> -command_ok([ 'pg_basebackup', '-D', "$tempdir/backupxf", '-X', 'fetch' ],
> +like(
> + $recovery_conf,
> + qr/^standby_mode = 'on[']$/m,
> + 'recovery.conf sets standby_mode');
> +like(
> + $recovery_conf,
> + qr/^primary_conninfo = '.*port=$ENV{PGPORT}.*'$/m,
> + 'recovery.conf sets primary_conninfo');

This now elicits a diagnostic:

Use of uninitialized value $ENV{"PGPORT"} in regexp compilation at t/010_pg_basebackup.pl line 175, <> line 1.

> --- a/src/bin/pg_controldata/t/001_pg_controldata.pl
> +++ b/src/bin/pg_controldata/t/001_pg_controldata.pl

> -standard_initdb "$tempdir/data";
> -command_like([ 'pg_controldata', "$tempdir/data" ],
> +
> +my $node = get_new_node();
> +$node->init;
> +$node->start;

Before the commit, this test did not start a server.

> --- /dev/null
> +++ b/src/test/perl/PostgresNode.pm

> +sub _update_pid
> +{
> + my $self = shift;
> +
> + # If we can open the PID file, read its first line and that's the PID we
> + # want. If the file cannot be opened, presumably the server is not
> + # running; don't be noisy in that case.
> + open my $pidfile, $self->data_dir . "/postmaster.pid";
> + if (not defined $pidfile)

$ grep -h 'at /.*line' src/bin/*/tmp_check/log/* |sort|uniq -c
1 cannot remove directory for /data/nmisch/src/pg/postgresql/src/bin/scripts/tmp_testNNCZ: Directory not empty at /home/nmisch/sw/cpan/lib/perl5/File/Temp.pm line 784
1 cannot remove directory for /data/nmisch/src/pg/postgresql/src/bin/scripts/tmp_testNNCZ/pgdata/base/13264: Directory not empty at /home/nmisch/sw/cpan/lib/perl5/File/Temp.pm line 784
1 cannot remove directory for /data/nmisch/src/pg/postgresql/src/bin/scripts/tmp_testNNCZ/pgdata/base: Directory not empty at /home/nmisch/sw/cpan/lib/perl5/File/Temp.pm line 784
1 cannot remove directory for /data/nmisch/src/pg/postgresql/src/bin/scripts/tmp_testNNCZ/pgdata: Directory not empty at /home/nmisch/sw/cpan/lib/perl5/File/Temp.pm line 784
28 readline() on closed filehandle $pidfile at /data/nmisch/src/pg/postgresql/src/bin/pg_rewind/../../../src/test/perl/PostgresNode.pm line 308.
28 Use of uninitialized value in concatenation (.) or string at /data/nmisch/src/pg/postgresql/src/bin/pg_rewind/../../../src/test/perl/PostgresNode.pm line 309.

$pidfile is always defined. Test the open() return value.

> + {
> + $self->{_pid} = undef;
> + print "# No postmaster PID\n";
> + return;
> + }
> +
> + $self->{_pid} = <$pidfile>;

chomp() that value to remove its trailing newline.

> + print "# Postmaster PID is $self->{_pid}\n";
> + close $pidfile;
> +}

> + my $devnull = $TestLib::windows_os ? "nul" : "/dev/null";

Unused variable.

> +sub DESTROY
> +{
> + my $self = shift;
> + return if not defined $self->{_pid};
> + print "# signalling QUIT to $self->{_pid}\n";
> + kill 'QUIT', $self->{_pid};

On Windows, that kill() is the wrong thing. I suspect "pg_ctl kill" will be
the right thing, but that warrants specific testing.

Postmaster log file names became less informative. Before the commit:

-rw-------. 1 nmisch nmisch 211265 2015-12-06 22:35:59.931114215 +0000 regress_log_001_basic
-rw-------. 1 nmisch nmisch 268887 2015-12-06 22:36:21.871165951 +0000 regress_log_002_databases
-rw-------. 1 nmisch nmisch 206808 2015-12-06 22:36:41.861213736 +0000 regress_log_003_extrafiles
-rw-------. 1 nmisch nmisch 7464 2015-12-06 22:37:00.371256795 +0000 master.log
-rw-------. 1 nmisch nmisch 6648 2015-12-06 22:37:01.381259211 +0000 standby.log
-rw-------. 1 nmisch nmisch 208561 2015-12-06 22:37:02.381261374 +0000 regress_log_004_pg_xlog_symlink

After:

-rw-------. 1 nmisch nmisch 219581 2015-12-06 23:00:50.504643175 +0000 regress_log_001_basic
-rw-------. 1 nmisch nmisch 276315 2015-12-06 23:01:12.924697085 +0000 regress_log_002_databases
-rw-------. 1 nmisch nmisch 213940 2015-12-06 23:01:33.574747195 +0000 regress_log_003_extrafiles
-rw-------. 1 nmisch nmisch 4114 2015-12-06 23:01:40.914764850 +0000 node_57834.log
-rw-------. 1 nmisch nmisch 6166 2015-12-06 23:01:43.184770615 +0000 node_57833.log
-rw-------. 1 nmisch nmisch 5550 2015-12-06 23:01:52.504792997 +0000 node_57835.log
-rw-------. 1 nmisch nmisch 9494 2015-12-06 23:01:53.514794802 +0000 node_57836.log
-rw-------. 1 nmisch nmisch 216212 2015-12-06 23:01:54.544797739 +0000 regress_log_004_pg_xlog_symlink

Should nodes get a name, so we instead see master_57834.log?

See also the things Tom Lane identified in <27550(dot)1449342569(at)sss(dot)pgh(dot)pa(dot)us>.

nm