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

From Heikki Linnakangas
Subject Re: Refactor ssl tests to avoid using internal PostgreSQL::Test::Cluster methods
Date
Msg-id c310e93e-1f0e-0296-c831-79749e648294@iki.fi
Whole thread Raw
In response to Refactor ssl tests to avoid using internal PostgreSQL::Test::Cluster methods  (Daniel Gustafsson <daniel@yesql.se>)
Responses Re: Refactor ssl tests to avoid using internal PostgreSQL::Test::Cluster methods
List pgsql-hackers
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)




pgsql-hackers by date:

Previous
From: Ranier Vilela
Date:
Subject: Re: Avoid unncessary always true test (src/backend/storage/buffer/bufmgr.c)
Next
From: vignesh C
Date:
Subject: Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication