On Wed, Dec 2, 2015 at 8:11 AM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> - I discovered that not setting PGPORT was causing some of the tests
> that fail (using command_fails) to fail to test what was being tested.
> The problem is that the command was failing with "could not connect to
> server" instead of failing because of trying to cluster a nonexistant
> table, etc. Unfortunately the only way to verify this is by looking
> at the regress_log_xxx_foobar file. Not ideal, but not this patch's
> fault.
Nice catch.
> - I changed the routines moved to PostgresNode so that they are instance
> methods, i.e. $node->poll_until_query; also psql and
> issues_query_like. The latter also sets "local $PGPORT" so that the
> command is run with the correct port.
OK.
> - It would be nice to have command_ok and command_fails in PostgresNode
> too; that would remove the need for setting $ENV{PGPORT} but it's
> possible to run commands outside a node too, so we'd need duplicates,
> which would be worse.
I am fine to let it the way your patch does it. There are already many changes.
> - I removed the @active_nodes array, which is now unnecessary (per
> above).
So that's basically replaced by @all_nodes.
> - I moved the "delete $ENV{PGxxx}" back to a BEGIN block in TestLib.
> I did it because it's possible to write test scripts without
> PostgresNode, and it's not nice to have those pick up the environment.
> This still affects anything using PostgresNode because that one uses
> TestLib.
OK.
> Finally, I ran perltidy on all the files, which strangely changed stuff
> that I didn't expect it to change. I wonder if this is related to the
> perltidy version. Do you see further changes if you run perltidy on the
> patched tree?
SimpleTee.pm shows some diffs, though it doesn't seem that this patch
should care about that. The rest is showing no diffs. And I have used
perltidy v20140711.
--
Michael
