On 23 February 2016 at 09:52, Michael Paquier <michael.paqu...@gmail.com>
wrote:

> On Tue, Feb 23, 2016 at 12:29 AM, Alvaro Herrera
> <alvhe...@2ndquadrant.com> wrote:
> > Craig Ringer wrote:
> >
> >> > +=pod
> >> > +
> >> > +=head2 Set up a node
> >> > pod format... Do we really want that? Considering that those modules
> >> > are only aimed at being dedicated for in-core testing, I would say no.
> >>
> >> If it's plain comments you have to scan through massive piles of verbose
> >> Perl to find what you want. If it's pod you can just perldoc
> >> /path/to/module it and get a nice summary of the functions etc.
> >>
> >> If these are intended to become usable facilities for people to write
> tests
> >> with then I think it's important that the docs be reasonably accessible.
> >
> > Yes, I think adding POD here is a good idea.  I considered doing it
> > myself back when I was messing with PostgresNode ...
>
> OK, withdrawal from here. If there are patches to add that to the
> existing tests, I'll review them, and rebase what I have depending on
> what gets in first. Could a proper patch split be done please?
>

Just finished doing that. Thanks for taking a look at the first patch so
quickly. I hope this one's better.

FWIW I think you were right that using pod for the actual test wasn't the
best choice, I should've just used comments. I do think it's important for
the modules to have structured docs.

I've removed the example suite in favour of adding a SYNOPSIS section to
PostgresNode.pm and describing the rest in the README. It won't be
necessary once your replication tests go in, they'll be a perfectly
adequate example.

I also cut out the changes to the backup method; I'll send a pull to add to
your proposed replication patch instead.

-- 
 Craig Ringer                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services
From 99d01a48aa9e4a131767e8565e6a678c54a7555a Mon Sep 17 00:00:00 2001
From: Craig Ringer <cr...@2ndquadrant.com>
Date: Tue, 23 Feb 2016 20:37:57 +0800
Subject: [PATCH 1/3] Document where to find PostgreSQL's tests

---
 src/test/README         | 40 ++++++++++++++++++++++++++++++++++++++++
 src/test/modules/README | 13 +++++++++++++
 2 files changed, 53 insertions(+)
 create mode 100644 src/test/README
 create mode 100644 src/test/modules/README

diff --git a/src/test/README b/src/test/README
new file mode 100644
index 0000000..7ed0706
--- /dev/null
+++ b/src/test/README
@@ -0,0 +1,40 @@
+PostgreSQL tests
+================
+
+This directory contains a variety of test infrastructure as well as some of the
+tests in PostgreSQL. Not all tests are here - in particular, there are more in
+individual contrib/ modules and in src/bin.
+
+Not all these tests get run by "make check". Check src/test/Makefile to see
+which tests get run automatically.
+
+examples/
+  demo programs for libpq that double as regression tests via "make check"
+
+isolation/
+  tests for concurrent behaviours at the SQL level
+
+locale/
+  sanity checks for locale data, encodings, etc
+
+mb/
+  tests for multibyte encoding (UTF-8) support
+
+modules/
+  extensions used only or mainly for test purposes, generally not useful
+  or suitable for installing in production databases. Some of these have
+  their own tests, some are used by tests elsewhere.
+
+perl/
+  infrastructure for Perl-based Test::More tests. There are no actual tests
+  here, the code is used by other tests in src/bin/, contrib/ and in the
+  subdirectories of src/test.
+
+regress/
+  PostgreSQL's main regression test suite, pg_regress
+
+ssl/
+  Tests to exercise and verify SSL certificate handling
+
+thread/
+  A thread-safety-testing utility used by configure, see its README
diff --git a/src/test/modules/README b/src/test/modules/README
new file mode 100644
index 0000000..42c5d38
--- /dev/null
+++ b/src/test/modules/README
@@ -0,0 +1,13 @@
+Test extensions and libraries
+=============================
+
+src/test/modules contains PostgreSQL extensions that are primarily or entirely
+intended for testing PostgreSQL and/or to serve as example code. The extensions
+here aren't intended to be installed in a production server and aren't useful
+for "real work".
+
+Most extensions have their own pg_regress tests. Some are also used by tests
+elsewhere in the test tree.
+
+If you're adding new hooks or other functionality exposed as C-level API this
+is where to add the tests for it.
-- 
2.1.0

From 2e89c8514a0b380d43f3feb1beab0e5cbec3b283 Mon Sep 17 00:00:00 2001
From: Craig Ringer <cr...@2ndquadrant.com>
Date: Tue, 23 Feb 2016 21:34:30 +0800
Subject: [PATCH 2/3] Add a README for the perl tests and some perldoc

---
 src/test/perl/PostgresNode.pm | 399 +++++++++++++++++++++++++++++++++++++++---
 src/test/perl/README          |  77 ++++++++
 2 files changed, 447 insertions(+), 29 deletions(-)
 create mode 100644 src/test/perl/README

diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
index 2ab9aee..a995d28 100644
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
@@ -1,8 +1,62 @@
-# PostgresNode, class representing a data directory and postmaster.
-#
-# This contains a basic set of routines able to work on a PostgreSQL node,
-# allowing to start, stop, backup and initialize it with various options.
-# The set of nodes managed by a given test is also managed by this module.
+=pod
+
+=head1 NAME
+
+PostgresNode - class representing a data directory and postmaster
+
+=head1 SYNOPSIS
+
+  use PostgresNode;
+
+  my $node = get_new_node('mynode');
+
+  # Create a data directory with initdb
+  # like: initdb ...
+  $node->init();
+
+  # Start PostgreSQL
+  # like: pg_ctl -w start ...
+  $node->start();
+
+  # Change a setting and restart
+  $node->append_conf('postgresql.conf', 'hot_standby = on');
+  $node->restart('fast');
+
+  # run a query with psql
+  # like: psql -qAXt postgres -c 'SELECT 1;'
+  $psql_stdout = $node->psql('postgres', 'SELECT 1');
+
+  # run query every second until it returns 't'
+  # or times out
+  $node->poll_query_until('postgres', q|SELECT random() < 0.1;|')
+    or print "timed out";
+
+  # Do an online pg_basebackup
+  my $ret = $node->backup('testbackup');
+
+  # Restore it to create a new independent node (not a replica)
+  my $replica = get_new_node('replica');
+  $replica->init_from_backup($node, 'testbackup');
+  $replica->start;
+
+  # Stop the server
+  # like: pg_ctl stop -m fast
+  $node->stop('fast');
+
+=head1 DESCRIPTION
+
+PostgresNode contains a set of routines able to work on a PostgreSQL node,
+allowing to start, stop, backup and initialize it with various options.
+The set of nodes managed by a given test is also managed by this module.
+
+It requires IPC::Run - install libperl-ipc-run (Debian/Ubuntu) or perl-IPC-Run
+(Fedora/RHEL).
+
+In addition to node management, PostgresNode instances have some wrappers
+around Test::More functions to run commands with an envronment set up to
+point to the instance.
+
+=cut
 
 package PostgresNode;
 
@@ -40,6 +94,21 @@ INIT
 	$last_port_assigned = int(rand() * 16384) + 49152;
 }
 
+=pod
+
+=head1 METHODS
+
+=over
+
+=item PostgresNode::new($class, $name, $pghost, $pgport)
+
+Create a new PostgresNode instance. Does not initdb or start it.
+
+You should generally prefer to use get_new_node() instead since it takes care
+of finding port numbers, registering instances for cleanup, etc.
+
+=cut
+
 sub new
 {
 	my $class  = shift;
@@ -61,36 +130,91 @@ sub new
 	return $self;
 }
 
+=pod
+
+=item $node->port()
+
+Get the port number assigned to the host. This won't necessarily be a TCP port
+open on the local host since we prefer to use unix sockets if possible.
+
+Use $node->connstr() if you want a connection string.
+
+=cut
+
 sub port
 {
 	my ($self) = @_;
 	return $self->{_port};
 }
 
+=pod
+
+=item $node->host()
+
+Return the host (like PGHOST) for this instance. May be a UNIX socket path.
+
+Use $node->connstr() if you want a connection string.
+
+=cut
+
 sub host
 {
 	my ($self) = @_;
 	return $self->{_host};
 }
 
+=pod
+
+=item $node->basedir()
+
+The directory all the node's files will be within - datadir, archive directory,
+backups, etc.
+
+=cut
+
 sub basedir
 {
 	my ($self) = @_;
 	return $self->{_basedir};
 }
 
+=pod
+
+=item $node->name()
+
+The name assigned to the node at creation time.
+
+=cut
+
 sub name
 {
 	my ($self) = @_;
 	return $self->{_name};
 }
 
+=pod
+
+=item $node->logfile()
+
+Path to the PostgreSQL log file for this instance.
+
+=cut
+
 sub logfile
 {
 	my ($self) = @_;
 	return $self->{_logfile};
 }
 
+=pod
+
+=item $node->connstr()
+
+Get a libpq connection string that will establish a connection to
+this node. Suitable for passing to psql, DBD::Pg, etc.
+
+=cut
+
 sub connstr
 {
 	my ($self, $dbname) = @_;
@@ -103,6 +227,15 @@ sub connstr
 	return "port=$pgport host=$pghost dbname=$dbname";
 }
 
+=pod
+
+=item $node->data_dir()
+
+Returns the path to the data directory. postgresql.conf and pg_hba.conf are
+always here.
+
+=cut
+
 sub data_dir
 {
 	my ($self) = @_;
@@ -110,6 +243,14 @@ sub data_dir
 	return "$res/pgdata";
 }
 
+=pod
+
+=item $node->archive_dir()
+
+If archiving is enabled, WAL files go here.
+
+=cut
+
 sub archive_dir
 {
 	my ($self) = @_;
@@ -117,6 +258,14 @@ sub archive_dir
 	return "$basedir/archives";
 }
 
+=pod
+
+=item $node->backup_dir()
+
+The output path for backups taken with $node->backup()
+
+=cut
+
 sub backup_dir
 {
 	my ($self) = @_;
@@ -124,23 +273,55 @@ sub backup_dir
 	return "$basedir/backup";
 }
 
-# Dump node information
+=pod
+
+=item $node->info()
+
+Return a string containing human-readable diagnostic information (paths, etc)
+about this node.
+
+=cut
+
+sub info
+{
+	my ($self) = @_;
+	my $_info = '';
+	open my $fh, '>', \$_info or die;
+	print $fh "Name: " . $self->name . "\n";
+	print $fh "Data directory: " . $self->data_dir . "\n";
+	print $fh "Backup directory: " . $self->backup_dir . "\n";
+	print $fh "Archive directory: " . $self->archive_dir . "\n";
+	print $fh "Connection string: " . $self->connstr . "\n";
+	print $fh "Log file: " . $self->logfile . "\n";
+	close $fh or die;
+	return $_info;
+}
+
+=pod
+
+=item $node->dump_info()
+
+Print $node->info()
+
+=cut
+
 sub dump_info
 {
 	my ($self) = @_;
-	print "Name: " . $self->name . "\n";
-	print "Data directory: " . $self->data_dir . "\n";
-	print "Backup directory: " . $self->backup_dir . "\n";
-	print "Archive directory: " . $self->archive_dir . "\n";
-	print "Connection string: " . $self->connstr . "\n";
-	print "Log file: " . $self->logfile . "\n";
+	print $self->info;
 }
 
+
+# Internal method to set up trusted pg_hba.conf for replication.  Not
+# documented because you shouldn't use it, it's called automatically if needed.
 sub set_replication_conf
 {
 	my ($self) = @_;
 	my $pgdata = $self->data_dir;
 
+	$self->host eq $test_pghost
+	  or die "set_replication_conf only works with the default host";
+
 	open my $hba, ">>$pgdata/pg_hba.conf";
 	print $hba "\n# Allow replication (set up by PostgresNode.pm)\n";
 	if (!$TestLib::windows_os)
@@ -155,13 +336,26 @@ sub set_replication_conf
 	close $hba;
 }
 
-# Initialize a new cluster for testing.
-#
-# Authentication is set up so that only the current OS user can access the
-# cluster. On Unix, we use Unix domain socket connections, with the socket in
-# a directory that's only accessible to the current user to ensure that.
-# On Windows, we use SSPI authentication to ensure the same (by pg_regress
-# --config-auth).
+=pod
+
+=item $node->init(...)
+
+Initialize a new cluster for testing.
+
+Authentication is set up so that only the current OS user can access the
+cluster. On Unix, we use Unix domain socket connections, with the socket in
+a directory that's only accessible to the current user to ensure that.
+On Windows, we use SSPI authentication to ensure the same (by pg_regress
+--config-auth).
+
+pg_hba.conf is configured to allow replication connections. Pass the keyword
+parameter hba_permit_replication => 0 to disable this.
+
+The new node is set up in a fast but unsafe configuration where fsync is
+disabled.
+
+=cut
+
 sub init
 {
 	my ($self, %params) = @_;
@@ -197,6 +391,19 @@ sub init
 	$self->set_replication_conf if ($params{hba_permit_replication});
 }
 
+=pod
+
+=item $node->append_conf(filename, str)
+
+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.
+
+=cut
+
 sub append_conf
 {
 	my ($self, $filename, $str) = @_;
@@ -206,6 +413,19 @@ sub append_conf
 	TestLib::append_to_file($conffile, $str);
 }
 
+=pod
+
+=item $node->backup(backup_name)
+
+Create a hot backup with pg_basebackup in $node->backup_dir,
+including the transaction logs. xlogs are fetched at the
+end of the backup, not streamed.
+
+You'll have to configure a suitable max_wal_senders on the
+target server since it isn't done by default.
+
+=cut
+
 sub backup
 {
 	my ($self, $backup_name) = @_;
@@ -218,6 +438,23 @@ sub backup
 	print "# Backup finished\n";
 }
 
+=pod
+
+=item $node->init_from_backup(root_node, backup_name)
+
+Initialize a node from a backup, which may come from this node or a different
+node. root_node must be a PostgresNode reference, backup_name the string name
+of a backup previously created on that node with $node->backup.
+
+Does not start the node after init.
+
+A recovery.conf is not created.
+
+The backup is copied, leaving the original unmodified. pg_hba.conf is
+unconditionally set to enable replication connections.
+
+=cut
+
 sub init_from_backup
 {
 	my ($self, $root_node, $backup_name) = @_;
@@ -248,6 +485,16 @@ port = $port
 	$self->set_replication_conf;
 }
 
+=pod
+
+=item $node->start()
+
+Wrapper for pg_ctl -w start
+
+Start the node and wait until it is ready to accept connections.
+
+=cut
+
 sub start
 {
 	my ($self) = @_;
@@ -268,6 +515,14 @@ sub start
 	$self->_update_pid;
 }
 
+=pod
+
+=item $node->stop(mode)
+
+Stop the node using pg_ctl -m $mode and wait for it to stop.
+
+=cut
+
 sub stop
 {
 	my ($self, $mode) = @_;
@@ -281,6 +536,14 @@ sub stop
 	$self->_update_pid;
 }
 
+=pod
+
+=item $node->restart()
+
+wrapper for pg_ctl -w restart
+
+=cut
+
 sub restart
 {
 	my ($self)  = @_;
@@ -294,6 +557,8 @@ sub restart
 	$self->_update_pid;
 }
 
+
+# Internal method
 sub _update_pid
 {
 	my $self = shift;
@@ -314,14 +579,20 @@ sub _update_pid
 	print "# No postmaster PID\n";
 }
 
-#
-# Cluster management functions
-#
+=pod
+
+=item get_new_node(node_name)
+
+Build a new PostgresNode object, assigning a free port number. Standalone
+function that's automatically imported.
+
+We also register the node, to avoid the port number from being reused
+for another node even when this one is not active.
+
+You should generally use this instead of PostgresNode::new(...).
+
+=cut
 
-# Build a new PostgresNode object, assigning a free port number.
-#
-# We also register the node, to avoid the port number from being reused
-# for another node even when this one is not active.
 sub get_new_node
 {
 	my $name  = shift;
@@ -360,6 +631,7 @@ sub get_new_node
 	return $node;
 }
 
+# Attempt automatic cleanup
 sub DESTROY
 {
 	my $self = shift;
@@ -369,6 +641,14 @@ sub DESTROY
 	TestLib::system_log('pg_ctl', 'kill', 'QUIT', $self->{_pid});
 }
 
+=pod
+
+=item $node->teardown_node()
+
+Do an immediate stop of the node
+
+=cut
+
 sub teardown_node
 {
 	my $self = shift;
@@ -376,6 +656,18 @@ sub teardown_node
 	$self->stop('immediate');
 }
 
+=pod
+
+=item $node->psql(dbname, sql)
+
+Run a query with psql and return stdout, or on error print stderr.
+
+Executes a query/script with psql and returns psql's standard output.  psql is
+run in unaligned tuples-only quiet mode with psqlrc disabled so simple queries
+will just return the result row(s) with fields separated by commas.
+
+=cut
+
 sub psql
 {
 	my ($self, $dbname, $sql) = @_;
@@ -399,7 +691,15 @@ sub psql
 	return $stdout;
 }
 
-# Run a query once a second, until it returns 't' (i.e. SQL boolean true).
+=pod
+
+=item $node->poll_query_until(dbname, query)
+
+Run a query once a second, until it returns 't' (i.e. SQL boolean true).
+Continues polling if psql returns an error result. Times out after 90 seconds.
+
+=cut
+
 sub poll_query_until
 {
 	my ($self, $dbname, $query) = @_;
@@ -432,6 +732,16 @@ sub poll_query_until
 	return 0;
 }
 
+=pod
+
+=item $node->command_ok(...)
+
+Runs a shell command like TestLib::command_ok, but with PGPORT
+set so that the command will default to connecting to this
+PostgresNode.
+
+=cut
+
 sub command_ok
 {
 	my $self = shift;
@@ -441,6 +751,14 @@ sub command_ok
 	TestLib::command_ok(@_);
 }
 
+=pod
+
+=item $node->command_fails(...) - TestLib::command_fails with our PGPORT
+
+See command_ok(...)
+
+=cut
+
 sub command_fails
 {
 	my $self = shift;
@@ -450,6 +768,14 @@ sub command_fails
 	TestLib::command_fails(@_);
 }
 
+=pod
+
+=item $node->command_like(...)
+
+TestLib::command_like with our PGPORT. See command_ok(...)
+
+=cut
+
 sub command_like
 {
 	my $self = shift;
@@ -459,8 +785,17 @@ sub command_like
 	TestLib::command_like(@_);
 }
 
-# Run a command on the node, then verify that $expected_sql appears in the
-# server log file.
+=pod
+
+=item $node->issues_sql_like(cmd, expected_sql, test_name)
+
+Run a command on the node, then verify that $expected_sql appears in the
+server log file.
+
+Reads the whole log file so be careful when working with large log outputs.
+
+=cut
+
 sub issues_sql_like
 {
 	my ($self, $cmd, $expected_sql, $test_name) = @_;
@@ -474,4 +809,10 @@ sub issues_sql_like
 	like($log, $expected_sql, "$test_name: SQL found in server log");
 }
 
+=pod
+
+=back
+
+=cut
+
 1;
diff --git a/src/test/perl/README b/src/test/perl/README
new file mode 100644
index 0000000..4c8f10c
--- /dev/null
+++ b/src/test/perl/README
@@ -0,0 +1,77 @@
+Perl-based (Test::More) tests
+=============================
+
+src/test/perl/ contains shared infrastructure that's used by Perl-based tests
+across the source tree, particularly tests in src/bin and src/test. It's used
+to drive tests for backup and restore, replication, etc - anything that can't
+really be expressed using pg_regress or the isolation test framework.
+
+You should prefer to write tests using pg_regress in src/test/regress, or
+isolation tester specs in src/test/isolation, if possible. If not, check to see
+if your new tests make sense under an existing tree in src/test, like
+src/test/ssl, or should be added to one of the suites for an existing utility
+
+Writing tests
+-------------
+
+Tests are written using Perl's Test::More with some PostgreSQL-specific
+infrastructure from src/test/perl providing node management, support for
+invoking 'psql' to run queries and get results, etc. You should read the
+documentation for Test::More before trying to write tests.
+
+Test scripts in the t/ directory of a suite are executed in order.
+
+Each test script should begin with:
+
+    use strict;
+    use warnings;
+    use PostgresNode;
+    use TestLib;
+    # Replace with the number of tests to execute:
+    use Test::More tests => 1;
+
+then it will generally need to set up one or more nodes, run commands
+against them and evaluate the results. For example:
+
+    my $node = get_new_node('master');
+    $node->init;
+    $node->start;
+
+    my $ret = $node->psql('postgres', 'SELECT 1');
+    is($ret, '1', 'SELECT 1 returns 1');
+
+    $node->stop('fast');
+
+Read the Test::More documentation for more on how to write tests:
+
+    perldoc Test::More
+
+For available PostgreSQL-specific test methods and some example tests read the
+perldoc for the test modules, e.g.:
+
+    perldoc src/test/perl/PostgresNode.pm
+
+Adding a suite
+--------------
+
+If your tests don't make sense under any of the existing test suites you can
+add a new suite in src/test/ . Try to avoid creating lots of new
+test suites - it's better to add new tests to an existing suite if they can
+reasonably fit there.
+
+Create a Makefile in the new directory with the usual boilerplate and a
+'check' rule:
+
+    subdir = src/test/my_suite
+    top_builddir = ../../..
+    include $(top_builddir)/src/Makefile.global
+
+    check:
+	    $(prove_check)
+
+    clean:
+	    rm -rf ./tmp_check
+
+Then create a t/ subdirectory and start adding tests.
+
+You should add the new suite to src/test/Makefile . See the comments there.
-- 
2.1.0

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to