Re: Re: In-core regression tests for replication, cascading, archiving, PITR, etc. - Mailing list pgsql-hackers

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.pmline 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.pmline 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.pmline 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.pmline 784    28 readline() on closed filehandle $pidfile at
/data/nmisch/src/pg/postgresql/src/bin/pg_rewind/../../../src/test/perl/PostgresNode.pmline 308.    28 Use of
uninitializedvalue in concatenation (.) or string at
/data/nmisch/src/pg/postgresql/src/bin/pg_rewind/../../../src/test/perl/PostgresNode.pmline 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.1449342569@sss.pgh.pa.us>.

nm