> On 27 Sep 2023, at 14:39, Masahiko Sawada <sawada.m...@gmail.com> wrote:

> I've attached new version patches. 0001 patch adds an option to
> background_psql to specify the timeout seconds, and 0002 patch is the
> main regression test patch.

-=item PostgreSQL::Test::BackgroundPsql->new(interactive, @params)
+=item PostgreSQL::Test::BackgroundPsql->new(interactive, @params, timeout)

Looking at this I notice that I made a typo in 664d757531e, the =item line
should have "@psql_params" and not "@params".  Perhaps you can fix that minor
thing while in there?


+       $timeout = $params{timeout} if defined $params{timeout};

I think this should be documented in the background_psql POD docs.


+       Not enabled by default it is resource intensive.

This sentence is missing a "because", should read: "..by default *because* it
is.."


+# Bump the query timeout to avoid false negatives on slow test systems.
+my $psql_timeout_secs = 4 * $PostgreSQL::Test::Utils::timeout_default;

Should we bump the timeout like this for all systems?  I interpreted Noah's
comment such that it should be possible for slower systems to override, not
that it should be extended everywhere, but I might have missed something.

--
Daniel Gustafsson



Reply via email to