| From: | Melih Mutlu <m(dot)melihmutlu(at)gmail(dot)com> |
|---|---|
| To: | Daniel Gustafsson <daniel(at)yesql(dot)se> |
| Cc: | Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Refactor ssl tests to avoid using internal PostgreSQL::Test::Cluster methods |
| Date: | 2023-05-31 13:46:23 |
| Message-ID: | CAGPVpCRjCW13f_9fUGZmMiLg5nf_TcHbGUTF6aM3NjuVY3O1zQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Daniel,
Thanks for the patch.
Daniel Gustafsson <daniel(at)yesql(dot)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
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Jacob Champion | 2023-05-31 14:08:39 | Re: Docs: Encourage strong server verification with SCRAM |
| Previous Message | Daniel Gustafsson | 2023-05-31 12:47:54 | Refactor ssl tests to avoid using internal PostgreSQL::Test::Cluster methods |