Horton

Thread

Commits

  1. Use library functions to edit config in SSL tests

  2. Test for PG_TEST_EXTRA separately in SSL tests

  1. Minor cleanups in the SSL tests

    Daniel Gustafsson <daniel@yesql.se> — 2024-05-16T07:24:12Z

    When writing a new SSL test for another patch it struck me that the SSL tests
are doing configuration management without using the test framework API's.  The
attached patches cleans this up, no testcases are altered as part of this.

0001 makes the test for PG_TEST_EXTRA a top-level if statement not attached to
any other conditional.  There is no change in functionality, it's mainly for
readability (PG_TEST_EXTRA is it's own concept, not tied to library presence).

0002 ports over editing configfiles to using append_conf() instead of opening
and writing to them directly.

--
Daniel Gustafsson

  2. Re: Minor cleanups in the SSL tests

    Peter Eisentraut <peter@eisentraut.org> — 2024-05-16T09:43:12Z

    On 16.05.24 09:24, Daniel Gustafsson wrote:
> When writing a new SSL test for another patch it struck me that the SSL tests
> are doing configuration management without using the test framework API's.  The
> attached patches cleans this up, no testcases are altered as part of this.
> 
> 0001 makes the test for PG_TEST_EXTRA a top-level if statement not attached to
> any other conditional.  There is no change in functionality, it's mainly for
> readability (PG_TEST_EXTRA is it's own concept, not tied to library presence).

Makes sense to me.

> 0002 ports over editing configfiles to using append_conf() instead of opening
> and writing to them directly.

Yes, it's probably preferable to use append_conf() here.  You might want 
to run your patch through pgperltidy.  The result doesn't look bad, but 
a bit different than what you had crafted.

append_conf() opens and closes the file for each call.  It might be nice 
if it could accept a list.  Or you can just pass the whole block as one 
string, like it was done for pg_ident.conf before.

  3. Re: Minor cleanups in the SSL tests

    Daniel Gustafsson <daniel@yesql.se> — 2024-05-16T21:27:57Z

    > On 16 May 2024, at 11:43, Peter Eisentraut <peter@eisentraut.org> wrote:

> You might want to run your patch through pgperltidy.  The result doesn't look bad, but a bit different than what you had crafted.

Ugh, I thought I had but clearly had forgotten. Fixed now.

> append_conf() opens and closes the file for each call.  It might be nice if it could accept a list.  Or you can just pass the whole block as one string, like it was done for pg_ident.conf before.

The attached v2 pass the whole block as a here-doc which seemed like the best
option to retain readability of the config.

--
Daniel Gustafsson

  4. Re: Minor cleanups in the SSL tests

    Peter Eisentraut <peter@eisentraut.org> — 2024-05-17T05:57:38Z

    On 16.05.24 23:27, Daniel Gustafsson wrote:
>> On 16 May 2024, at 11:43, Peter Eisentraut <peter@eisentraut.org> wrote:
> 
>> You might want to run your patch through pgperltidy.  The result doesn't look bad, but a bit different than what you had crafted.
> 
> Ugh, I thought I had but clearly had forgotten. Fixed now.
> 
>> append_conf() opens and closes the file for each call.  It might be nice if it could accept a list.  Or you can just pass the whole block as one string, like it was done for pg_ident.conf before.
> 
> The attached v2 pass the whole block as a here-doc which seemed like the best
> option to retain readability of the config.

Works for me.

  5. Re: Minor cleanups in the SSL tests

    Daniel Gustafsson <daniel@yesql.se> — 2024-05-17T07:58:59Z

    > On 17 May 2024, at 07:57, Peter Eisentraut <peter@eisentraut.org> wrote:
> 
> On 16.05.24 23:27, Daniel Gustafsson wrote:
>>> On 16 May 2024, at 11:43, Peter Eisentraut <peter@eisentraut.org> wrote:
>>> You might want to run your patch through pgperltidy.  The result doesn't look bad, but a bit different than what you had crafted.
>> Ugh, I thought I had but clearly had forgotten. Fixed now.
>>> append_conf() opens and closes the file for each call.  It might be nice if it could accept a list.  Or you can just pass the whole block as one string, like it was done for pg_ident.conf before.
>> The attached v2 pass the whole block as a here-doc which seemed like the best
>> option to retain readability of the config.
> 
> Works for me.

Thanks for review. Once the tree opens up for v18 I'll go ahead with this.

--
Daniel Gustafsson

  6. Re: Minor cleanups in the SSL tests

    Daniel Gustafsson <daniel@yesql.se> — 2024-09-03T18:35:12Z

    > On 17 May 2024, at 09:58, Daniel Gustafsson <daniel@yesql.se> wrote:
> 
>> On 17 May 2024, at 07:57, Peter Eisentraut <peter@eisentraut.org> wrote:
>> 
>> On 16.05.24 23:27, Daniel Gustafsson wrote:
>>>> On 16 May 2024, at 11:43, Peter Eisentraut <peter@eisentraut.org> wrote:
>>>> You might want to run your patch through pgperltidy.  The result doesn't look bad, but a bit different than what you had crafted.
>>> Ugh, I thought I had but clearly had forgotten. Fixed now.
>>>> append_conf() opens and closes the file for each call.  It might be nice if it could accept a list.  Or you can just pass the whole block as one string, like it was done for pg_ident.conf before.
>>> The attached v2 pass the whole block as a here-doc which seemed like the best
>>> option to retain readability of the config.
>> 
>> Works for me.
> 
> Thanks for review. Once the tree opens up for v18 I'll go ahead with this.

This has now been pushed after a little bit of editorializing and another
pgperltidy run.

--
Daniel Gustafsson