> On 15 Jan 2018, at 02:33, Michael Paquier <michael.paquier@gmail.com> wrote:
> On Fri, Jan 12, 2018 at 11:35:48PM +0100, Daniel Gustafsson wrote:
>> On 11 Jan 2018, at 09:01, Michael Paquier <michael.paquier@gmail.com> wrote:
>>>> One open question from this excercise is how to write a good test for
>>>> this. It can either be made part of the already existing test queries
>>>> or a separate suite. I’m leaning on the latter simply because the
>>>> case-flipping existing tests seems like something that can be cleaned
>>>> up years from now accidentally because it looks odd.
>>>
>>> Adding them into src/test/regress/ sounds like a good plan to me.
>>
>> If there is interest in this patch now that the list exists and aids review, I
>> can turn the list into a proper test that makes a little more sense than the
>> current list which is rather aimed at helping reviewers.
>
> Sorry if my words were hard to catch here. What I mean here is to add in
> each command's test file the set of commands which check the
> compatibility. There is no need to test all the options in my opinion,
> as just testing one option is enoughto show the purpose. So for example
> to cover the grounds of DefineAggregate(), you could add a set of
> commands in create_aggregate.sql. For DefineCollation(), those can go in
> collate.sql, etc.
Gotcha. I’ve added some tests to the patch. The test for CREATE FUNCTION was
omitted for now awaiting the outcome of the discussion around isStrict
isCachable.
Not sure how much they’re worth in "make check” though as it’s quite easy to
add a new option checked with pg_strcasecmp which then isn’t tested. Still, it
might aid review so definitely worth it.
cheers ./daniel
