On 08/16/2015 08:30 PM, Michael Paquier wrote:
> On Mon, Aug 17, 2015 at 7:15 AM, Noah Misch <noah@leadboat.com> wrote:
>> 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.
> FWIW, I agree with Andrew's approach here. We are going to need this
> parameter switch once TAP routines are used in another code path than
> src/bin to control if a test can be run or not based on the presence
> of t/ and the value of this parameter. See for example the patch aimed
> at testing dumpable tables with pg_dump which should be part of the
> target modulescheck as argued upthread.
> My 2c.
I spoke to Heikki about this the other day, and he's fine with using the
test if there's a need for it. In addition to Michael's point, the
buildfarm has a need for it - if the flag isn't set it won't run the
checks, so the flag should be supported. I'm therefore going to stick
with the code above.
cheers
andrew
