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.5090...@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. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers