Re: Writing new unit tests with PostgresNode

From: Craig Ringer <craig(at)2ndquadrant(dot)com>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Writing new unit tests with PostgresNode
Date: 2016-02-22 13:16:24
Message-ID: CAMsr+YEOXk5T2TpvnHDt0q4wVn-gr8bF5dkcKTCjuMSXRX83fA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 22 February 2016 at 20:21, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
wrote:

> On Mon, Feb 22, 2016 at 7:55 PM, Craig Ringer <craig(at)2ndquadrant(dot)com>
> wrote:
> > Er, the patch is attached this time.
>
> top_builddir = ../..
> include $(top_builddir)/src/Makefile.global
>
> -SUBDIRS = regress isolation modules
> +SUBDIRS = regress isolation modules example_suite
>
> I have no problem with a test suite to be included in SUBDIRS, but if
> that's a template test it would be better if added in ALWAYS_SUBDIRS.
>

Fair enough. My thinking in adding it to SUBDIRS is that it's trivial
enough to cost practically nothing, and things that aren't run by default
tend to bitrot.

> I think as well that having it in src/test/perl/example_suite would
> make more sense.
>

Yeah, it probably would.

> --- /dev/null
> +++ b/src/test/README
> @@ -0,0 +1,37 @@
> +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.
> The usual format for README files is more like that:
>

Can do.

>
> + use strict;
> + use warnings;
> + use 5.8.8;
> + use PostgresNode;
> + use TestLib;
> I am -1 for including a forced version number in the list of
> inclusions. We have been careful about not using routines that are not
> supported with perl < 5.8.8. Let's continue like that. Simply
> mentioning in the README this minimal requirement is enough IMO.
>

Ok, no complaint from me.

>
> +
> +=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.
You don't write Test::More tests by reading Test/More.pm, you read its
perldoc. Same deal.

SGML ghastly to write and would separate the test docs from the tests
themselves, so I think it's pretty much the worst of all worlds.

So. Yes. I do think pod is desirable. Plus anything but the most extremely
primitive editor highlights it nicely and it's trivial to write even if,
like me, you haven't really used Perl in five years.

Have a look at

perldoc src/test/perl/PostgresNode.pm

(and imagine there was a SYNOPSIS and EXAMPLES section, which there isn't
yet). I think that's a *lot* more readable than dredging through the
sources, and it's only going to get more so as the functionality grows.

diff --git a/src/test/mb/.gitignore b/src/test/mb/.gitignore
> new file mode 100644
> index 0000000..6628455
> --- /dev/null
> +++ b/src/test/mb/.gitignore
> @@ -0,0 +1 @@
> +/results/
> This should be a separate patch.
>

Eh, sure.

> This patch is going a bit more far than I expected first, I thought
> that this would be only a set of README files added into core.

Yeah, as I said I realised while writing the "how to write tests" and "how
to write modules" parts that I'd basically be writing code-in-a-README that
was guaranteed to bitrot into uselessness since it couldn't be run and
tested automatically. A sample module with some comments/pod describing
what it does is a much, much more useful way to achieve that end and the
README can just refer to it.

> And it
> is not completely clear what we would gain by using perlpod in the
> case of those in-core modules

As above, readability when you're authoring tests not hacking on the test
framework.

I really don't like writing Perl. The last thing I want to do is read
through reams of Perl to find out if TestLib or PostgresNode have some
functionality I need in a test or if I need to add it / do it in my own
code. I certainly don't want to be doing that repeatedly when I'm just
trying to look up the signature for methods as I write tests.

It might make more sense if you think of it as infrastructure that lets
people write tests, not the end product its self. In an ideal world nobody
would have to look at it, they'd just use it. Do you read pg_regress.c when
you're writing regression tests? Nope, you read the README and some
examples. Same deal.

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2016-02-22 13:40:11 Re: Re: [COMMITTERS] pgsql: Add some isolation tests for deadlock detection and resolution.
Previous Message Fabien COELHO 2016-02-22 13:11:05 Re: checkpointer continuous flushing - V18