From: | Noah Misch <noah(at)leadboat(dot)com> |
---|---|
To: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: TAP tests are badly named |
Date: | 2015-08-16 22:15:49 |
Message-ID: | 20150816221549.GB2078904@tornado.leadboat.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Sun, Aug 16, 2015 at 05:08:56PM -0400, Andrew Dunstan wrote:
> 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.5090506@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.
While that one piece of Heikki's argument was in error, I didn't feel that it
affected the conclusion. Please reply to the aforementioned email to dispute
the decision; some involved parties may not be following this thread.
> >>+ # 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.
With the comment gone, the way you've done this section is fine.
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2015-08-17 00:30:50 | Re: TAP tests are badly named |
Previous Message | Andrew Dunstan | 2015-08-16 21:08:56 | Re: TAP tests are badly named |