Re: [HACKERS] Fix bloom WAL tap test - Mailing list pgsql-hackers

From Masahiko Sawada
Subject Re: [HACKERS] Fix bloom WAL tap test
Date
Msg-id CAD21AoCXAhZeiOMzr6B8ThgFD50=o_BKbhB83hB3eG3eiwWyeA@mail.gmail.com
Whole thread Raw
In response to Re: [HACKERS] Fix bloom WAL tap test  (Michael Paquier <michael.paquier@gmail.com>)
Responses Re: [HACKERS] Fix bloom WAL tap test  (Alexander Korotkov <a.korotkov@postgrespro.ru>)
List pgsql-hackers
On Wed, Nov 8, 2017 at 11:20 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Wed, Nov 8, 2017 at 1:58 AM, Alexander Korotkov
> <a.korotkov@postgrespro.ru> wrote:
>> On Tue, Nov 7, 2017 at 4:34 PM, Masahiko Sawada <sawada.mshk@gmail.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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

pgsql-hackers by date:

Previous
From: Amit Kapila
Date:
Subject: Re: [HACKERS] why not parallel seq scan for slow functions
Next
From: Michael Paquier
Date:
Subject: Re: [HACKERS] [PATCH] A hook for session start