Re: tests for client programs - Mailing list pgsql-hackers

From Andres Freund
Subject Re: tests for client programs
Date
Msg-id 20140404144446.GF14419@alap3.anarazel.de
Whole thread Raw
In response to Re: tests for client programs  (Peter Eisentraut <peter_e@gmx.net>)
Responses Re: tests for client programs
Re: tests for client programs
List pgsql-hackers
Hi,

I personally would very much like to get this patch commited. It doesn't
have much risk in destabilizing stuff, rather the contrary.

Peter, what's you opinion about the current state?

On 2014-02-27 21:44:48 -0500, Peter Eisentraut wrote:
> diff --git a/doc/src/sgml/regress.sgml b/doc/src/sgml/regress.sgml
> index 16b3621..aee049a 100644
> --- a/doc/src/sgml/regress.sgml
> +++ b/doc/src/sgml/regress.sgml
> @@ -204,6 +204,12 @@ <title>Additional Test Suites</title>
>       located in <filename>src/test/isolation</>.
>      </para>
>     </listitem>
> +   <listitem>
> +    <para>
> +     Tests of client programs under <filename>src/bin</filename>.  See
> +     also <xref linkend="regress-tap">.
> +    </para>
> +   </listitem>
>    </itemizedlist>
>  
>    <para>
> @@ -660,6 +666,28 @@ <title>Variant Comparison Files</title>
>  
>    </sect1>
>  
> +  <sect1 id="regress-tap">
> +   <title>TAP Tests</title>
> +
> +   <para>
> +    The client program tests under <filename>src/bin</filename> use the Perl
> +    TAP tools and are run by <command>prove</command>.  You can pass
> +    command-line options to <command>prove</command> by setting
> +    the <command>make</command> variable <varname>PROVE_FLAGS</>, for example:
> +<programlisting>
> +make -C src/bin check PROVE_FLAGS='--reverse'
> +</programlisting>
> +    The default is <literal>--verbose</literal>.  See the manual page
> +    of <command>prove</command> for more information.
> +   </para>
> +
> +   <para>
> +    The tests written in Perl require the Perl
> +    module <literal>IPC::Run</literal>, otherwise most tests will be skipped.
> +    This module is available from CPAN or an operating system package.
> +   </para>
> +  </sect1>
> +

There's actually also some binaries in /contrib, so maybe phrase this a
bit more generally?

>    <sect1 id="regress-coverage">

>  lcov.info: $(gcda_files)
>      rm -f *.gcov
> -    $(if $^,$(LCOV) -d . -c -o $@ $(LCOVFLAGS))
> +    $(if $^,$(LCOV) -d . -c -o $@ $(LCOVFLAGS) --gcov-tool $(GCOV))

Looks unrelated, but whatever.

> +open HBA, ">>$tempdir/pgdata/pg_hba.conf";
> +print HBA "local replication all trust\n";
> +print HBA "host replication all 127.0.0.1/32 trust\n";
> +print HBA "host replication all ::1/128 trust\n";
> +close HBA;

Given the recent make check security discussions, this doesn't seem like
a good idea...

> +issues_sql_like(['createdb', 'foobar1'], qr/statement: CREATE DATABASE foobar1/, 'SQL CREATE DATABASE run');
> +issues_sql_like(['createdb', 'foobar2', '-l', 'C', '-E', 'LATIN1', '-T', 'template0'], qr/statement: CREATE DATABASE
foobar2ENCODING 'LATIN1'/, 'create database with encoding');
 

Hm. Are all platforms guaranteed to provide latin1?

> diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm
> +if (!$ENV{PGPORT}) {
> +    $ENV{PGPORT} = 65432;
> +}
> +
> +$ENV{PGPORT} = int($ENV{PGPORT}) % 65536;

Hm. I think this should use logical similar to what pg_regress is using,
namely test a few ports.

> +sub start_test_server {
> +    my ($tempdir) = @_;
> +    my $ret;
> +
> +    system "initdb -D $tempdir/pgdata -A trust -N >/dev/null";
> +    $ret = system 'pg_ctl', '-D', "$tempdir/pgdata", '-s', '-w', '-l', "$tempdir/logfile", '-o', "--fsync=off -k
$tempdir--listen-addresses='' --log-statement=all", 'start';
 
> +
> +    if ($ret != 0) {
> +        system('cat', "$tempdir/logfile");
> +        BAIL_OUT("pg_ctl failed");
> +    }
> +
> +    $ENV{PGHOST} = $tempdir;
> +    $test_server_datadir = "$tempdir/pgdata";
> +    $test_server_logfile = "$tempdir/logfile";
> +}

Should stuff like --fsync-off, -k, really be on by default?

I think the code to massage pg_hba.conf should also be here, there'll be
a fair number of tests that need it.


Some questions:
* I haven't looked very careful, but does this set PATH correctly to pick up programs?
* What does installcheck mean for this?
* I think there should be support for contrib modules to use this automatically, without overwriting makefile targets.

Greetings,

Andres Freund

-- Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services



pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: Observed an issue in CREATE TABLE syntax
Next
From: Andreas Karlsson
Date:
Subject: Re: GiST support for inet datatypes