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)