Michael Paquier wrote: > On Mon, Nov 30, 2015 at 6:28 AM, Noah Misch <n...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
