Re: Refactor ssl tests to avoid using internal PostgreSQL::Test::Cluster methods - Mailing list pgsql-hackers

Hi Daniel,

Thanks for the patch.

Daniel Gustafsson <daniel@yesql.se>, 31 May 2023 Çar, 15:48 tarihinde
şunu yazdı:
>
> To avoid this, the attached adds fail_ok functionality to restart() which makes
> it easier to use it in tests, and aligns it with how stop() and start() works.
> The resulting SSL tests are also more readable IMO.

I agree that it's more readable this way.

I only have a few comments:

>
> +               BAIL_OUT("pg_ctl restart failed") unless $params{fail_ok};
> +               return 0;
> +       }
>
>
>         $self->_update_pid(1);
>         return;

I was comparing this new restart function to start and stop functions.
I see that restart() does not return a value if it's successful while
others return 1.
Its return value is not checked anywhere, so it may not be useful at
the moment. But I feel like it would be nice to make it look like
start()/stop(). What do you think?

> command_fails(
>     [ 'pg_ctl', '-D', $node->data_dir, '-l', $node->logfile, 'restart' ],
>     'restart fails with incorrect SSL protocol bounds');

There are two other places where ssl tests restart the node like
above. We can call $node->restart in those lines too.


Best,
--
Melih Mutlu
Microsoft