Re: Fix bloom WAL tap test

On Wed, Nov 8, 2017 at 11:20 AM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:
> On Wed, Nov 8, 2017 at 1:58 AM, Alexander Korotkov
> <a(dot)korotkov(at)postgrespro(dot)ru> wrote:
>> On Tue, Nov 7, 2017 at 4:34 PM, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
>> wrote:
>>> I understood the necessity of this patch and reviewed two patches.
>>
>> Good, thank you.
>
> That's clearly a bug fix.
>
>>> diff --git a/contrib/bloom/Makefile b/contrib/bloom/Makefile
>>> index 13bd397..c1566d4 100644
>>> --- a/contrib/bloom/Makefile
>>> +++ b/contrib/bloom/Makefile
>>> @@ -20,5 +20,7 @@ include $(top_builddir)/src/Makefile.global
>>> include $(top_srcdir)/contrib/contrib-global.mk
>>> endif
>>>
>>> +check: wal-check
>>> +
>>> wal-check: temp-install
>>> $(prove_check)
>>
>>
>> I've tried this version Makefile. And I've seen the only difference: when
>> tap tests are enabled, this version of Makefile runs tap tests before
>> regression tests. While my version of Makefile runs tap tests after
>> regression tests. That seems like more desirable behavior for me. But it
>> would be sill nice to simplify Makefile.
>
> Why do you care about the order of actions? There is no dependency
> between each test and it seems to me that it should remain so to keep
> a maximum flexibility. This does not sacrifice coverage as well. In
> short, Sawada-san's suggestion looks like a thing to me. One piece
> that it is missing though is that installcheck triggers only the
> SQL-based tests, and it should also trigger the TAP tests.

+1

> So I think
> that you should instead do something like that:
>
> --- a/contrib/bloom/Makefile
> +++ b/contrib/bloom/Makefile
> @@ -20,5 +20,12 @@ include $(top_builddir)/src/Makefile.global
> include $(top_srcdir)/contrib/contrib-global.mk
> endif
>
> +installcheck: wal-installcheck
> +
> +check: wal-check
> +
> +wal-installcheck:
> + $(prove_installcheck)
> +
> wal-check: temp-install
> $(prove_check)
>
> This works for me as I would expect it should.

Looks good to me too.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center