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 18:23:35 |
Message-ID: | 20150816182335.GA2078904@tornado.leadboat.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Fri, Aug 14, 2015 at 09:31:43AM -0400, Andrew Dunstan wrote:
> On 08/14/2015 09:27 AM, Andrew Dunstan wrote:
> >The code
> >is rearranged a little, and an incorrect piece of code setting
> >$ENV{PERL5LIB} is fixed (in the case where it was previously empty it
> >would have added a spurious ";" and possibly caused a warning as well).
The PERL5LIB bit is clearly good, but please commit it separately.
> >Instead of looking everywhere in the tree for /t directories, the new
> >bincheck function only looks for them in src/bin.
Check.
> >And the tests would die
> >on the first failure, as we would also expect the checks run under "make"
> >to do.
"make" offers a choice. If I had to settle for just one of its behaviors, I
would choose -k. One can emulate "make -S" by killing "make -k" after the
first error, but there's no straightforward way to emulate "make -k" in terms
of "make -S". Thus, I prefer the ancient vcregress decision in this area. In
any event, a proposal to change it would need its own thread and a patch that
changes all targets facing this decision.
> >The effect is to remove the target "tapcheck" for which there is no "make"
> >equivalent, and replace it with the target "bincheck", which is the
> >equivalent of "make -C src/bin installcheck", which happens to be what the
> >buildfarm runs.
This "vcregress bincheck" differs from "make -C src/bin installcheck" by using
"check" semantics, and it differs from "make -C src/bin check" by skipping the
pg_upgrade test suite. (I don't have any point in saying that beyond
clarifying the record.)
> -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
> + # 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.
From | Date | Subject | |
---|---|---|---|
Next Message | Andrew Dunstan | 2015-08-16 21:08:56 | Re: TAP tests are badly named |
Previous Message | Tom Lane | 2015-08-16 16:49:04 | Re: Raising our compiler requirements for 9.6 |