On Fri, Aug 14, 2015 at 1:18 AM, Andrew Dunstan <andrew@dunslane.net> wrote:
>
>
> On 08/13/2015 12:03 PM, Michael Paquier wrote:
>>
>> On Fri, Aug 14, 2015 at 12:17 AM, Andrew Dunstan wrote:
>>>
>>> here's what I propose.
>>
>> This patch does not take into account that there may be other code
>> paths than src/bin/ that may have TAP tests (see my pending patch to
>> test pg_dump with extensions including dumpable relations for
>> example). I guess that it is done on purpose, now what are we going to
>> do about the following things:
>> - for src/test/ssl, should we have a new target in vcregress? Like
>> ssltest?
>> - for the pending patch I just mentioned, what should we do then?
>> Should we expect it to work under modulescheck?
>
>
>
> Of course it takes it into account.
Yes, sorry. I misread your patch, reading code late in the evening is
not a good idea :)
> What it does is let you add extra checks
> easily. But I am not going to accept the divergence in vcregress.pl from the
> way we run tests using the standard tools.
OK, this sounds fair to keep up with. And sorry for actually breaking
vcregress.pl regarding this consistency. Shouldn't we remove
upgradecheck and move it under the banner bincheck then? Perhaps
testing the upgrade made sense before but now it is located under
src/bin so it sounds weird to do things separately.
> Yes, ssl tests should have a new
> target, as should any other new set of tests. We don't have a single make
> target for tap tests and neither should we in vcregress.pl.
Actually it is not only ssl, I would think that all those targets
should run under the same banner:
ALWAYS_SUBDIRS = examples locale thread ssl
But that's a separate discussion...
+ die "Tap tests not enabled in configuration"
+ unless $config->{tap_tests};
I think that you are missing to update a couple of places with this
parameter, one being GetFakeConfigure@Solution.pm, a second being
config_default.pl. Also, I would think that it is saner to simply
return here and not die, then log a message to user to tell him that
the test has been skipped. This is thinking about the module having
TAP tests that should be located under src/test/modules
Regards,
--
Michael