Re: [HACKERS] tests for client programs
Hi, On 2014-06-04 20:40:40 -0400, Peter Eisentraut wrote: On Wed, 2014-05-07 at 03:08 +0200, Andres Freund wrote: As an additional issue it currently doesn't seem to work in VPATH builds. That's imo a must fix. A cd $(srcdir) .. in prove_installcheck and prove_check seems to do the trick. Here is my proposed patch for this. Works here. The tmpdir thing is probably a separate patch... Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] tests for client programs
On Thu, 2014-06-05 at 21:57 -0400, Noah Misch wrote: I recommend TMPDIR = 1 instead of setting DIR. I originally decided against doing that, because 1) I don't know if all systems would have enough space in their regular temporary directory for the kinds of things we put there. Using the build directory seems safer. 2) One debugging method is to set CLEANUP to false and then manually inspect the data directory left behind. (In the future, this might be exposed via a command-line option.) This would become more cumbersome and error-prone if we used TMPDIR. This temporary directory is used for Unix sockets, so path length limitations will be a problem: http://www.postgresql.org/message-id/20140329172224.ga170...@tornado.leadboat.com That, however, is a good argument for doing it the other way. Maybe we need two temporary directories. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] tests for client programs
On Mon, Jun 09, 2014 at 09:12:27PM -0400, Peter Eisentraut wrote: On Thu, 2014-06-05 at 21:57 -0400, Noah Misch wrote: I recommend TMPDIR = 1 instead of setting DIR. I originally decided against doing that, because 1) I don't know if all systems would have enough space in their regular temporary directory for the kinds of things we put there. Using the build directory seems safer. 2) One debugging method is to set CLEANUP to false and then manually inspect the data directory left behind. (In the future, this might be exposed via a command-line option.) This would become more cumbersome and error-prone if we used TMPDIR. This temporary directory is used for Unix sockets, so path length limitations will be a problem: http://www.postgresql.org/message-id/20140329172224.ga170...@tornado.leadboat.com That, however, is a good argument for doing it the other way. Maybe we need two temporary directories. Two temporary directories sounds fair, given those constraints. -- Noah Misch EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] tests for client programs
On 2014-06-04 20:40:40 -0400, Peter Eisentraut wrote:
On Wed, 2014-05-07 at 03:08 +0200, Andres Freund wrote:
As an additional issue it currently doesn't seem to work in VPATH
builds. That's imo a must fix.
A cd $(srcdir) .. in prove_installcheck and prove_check seems to do
the trick.
Here is my proposed patch for this.
Except that I'd rather named CURDIR REGRESSDIR or such this looks sane.
sub tempdir
{
- return File::Temp::tempdir('test', DIR = cwd(), CLEANUP = 1);
+ return File::Temp::tempdir('test', DIR = $ENV{CURDIR} || cwd(),
CLEANUP = 1);
}
Unrelated to this, but for me cleanup doesn't always seem to succeed?
Also could we name the directories tmp_test akin to tmp_check?
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] tests for client programs
On Thu, Jun 05, 2014 at 10:57:03AM +0200, Andres Freund wrote:
On 2014-06-04 20:40:40 -0400, Peter Eisentraut wrote:
On Wed, 2014-05-07 at 03:08 +0200, Andres Freund wrote:
As an additional issue it currently doesn't seem to work in VPATH
builds. That's imo a must fix.
A cd $(srcdir) .. in prove_installcheck and prove_check seems to do
the trick.
Here is my proposed patch for this.
Except that I'd rather named CURDIR REGRESSDIR or such this looks sane.
sub tempdir
{
- return File::Temp::tempdir('test', DIR = cwd(), CLEANUP = 1);
+ return File::Temp::tempdir('test', DIR = $ENV{CURDIR} || cwd(),
CLEANUP = 1);
}
I recommend TMPDIR = 1 instead of setting DIR. This temporary directory is
used for Unix sockets, so path length limitations will be a problem:
http://www.postgresql.org/message-id/20140329172224.ga170...@tornado.leadboat.com
--
Noah Misch
EnterpriseDB http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] tests for client programs
On Wed, 2014-05-07 at 03:08 +0200, Andres Freund wrote:
As an additional issue it currently doesn't seem to work in VPATH
builds. That's imo a must fix.
A cd $(srcdir) .. in prove_installcheck and prove_check seems to do
the trick.
Here is my proposed patch for this.
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index 14119a1..93be859 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -301,13 +301,13 @@ PG_PROVE_FLAGS = --ext='.pl' -I $(top_srcdir)/src/test/perl/
PROVE_FLAGS = --verbose
define prove_installcheck
-PATH=$(bindir):$$PATH PGPORT='6$(DEF_PGPORT)' $(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS)
+cd $(srcdir) CURDIR='$(CURDIR)' PATH=$(bindir):$$PATH PGPORT='6$(DEF_PGPORT)' $(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS)
endef
define prove_check
$(MKDIR_P) tmp_check/log
$(MAKE) -C $(top_builddir) DESTDIR=$(CURDIR)/tmp_check/install install $(CURDIR)/tmp_check/log/install.log 21
-PATH=$(CURDIR)/tmp_check/install$(bindir):$$PATH PGPORT='6$(DEF_PGPORT)' $(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS)
+cd $(srcdir) CURDIR='$(CURDIR)' PATH=$(CURDIR)/tmp_check/install$(bindir):$$PATH PGPORT='6$(DEF_PGPORT)' $(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS)
endef
# Installation.
diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm
index 8a31110..78622fa 100644
--- a/src/test/perl/TestLib.pm
+++ b/src/test/perl/TestLib.pm
@@ -62,7 +62,7 @@ $ENV{PGPORT} = int($ENV{PGPORT}) % 65536;
sub tempdir
{
- return File::Temp::tempdir('test', DIR = cwd(), CLEANUP = 1);
+ return File::Temp::tempdir('test', DIR = $ENV{CURDIR} || cwd(), CLEANUP = 1);
}
my ($test_server_datadir, $test_server_logfile);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] tests for client programs
Peter Eisentraut pete...@gmx.net writes: On Wed, 2014-05-07 at 03:08 +0200, Andres Freund wrote: As an additional issue it currently doesn't seem to work in VPATH builds. That's imo a must fix. A cd $(srcdir) .. in prove_installcheck and prove_check seems to do the trick. Here is my proposed patch for this. BTW, my Salesforce colleagues were complaining to me that this stuff doesn't work at all on older Perl versions; apparently IPC::Run has changed significantly since Perl 5.8 or so. Can we do anything about that? They were also not too happy that the checks get skipped if IPC::Run isn't installed (as it is not on stock RHEL, for instance). It'd be better if we could avoid depending on stuff that isn't in a pretty-vanilla Perl installation. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] tests for client programs
On Wed, 2014-04-30 at 18:09 +0200, Andres Freund wrote:
On 2014-04-04 16:44:46 +0200, Andres Freund wrote:
On 2014-02-27 21:44:48 -0500, Peter Eisentraut wrote:
+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...
The socket file for the test server instance is in a private directory,
so that should be safe enough.
+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 foobar2 ENCODING 'LATIN1'/,
'create database with encoding');
Hm. Are all platforms guaranteed to provide latin1?
Yes.
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.
That could be improved in the future.
+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?
-k is to set the socket directory. You might be thinking of initdb -k?
I think the code to massage pg_hba.conf should also be here, there'll be
a fair number of tests that need it.
More refactoring is always possible as needs arise.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] tests for client programs
On 2014-05-06 20:44:56 -0400, Peter Eisentraut wrote:
On Wed, 2014-04-30 at 18:09 +0200, Andres Freund wrote:
On 2014-04-04 16:44:46 +0200, Andres Freund wrote:
On 2014-02-27 21:44:48 -0500, Peter Eisentraut wrote:
+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...
The socket file for the test server instance is in a private directory,
so that should be safe enough.
Well, you're explicitly configuring host connections... That's why I was
wondering.
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.
That could be improved in the future.
Oddly enough you're overwriting it in Magefile.global.in's prove_check anyway.
Should stuff like --fsync-off, -k, really be on by default?
-k is to set the socket directory. You might be thinking of initdb -k?
Yes, sorry. Confused the line with initdb with the pg_ctl one.
I think the code to massage pg_hba.conf should also be here, there'll be
a fair number of tests that need it.
More refactoring is always possible as needs arise.
I was thinking of
+command_ok(['pg_ctl', 'initdb', '-D', $tempdir/data], 'pg_ctl initdb');
+open CONF, $tempdir/data/postgresql.conf;
+print CONF listen_addresses = ''\n;
+print CONF unix_socket_directories = '$tempdir'\n;
+close CONF;
Sorry, accidentally wrote hba.conf instead of postgresql.conf because I
was thinking about listen_addresses/authentication.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] tests for client programs
On 2014-04-30 18:17:54 +0200, Andres Freund wrote: On 2014-04-30 18:09:15 +0200, Andres Freund wrote: The issues here don't seem to have been addressed in the commit. At least the latin1 thing should be fixed. As an additional issue it currently doesn't seem to work in VPATH builds. That's imo a must fix. A cd $(srcdir) .. in prove_installcheck and prove_check seems to do the trick. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] tests for client programs
On 2014-04-04 16:44:46 +0200, Andres Freund wrote:
On 2014-02-27 21:44:48 -0500, Peter Eisentraut wrote:
+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 foobar2 ENCODING '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.
The issues here don't seem to have been addressed in the commit. At
least the latin1 thing should be fixed.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] tests for client programs
On 2014-04-30 18:09:15 +0200, Andres Freund wrote: The issues here don't seem to have been addressed in the commit. At least the latin1 thing should be fixed. As an additional issue it currently doesn't seem to work in VPATH builds. That's imo a must fix. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] tests for client programs
On 4/4/14, 10:44 AM, Andres Freund wrote: 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? I opine it's committed. ;-) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] tests for client programs
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 @@ titleAdditional Test Suites/title
located in filenamesrc/test/isolation/.
/para
/listitem
+ listitem
+para
+ Tests of client programs under filenamesrc/bin/filename. See
+ also xref linkend=regress-tap.
+/para
+ /listitem
/itemizedlist
para
@@ -660,6 +666,28 @@ titleVariant Comparison Files/title
/sect1
+ sect1 id=regress-tap
+ titleTAP Tests/title
+
+ para
+The client program tests under filenamesrc/bin/filename use the Perl
+TAP tools and are run by commandprove/command. You can pass
+command-line options to commandprove/command by setting
+the commandmake/command variable varnamePROVE_FLAGS/, for
example:
+programlisting
+make -C src/bin check PROVE_FLAGS='--reverse'
+/programlisting
+The default is literal--verbose/literal. See the manual page
+of commandprove/command for more information.
+ /para
+
+ para
+The tests written in Perl require the Perl
+module literalIPC::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 foobar2 ENCODING '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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] tests for client programs
Updated patch. Changes: - added documentation - avoid port conflicts with running instances - added tests for pg_basebackup -T - removed TODO tests for rejected pg_basebackup feature A test on Windows would be nice. Otherwise we'll let the buildfarm do it. From 8205e58442720965c98794cb2f234c46b70dada7 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut pete...@gmx.net Date: Thu, 27 Feb 2014 21:39:35 -0500 Subject: [PATCH v3] Add TAP tests for client programs --- GNUmakefile.in | 4 +- configure | 47 +++ configure.in | 5 + doc/src/sgml/installation.sgml | 3 +- doc/src/sgml/regress.sgml | 28 src/Makefile.global.in | 19 ++- src/bin/initdb/.gitignore | 2 + src/bin/initdb/Makefile| 7 + src/bin/initdb/t/001_initdb.pl | 37 + src/bin/pg_basebackup/.gitignore | 2 + src/bin/pg_basebackup/Makefile | 6 + src/bin/pg_basebackup/t/010_pg_basebackup.pl | 91 src/bin/pg_basebackup/t/020_pg_receivexlog.pl | 8 ++ src/bin/pg_config/.gitignore | 1 + src/bin/pg_config/Makefile | 6 + src/bin/pg_config/t/001_pg_config.pl | 12 ++ src/bin/pg_controldata/.gitignore | 1 + src/bin/pg_controldata/Makefile| 6 + src/bin/pg_controldata/t/001_pg_controldata.pl | 14 ++ src/bin/pg_ctl/.gitignore | 1 + src/bin/pg_ctl/Makefile| 6 + src/bin/pg_ctl/t/001_start_stop.pl | 25 src/bin/pg_ctl/t/002_status.pl | 19 +++ src/bin/scripts/.gitignore | 2 + src/bin/scripts/Makefile | 7 + src/bin/scripts/t/010_clusterdb.pl | 18 +++ src/bin/scripts/t/011_clusterdb_all.pl | 9 ++ src/bin/scripts/t/020_createdb.pl | 16 +++ src/bin/scripts/t/030_createlang.pl| 18 +++ src/bin/scripts/t/040_createuser.pl| 26 src/bin/scripts/t/050_dropdb.pl| 16 +++ src/bin/scripts/t/060_droplang.pl | 15 ++ src/bin/scripts/t/070_dropuser.pl | 16 +++ src/bin/scripts/t/080_pg_isready.pl| 15 ++ src/bin/scripts/t/090_reindexdb.pl | 21 +++ src/bin/scripts/t/091_reindexdb_all.pl | 11 ++ src/bin/scripts/t/100_vacuumdb.pl | 17 +++ src/bin/scripts/t/101_vacuumdb_all.pl | 9 ++ src/test/perl/TestLib.pm | 186 + 39 files changed, 748 insertions(+), 4 deletions(-) create mode 100644 src/bin/initdb/t/001_initdb.pl create mode 100644 src/bin/pg_basebackup/t/010_pg_basebackup.pl create mode 100644 src/bin/pg_basebackup/t/020_pg_receivexlog.pl create mode 100644 src/bin/pg_config/t/001_pg_config.pl create mode 100644 src/bin/pg_controldata/t/001_pg_controldata.pl create mode 100644 src/bin/pg_ctl/t/001_start_stop.pl create mode 100644 src/bin/pg_ctl/t/002_status.pl create mode 100644 src/bin/scripts/t/010_clusterdb.pl create mode 100644 src/bin/scripts/t/011_clusterdb_all.pl create mode 100644 src/bin/scripts/t/020_createdb.pl create mode 100644 src/bin/scripts/t/030_createlang.pl create mode 100644 src/bin/scripts/t/040_createuser.pl create mode 100644 src/bin/scripts/t/050_dropdb.pl create mode 100644 src/bin/scripts/t/060_droplang.pl create mode 100644 src/bin/scripts/t/070_dropuser.pl create mode 100644 src/bin/scripts/t/080_pg_isready.pl create mode 100644 src/bin/scripts/t/090_reindexdb.pl create mode 100644 src/bin/scripts/t/091_reindexdb_all.pl create mode 100644 src/bin/scripts/t/100_vacuumdb.pl create mode 100644 src/bin/scripts/t/101_vacuumdb_all.pl create mode 100644 src/test/perl/TestLib.pm diff --git a/GNUmakefile.in b/GNUmakefile.in index a573880..69e0824 100644 --- a/GNUmakefile.in +++ b/GNUmakefile.in @@ -66,9 +66,9 @@ check check-tests: all check check-tests installcheck installcheck-parallel installcheck-tests: $(MAKE) -C src/test/regress $@ -$(call recurse,check-world,src/test src/pl src/interfaces/ecpg contrib,check) +$(call recurse,check-world,src/test src/pl src/interfaces/ecpg contrib src/bin,check) -$(call recurse,installcheck-world,src/test src/pl src/interfaces/ecpg contrib,installcheck) +$(call recurse,installcheck-world,src/test src/pl src/interfaces/ecpg contrib src/bin,installcheck) GNUmakefile: GNUmakefile.in $(top_builddir)/config.status ./config.status $@ diff --git a/configure b/configure index 122ace7..e0dbdfe 100755 --- a/configure +++ b/configure @@ -627,6 +627,7 @@ ac_includes_default=\ ac_subst_vars='LTLIBOBJS vpath_build +PROVE OSX XSLTPROC COLLATEINDEX @@ -14350,6 +14351,52 @@ fi done +# +# Check for test tools +# +for ac_prog in prove +do + #
Re: [HACKERS] tests for client programs
On Wed, 2014-01-15 at 20:56 +0100, Erik Rijkers wrote:
2 tests stumbled:
1. One test ( pg_ctl/t/001_start_stop.pl ) failed because I had PGDATA set.
I unset all PG+ vars after that. No a big
problem but nonetheless it might be better if the test suite removes
/controls the variables before running.
2. The pg_isready test failed command_fails() ('fails with no server
running') because it defaults to the compiled-in
server-port (and that server was running). I added the test-designated port
(65432, as defined in TestLib.pm). This
simple change is in the attached patch.
With these two changes the whole test suite passed.
Fixed those two things by unsetting environment variables and picking a
different port.
New patch attached.
From 5af8f6849506b993ab7cb3fc8bb167d2f93424fc Mon Sep 17 00:00:00 2001
From: Peter Eisentraut pete...@gmx.net
Date: Sat, 8 Feb 2014 22:05:09 -0500
Subject: [PATCH v2] Add TAP tests for client programs
---
GNUmakefile.in | 4 +-
configure | 47 +++
configure.in | 5 +
src/Makefile.global.in | 19 ++-
src/bin/initdb/.gitignore | 2 +
src/bin/initdb/Makefile| 7 +
src/bin/initdb/t/001_initdb.pl | 37 +
src/bin/pg_basebackup/.gitignore | 2 +
src/bin/pg_basebackup/Makefile | 6 +
src/bin/pg_basebackup/t/010_pg_basebackup.pl | 67 ++
src/bin/pg_basebackup/t/020_pg_receivexlog.pl | 8 ++
src/bin/pg_config/.gitignore | 1 +
src/bin/pg_config/Makefile | 6 +
src/bin/pg_config/t/001_pg_config.pl | 12 ++
src/bin/pg_controldata/.gitignore | 1 +
src/bin/pg_controldata/Makefile| 6 +
src/bin/pg_controldata/t/001_pg_controldata.pl | 14 ++
src/bin/pg_ctl/.gitignore | 1 +
src/bin/pg_ctl/Makefile| 6 +
src/bin/pg_ctl/t/001_start_stop.pl | 21 +++
src/bin/pg_ctl/t/002_status.pl | 14 ++
src/bin/scripts/.gitignore | 2 +
src/bin/scripts/Makefile | 7 +
src/bin/scripts/t/010_clusterdb.pl | 18 +++
src/bin/scripts/t/011_clusterdb_all.pl | 9 ++
src/bin/scripts/t/020_createdb.pl | 16 +++
src/bin/scripts/t/030_createlang.pl| 18 +++
src/bin/scripts/t/040_createuser.pl| 26
src/bin/scripts/t/050_dropdb.pl| 16 +++
src/bin/scripts/t/060_droplang.pl | 15 +++
src/bin/scripts/t/070_dropuser.pl | 16 +++
src/bin/scripts/t/080_pg_isready.pl| 15 +++
src/bin/scripts/t/090_reindexdb.pl | 21 +++
src/bin/scripts/t/091_reindexdb_all.pl | 11 ++
src/bin/scripts/t/100_vacuumdb.pl | 17 +++
src/bin/scripts/t/101_vacuumdb_all.pl | 9 ++
src/test/perl/TestLib.pm | 178 +
37 files changed, 677 insertions(+), 3 deletions(-)
create mode 100644 src/bin/initdb/t/001_initdb.pl
create mode 100644 src/bin/pg_basebackup/t/010_pg_basebackup.pl
create mode 100644 src/bin/pg_basebackup/t/020_pg_receivexlog.pl
create mode 100644 src/bin/pg_config/t/001_pg_config.pl
create mode 100644 src/bin/pg_controldata/t/001_pg_controldata.pl
create mode 100644 src/bin/pg_ctl/t/001_start_stop.pl
create mode 100644 src/bin/pg_ctl/t/002_status.pl
create mode 100644 src/bin/scripts/t/010_clusterdb.pl
create mode 100644 src/bin/scripts/t/011_clusterdb_all.pl
create mode 100644 src/bin/scripts/t/020_createdb.pl
create mode 100644 src/bin/scripts/t/030_createlang.pl
create mode 100644 src/bin/scripts/t/040_createuser.pl
create mode 100644 src/bin/scripts/t/050_dropdb.pl
create mode 100644 src/bin/scripts/t/060_droplang.pl
create mode 100644 src/bin/scripts/t/070_dropuser.pl
create mode 100644 src/bin/scripts/t/080_pg_isready.pl
create mode 100644 src/bin/scripts/t/090_reindexdb.pl
create mode 100644 src/bin/scripts/t/091_reindexdb_all.pl
create mode 100644 src/bin/scripts/t/100_vacuumdb.pl
create mode 100644 src/bin/scripts/t/101_vacuumdb_all.pl
create mode 100644 src/test/perl/TestLib.pm
diff --git a/GNUmakefile.in b/GNUmakefile.in
index 40ab280..3910abb 100644
--- a/GNUmakefile.in
+++ b/GNUmakefile.in
@@ -66,9 +66,9 @@ check check-tests: all
check check-tests installcheck installcheck-parallel installcheck-tests:
$(MAKE) -C src/test/regress $@
-$(call recurse,check-world,src/test src/pl src/interfaces/ecpg contrib,check)
+$(call recurse,check-world,src/test src/pl src/interfaces/ecpg contrib src/bin,check)
-$(call recurse,installcheck-world,src/test src/pl src/interfaces/ecpg contrib,installcheck)
+$(call recurse,installcheck-world,src/test src/pl src/interfaces/ecpg contrib src/bin,installcheck)
GNUmakefile:
[HACKERS] tests for client programs
Hello I am looking on this patch. It is great idea, and I am sure, so we want this patch - it was requested and proposed more time. Some tips: a) possibility to test only selected tests b) possibility to verify generated file against expected file c) detection some warnings (expected/unexpected) p.s. some tests fails when other Postgres is up. It should be checked on start.and raise warning or stop testing. Regards Pavel ok 4 - pg_ctl initdb waiting for server to startLOG: could not bind IPv6 socket: Address already in use HINT: Is another postmaster already running on port 5432? If not, wait a few seconds and retry. LOG: could not bind IPv4 socket: Address already in use HINT: Is another postmaster already running on port 5432? If not, wait a few seconds and retry. WARNING: could not create listen socket for localhost FATAL: could not create any TCP/IP sockets stopped waiting pg_ctl: could not start server Examine the log output. not ok 5 - pg_ctl start -w # Failed test 'pg_ctl start -w' # at /home/pavel/src/postgresql/src/bin/pg_ctl/../../../src/test/perl/TestLib.pm line 89. waiting for server to startLOG: could not bind IPv6 socket: Address already in use HINT: Is another postmaster already running on port 5432? If not, wait a few seconds and retry. LOG: could not bind IPv4 socket: Address already in use HINT: Is another postmaster already running on port 5432? If not, wait a few seconds and retry. WARNING: could not create listen socket for localhost FATAL: could not create any TCP/IP sockets stopped waiting pg_ctl: could not start server Examine the log output. not ok 6 - second pg_ctl start succeeds # Failed test 'second pg_ctl start succeeds' # at /home/pavel/src/postgresql/src/bin/pg_ctl/../../../src/test/perl/TestLib.pm line 89. pg_ctl: PID file /home/pavel/src/postgresql/src/bin/pg_ctl/testf2KW/data/postmaster.pid does not exist Is server running? not ok 7 - pg_ctl stop -w # Failed test 'pg_ctl stop -w' # at /home/pavel/src/postgresql/src/bin/pg_ctl/../../../src/test/perl/TestLib.pm line 89. ok 8 - second pg_ctl stop fails pg_ctl: PID file /home/pavel/src/postgresql/src/bin/pg_ctl/testf2KW/data/postmaster.pid does not exist Is server running? starting server anyway pg_ctl: could not read file /home/pavel/src/postgresql/src/bin/pg_ctl/testf2KW/data/postmaster.opts not ok 9 - pg_ctl restart with server not running # Failed test 'pg_ctl restart with server not running' # at /home/pavel/src/postgresql/src/bin/pg_ctl/../../../src/test/perl/TestLib.pm line 89. pg_ctl: PID file /home/pavel/src/postgresql/src/bin/pg_ctl/testf2KW/data/postmaster.pid does not exist Is server running? starting server anyway pg_ctl: could not read file /home/pavel/src/postgresql/src/bin/pg_ctl/testf2KW/data/postmaster.opts not ok 10 - pg_ctl restart with server running # Failed test 'pg_ctl restart with server running' # at /home/pavel/src/postgresql/src/bin/pg_ctl/../../../src/test/perl/TestLib.pm line 89. pg_ctl: PID file /home/pavel/src/postgresql/src/bin/pg_ctl/testf2KW/data/postmaster.pid does not exist Is server running? Bailout called. Further testing stopped: system pg_ctl stop -D /home/pavel/src/postgresql/src/bin/pg_ctl/testf2KW/data -m fast failed: 256 Bail out! system pg_ctl stop -D /home/pavel/src/postgresql/src/bin/pg_ctl/testf2KW/data -m fast failed: 256 FAILED--Further testing stopped: system pg_ctl stop -D /home/pavel/src/postgresql/src/bin/pg_ctl/testf2KW/data -m fast failed: 256 make[1]: *** [check] Error 255 make[1]: Leaving directory `/home/pavel/src/postgresql/src/bin/pg_ctl' make: *** [check-pg_ctl-recurse] Error 2 make: Leaving directory `/home/pavel/src/postgresql/src/bin'
Re: [HACKERS] tests for client programs
On 1/15/14, 1:46 AM, Erik Rijkers wrote: The seems to be a dependency on IPC::Run I can install that, of course... but I suppose you want to make this work without that. No, IPC::Run will be required. It looked like it was part of the default installation where I tested, but apparently not. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] tests for client programs
On Wed, January 15, 2014 06:30, Peter Eisentraut wrote:
As we all know, the client programs (src/bin/) don't have any real test
So I wrote something.
I chose to use Perl-based tools, prove and Test::More, because those are
[ 0001-Add-TAP-tests-for-client-programs.patch ] 32 k
I gave this a quick try.
Centos 6.5 final / perl 5.18.2
As mentioned earlier I had to install IPC::Run.
2 tests stumbled:
1. One test ( pg_ctl/t/001_start_stop.pl ) failed because I had PGDATA set.
I unset all PG+ vars after that. No a big
problem but nonetheless it might be better if the test suite removes /controls
the variables before running.
2. The pg_isready test failed command_fails() ('fails with no server running')
because it defaults to the compiled-in
server-port (and that server was running). I added the test-designated port
(65432, as defined in TestLib.pm). This
simple change is in the attached patch.
With these two changes the whole test suite passed.
Thanks,
Erik Rijkers
--- src/bin/scripts/t/080_pg_isready.pl.orig 2014-01-15 20:08:16.325916223 +0100
+++ src/bin/scripts/t/080_pg_isready.pl 2014-01-15 20:18:24.705927054 +0100
@@ -7,7 +7,7 @@
program_version_ok('pg_isready');
program_options_handling_ok('pg_isready');
-command_fails(['pg_isready'], 'fails with no server running');
+command_fails(['pg_isready', '-p65432'], 'fails with no server running');
my $tempdir = tempdir;
start_test_server $tempdir;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] tests for client programs
As we all know, the client programs (src/bin/) don't have any real test
suites. Some pieces are tested as part of the backend regression tests,
some as part of the pg_upgrade test script, but nothing is specifically
targeted, and pg_basebackup for example is not tested at all.
So I wrote something.
I chose to use Perl-based tools, prove and Test::More, because those are
available in a standard Perl installation, and we already require that.
I put together three handfuls of tests to show what it would look like.
For extra fun, I added a todo test in pg_basebackup for a feature
that's currently being proposed in the commit fest.
A significant near-future project would be adding tests for pg_dump and
pg_upgrade.
Lots of things to argue about here: tools, file layout, naming, Perl
code, test quality, etc.
To try it out, apply the attached patch and run make -C src/bin check
(or installcheck, after installation).
From 9805e2dc70b8c6174537423001c62c529da2c335 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut pete...@gmx.net
Date: Tue, 14 Jan 2014 21:51:22 -0500
Subject: [PATCH] Add TAP tests for client programs
---
configure | 47 +++
configure.in | 5 +
src/Makefile.global.in | 17 ++-
src/bin/initdb/.gitignore | 2 +
src/bin/initdb/Makefile| 7 ++
src/bin/initdb/t/001_initdb.pl | 37 ++
src/bin/pg_basebackup/.gitignore | 2 +
src/bin/pg_basebackup/Makefile | 6 +
src/bin/pg_basebackup/t/010_pg_basebackup.pl | 67 ++
src/bin/pg_basebackup/t/020_pg_receivexlog.pl | 8 ++
src/bin/pg_config/.gitignore | 1 +
src/bin/pg_config/Makefile | 6 +
src/bin/pg_config/t/001_pg_config.pl | 12 ++
src/bin/pg_controldata/.gitignore | 1 +
src/bin/pg_controldata/Makefile| 6 +
src/bin/pg_controldata/t/001_pg_controldata.pl | 14 +++
src/bin/pg_ctl/.gitignore | 1 +
src/bin/pg_ctl/Makefile| 6 +
src/bin/pg_ctl/t/001_start_stop.pl | 21
src/bin/pg_ctl/t/002_status.pl | 14 +++
src/bin/scripts/.gitignore | 2 +
src/bin/scripts/Makefile | 7 ++
src/bin/scripts/t/010_clusterdb.pl | 18 +++
src/bin/scripts/t/011_clusterdb_all.pl | 9 ++
src/bin/scripts/t/020_createdb.pl | 16 +++
src/bin/scripts/t/030_createlang.pl| 18 +++
src/bin/scripts/t/040_createuser.pl| 26
src/bin/scripts/t/050_dropdb.pl| 16 +++
src/bin/scripts/t/060_droplang.pl | 15 +++
src/bin/scripts/t/070_dropuser.pl | 16 +++
src/bin/scripts/t/080_pg_isready.pl| 15 +++
src/bin/scripts/t/090_reindexdb.pl | 21
src/bin/scripts/t/091_reindexdb_all.pl | 11 ++
src/bin/scripts/t/100_vacuumdb.pl | 17 +++
src/bin/scripts/t/101_vacuumdb_all.pl | 9 ++
src/test/perl/TestLib.pm | 164 +
36 files changed, 659 insertions(+), 1 deletion(-)
create mode 100644 src/bin/initdb/t/001_initdb.pl
create mode 100644 src/bin/pg_basebackup/t/010_pg_basebackup.pl
create mode 100644 src/bin/pg_basebackup/t/020_pg_receivexlog.pl
create mode 100644 src/bin/pg_config/t/001_pg_config.pl
create mode 100644 src/bin/pg_controldata/t/001_pg_controldata.pl
create mode 100644 src/bin/pg_ctl/t/001_start_stop.pl
create mode 100644 src/bin/pg_ctl/t/002_status.pl
create mode 100644 src/bin/scripts/t/010_clusterdb.pl
create mode 100644 src/bin/scripts/t/011_clusterdb_all.pl
create mode 100644 src/bin/scripts/t/020_createdb.pl
create mode 100644 src/bin/scripts/t/030_createlang.pl
create mode 100644 src/bin/scripts/t/040_createuser.pl
create mode 100644 src/bin/scripts/t/050_dropdb.pl
create mode 100644 src/bin/scripts/t/060_droplang.pl
create mode 100644 src/bin/scripts/t/070_dropuser.pl
create mode 100644 src/bin/scripts/t/080_pg_isready.pl
create mode 100644 src/bin/scripts/t/090_reindexdb.pl
create mode 100644 src/bin/scripts/t/091_reindexdb_all.pl
create mode 100644 src/bin/scripts/t/100_vacuumdb.pl
create mode 100644 src/bin/scripts/t/101_vacuumdb_all.pl
create mode 100644 src/test/perl/TestLib.pm
diff --git a/configure b/configure
index 8760643..9a9c729 100755
--- a/configure
+++ b/configure
@@ -627,6 +627,7 @@ ac_includes_default=\
ac_subst_vars='LTLIBOBJS
vpath_build
+PROVE
OSX
XSLTPROC
COLLATEINDEX
@@ -14614,6 +14615,52 @@ fi
done
+#
+# Check for test tools
+#
+for ac_prog in prove
+do
+ # Extract the first word of $ac_prog, so it can be a program name with args.
+set dummy $ac_prog; ac_word=$2
+{ $as_echo $as_me:${as_lineno-$LINENO}: checking for $ac_word 5
+$as_echo_n checking for $ac_word... 6; }
+if
Re: [HACKERS] tests for client programs
On Wed, January 15, 2014 06:30, Peter Eisentraut wrote: As we all know, the client programs (src/bin/) don't have any real test suites. Some pieces are tested as part of the backend regression tests, some as part of the pg_upgrade test script, but nothing is specifically targeted, and pg_basebackup for example is not tested at all. So I wrote something. I chose to use Perl-based tools, prove and Test::More, because those are available in a standard Perl installation, and we already require that. [ 0001-Add-TAP-tests-for-client-programs.patch ] With perl 5.18.2, in centos 6.5; system provided perl 5.10 has the same problem. The seems to be a dependency on IPC::Run I can install that, of course... but I suppose you want to make this work without that. Thanks, Erik Rijkers -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
