On 22 February 2016 at 14:30, Michael Paquier <michael.paqu...@gmail.com>
wrote:


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

Thanks. Hopefully what I wrote will be useful as a starting point.

As I read through the modules I'll write some draft Perldoc too.


> > 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.
>
> src/test/modules
>

Could use a README to explain what the directory is for.

It looks like it contains a bunch of the extensions that used to be in
contrib/ . So that'd be where I'd want to put any extension needed as part
of failover slots testing, but not the Perl test control code for setting
up the nodes, etc. This testing cannot usefully be done with pg_regress and
the part that can be is already added to contrib/test_decoding.



> 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.
>

Yep, did that.


> - All the core routines used should be compatible down to perl 5.8.8.
>

Ugh. So not just Perl, ancient perl.

I don't suppose Perl offers any kind of "compatible(5.8.8)" statement or
anything? Do I have to compile a ten-year-old Perl and its dependencies to
work on PostgreSQL tests?
http://search.cpan.org/dist/Perl-MinimumVersion/lib/Perl/MinimumVersion.pm
looks useful; do you think it's reasonable for code that passes that check
to just be thrown at the buildfarm?


> - The code produced should be sanitized with perltidy before commit.
>

Makes sense. Keeps things consistent.


> > 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.
>

Ok, that makes sense.


> > 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.
>

Will do that now.


> > 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.
>

Not committed yet, I see. That's https://commitfest.postgresql.org/9/438/
right?

I'd rather not duplicate your work there, so I should build on that. Is
there a public git tree for that?

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

Reply via email to