Re: can we add subscription TAP test option "vcregress subscriptioncheck" for MSVC builds?

On 10/18/21 1:41 AM, Bharath Rupireddy wrote:
> On Sat, Oct 16, 2021 at 6:35 PM Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
>>> The problems with having "vcregress checkworld" are: 1) required code
>>> modifications are more as the available "vcregress" test functions,
>>> which required pre-started pg instance, can't be used directly. 2) it
>>> looks like spinning up a separate postgres instance for all module
>>> tests takes time on Windows which might make the test time longer. If
>>> we were to have "vcregress installcheckworld", we might have to add
>>> new code for converting some of the existing functions to not use a
>>> pre-started pg instance.
>>>
>>> IMHO, we can just have "vcregress subscriptioncheck" and let users
>>> decide which tests they want to run.
>>>
>>> I would like to hear more opinions on this.
>>>
>> My opinion hasn't changed. There is already a way to spell this and I'm
>> opposed to adding more such specific tests to vcregress.pl. Every such
>> addition we make increases the maintenance burden.
> Thanks for your opinion. IIUC, the subscription tests can be run with
> setting environment variables PROVE_FLAGS, PROVE_TESTS and the
> "vcregress taptest" command right? I failed to set the environment
> variables appropriately and couldn't run. Can you please let me know
> the right way to run the test?

No extra environment flags should be required for MSVC.

This should suffice:

    vcregress taptest src/test/subscription

If you want to set PROVE_FLAGS the simplest thing is just to set it in
the environment before the above invocation

>
> If any test can be run with a set of environment flags and "vcregress
> taptest" command, then in the first place, it doesn't make sense to
> have recoverycheck, upgragecheck and so on. Another thing is that the
> list of "vcregress" commands cover almost all the tests core, tap,
> bin, isolation, contrib tests except, subscription tests. If we add
> "vcregress subscrtptioncheck", the list of tests that can be run with
> the "vcregress" command will be complete without having to depend on
> the environment variables.

The reason we have some of those other tests is because we didn't start
with having a generic taptest command in vcregress.pl.  So they are
simply legacy code. But that is no reason for adding to them.

>
> IMHO, we can have "vcregress subscriptioncheck" to make it complete
> and easier for the user to run those tests. However, let's hear what
> other hackers have to say about this.

I really fail to see how the invocation above is in any sense
significantly more complicated.

>
> Another thing I noticed is that we don't have any mention of
> "vcregress taptest" command in the docs [1], if I read the docs
> correctly. How about we have it along with a sample example on how to
> run a specific TAP tests with it in the docs?
>
> [1] - https://www.postgresql.org/docs/current/install-windows-full.html
>

Yes, that's probably something that should be remedied.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com