Re: Writing new unit tests with PostgresNode

On Mon, Feb 22, 2016 at 2:19 PM, Craig Ringer <craig(at)2ndquadrant(dot)com> wrote:
> I've been taking a look at the Perl test infrastructure ( src/test/perl )
> for writing multi-node tests, starting with PostgresNode.pm and I have a few
> comments based on my first approach to the code "cold".
>
> I think a README in src/test/perl/ would be very helpful. It's currently not
> at all clear how to go about using the new test infrastructure when first
> looking at it, particularly adding new test suites. Also a summary of
> available test facilities and some short examples or pointers to where they
> can be found ( src/bin/, src/test/ssl, etc).

+1. I will be happy to contribute into that.

> The docs at http://www.postgresql.org/docs/devel/static/regress-tap.html
> mention the use of tap for client programs but don't have anything on
> writing/using the framework. It doesn't seem to be very close to /
> compatible with http://pgtap.org/ unless I'm missing something obvious.
>
> I want to add tests for failover slots but I'm not sure where to put them or
> how to set up the test skeleton. So I went looking, and I could use
> confirmation that the below is roughly right so I can write it up for some
> quickstart docs.

src/test/modules

> It *looks* like one needs to create a Makefile with:
>
> subdir = src/test/mytests
> top_builddir = ../../..
> include $(top_builddir)/src/Makefile.global
>
> check:
> $(prove_check)
>
> clean:
> rm -rf ./tmp_check

Yep.

> Then create a subdir src/test/mytests/t and in it put files named
> 001_sometest.pl etc. Each of which should import PostgresNode and generally
> then create new instances indirectly via the PostgresNode::get_new_node
> function rather than by using the constructor. Then init and start the
> server and run tests. Like:

A couple of other things that I am aware of:
- including strict and warnings is as well mandatory to ensure that
test grammar is sane, and the tests written should have no warnings.
- All the core routines used should be compatible down to perl 5.8.8.
- The code produced should be sanitized with perltidy before commit.

> use strict;
> use warnings;
> use PostgresNode;
> use TestLib;
> use Test::More tests => 1;
>
> diag 'Setting up node';
> my $node = get_new_node('master');
> $node->init;
> $node->start;
>
> my $ret = $node->psql('postgres', 'SELECT 1;');
> is($ret, '1', 'simple SELECT');
>
> $node->teardown_node;

Teardown is not mandatory at the end, though I would recommend having
it, PostgresNode::DESTROY enforcing the nodes to stop at the end of a
test, and the temporary paths are unconditionally removed (those
should not be removed in case of a failure honestly).

> Knowledge of perl's Test::More is required since most of the suite is built
> on it.
>
> The suite should be added to src/test/Makefile's ALWAYS_SUBDIRS entry.

SUBDIRS is more suited for tests that do not represent a security
risk. The ssl suite is part of ALWAYS_SUBDIRS because it enforces the
use of 127.0.0.1.

> Sound about right? I can tidy that up a bit and turn it into a README and
> add a reference to that to the public tap docs to tell users where to go if
> they want to write more tests.

Yes, please.

> I don't know how many suites we'll want - whether it'll be desirable to have
> a few suites with lots of tests or to have lots of suites with just a few
> tests. I'm planning on starting by adding a suite named 'replication' and
> putting some tests for failover slots in there. Reasonable?

It seems to me that the failover slot tests should be part of the
recovery test suite I have proposed already. Those are located in
src/test/recovery, introducing as well a set of routines in
PostgresNode.pm that allows one to pass options to PostgresNode::init
to enable archive or streaming on a given node. I would believe that
any replication suite is going to need that, and I have done a bunch
of legwork to make sure that this works on Windows as well.

> (BTW, the ssl tests don't seem to use a bunch of the facilities provided by
> PostgresNode, instead rolling their own, so they don't serve as a good
> example.)

Yep.
--
Michael