Michael Paquier wrote:
> On Mon, Nov 30, 2015 at 6:28 AM, Noah Misch <noah@leadboat.com> wrote:
> > The proposed code is short on guidance about when to put a function in TestLib
> > versus TestBase. TestLib has no header comment. The TestBase header comment
> > would permit, for example, command_ok() in that module. I would try instead
> > keeping TestLib as the base module and moving into PostgresNode the functions
> > that deal with PostgreSQL clusters (get_new_node teardown_node psql
> > poll_query_until issues_sql_like).
>
> PostgresNode is wanted to be a base representation of how of node is,
> not of how to operate on it. The ways to perform the tests, which
> works on a node, is wanted as a higher-level operation.
>
> Logging and base configuration of a test set is a lower level of
> operations than PostgresNode, because cluster nodes need actually to
> perform system calls, some of those system calls like run_log allowing
> to log in the centralized log file. I have tried to make the headers
> of those modules more verbose, please see attached.
I'm not terribly convinced by this argument TBH. Perhaps we can have
PostgresNode be one package, and the logging routines be another
package, and we create a higher-level package whose @ISA=(PostgresNode,
LoggingWhatever) and then we move the routines suggested by Noah into
that new package. Then the tests use that instead of PostgresNode
directly.
> > > --- a/src/bin/pg_rewind/t/001_basic.pl
> > > +++ b/src/bin/pg_rewind/t/001_basic.pl
> > > @@ -1,9 +1,11 @@
> > > +# Basic pg_rewind test.
> > > +
> > > use strict;
> > > use warnings;
> > > -use TestLib;
> > > -use Test::More tests => 8;
> > >
> > > use RewindTest;
> > > +use TestLib;
> > > +use Test::More tests => 8;
> >
> > Revert all changes to this file. Audit the rest of the patch for whitespace
> > change unrelated to the subject.
>
> Done.
I perltidied several files, though not consistently. Regarding this
particular hunk, what is going on here is that I moved "use strict;use
warnings" as one stanza, followed by all the other "use" lines as
another stanza, alphabetically. It was previously a bit messy, with
@EXPORTS and other stuff in between "use" lines.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
