Re: Refactor ssl tests to avoid using internal PostgreSQL::Test::Cluster methods

On 31/05/2023 15:47, Daniel Gustafsson wrote:
> The SSL tests for pg_ctl restarts with an incorrect key passphrase run pg_ctl
> manually and use the internal method _update_pid to set the server PID file
> accordingly.  This is needed since $node->restart will BAIL in case the restart
> fails, which clearly isn't useful to anyone wanting to test restarts.  This is
> the only use of _update_pid outside of Cluster.pm.
> 
> 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.

Makes sense.

> diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
> index 76442de063..e33f648aae 100644
> --- a/src/test/ssl/t/001_ssltests.pl
> +++ b/src/test/ssl/t/001_ssltests.pl
> @@ -85,10 +85,8 @@ switch_server_cert(
>      passphrase_cmd => 'echo wrongpassword',
>      restart => 'no');
>  
> -command_fails(
> -    [ 'pg_ctl', '-D', $node->data_dir, '-l', $node->logfile, 'restart' ],
> -    'restart fails with password-protected key file with wrong password');
> -$node->_update_pid(0);
> +$result = $node->restart(fail_ok => 1);
> +is($result, 0, 'restart fails with password-protected key file with wrong password');
>  
>  switch_server_cert(
>      $node,

In principle, this makes the tests more lenient. If "pg_ctl restart" 
fails because of a timeout, for example, the PID file could be present. 
Previously, the _update_pid(0) would error out on that, but now it's 
accepted. I think that's fine, but just wanted to point it out.

-- 
Heikki Linnakangas
Neon (https://neon.tech)