On 08/16/2015 02:23 PM, Noah Misch wrote:
-sub tapcheck
+sub tap_check
{
- InstallTemp();
+ die "Tap tests not enabled in configuration"
+ unless $config->{tap_tests};
I endorse Heikki's argument for having omitted the configuration flag:
http://www.postgresql.org/message-id/55b90161.5090...@iki.fi
That argument is not correct. None of the tap tests can be run via make
if --enable-tap-tests is not set. This doesn't just apply to the
check-world target as Heikki asserted. Have a look at the definitions of
prove_check and prove_installcheck in src/Makefile.global.in if you
don't believe me.
+ # Reset those values, they may have been changed by another test.
+ # XXX is this true?
+ local %ENV = %ENV;
$ENV{PERL5LIB} = "$topdir/src/test/perl;$ENV{PERL5LIB}";
$ENV{PG_REGRESS} = "$topdir/$Config/pg_regress/pg_regress";
+ $ENV{TESTDIR} = "$dir";
The comment pertained only to the TESTDIR environment variable. Adding the
local %ENV is unnecessary. I think you can just delete the comment; there's
nothing noteworthy about setting a different TESTDIR value for each suite.
The PERL5LIB and PG_REGRESS updates need happen just once per bincheck, though
keeping them here seems good for the benefit of future TAP targets.
The local decl is clearly needed. Otherwise repeated invocations of the
function will result in repeated prepending of $topdir/src/test/perl to
PERL5LIB. It's incredibly cheap anyway. And as you say, this is a much
better place to do that than in bincheck. If you prefer, I could
dispense with the local and instead only set to PERL5LIB conditionally.
It's just a matter of style.
cheers
andrew
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
