Обсуждение: Allow passing extra options to initdb for tests

Поиск
Список
Период
Сортировка

Allow passing extra options to initdb for tests

От
Peter Eisentraut
Дата:
I'm proposing here a way to pass extra options to initdb when run 
internally during test setup in pg_regress or 
PostgreSQL::Test::Cluster's init (which covers just about all test 
suites other than initdb's own tests).

For example:

     make check PG_TEST_INITDB_EXTRA_OPTS='-k -c work_mem=50MB'

We currently document at [0] a way to pass custom server settings to the 
tests via PGOPTIONS.  But this only works for pg_regress and only for 
options that can be changed at run time.  My proposal can set initdb 
options, and since initdb has the -c option now, it can set any GUC 
parameter as well.

I think this can be useful for a wide variety of uses, like running all 
tests with checksums enabled, or with JIT enabled, or with different GUC 
settings, or with different locale settings.  (The existing pg_regress 
--no-locale option is essentially a special case of this, but it only 
provides one particular locale setting, not things like changing the 
default provider etc.)

Of course, not all tests are going to pass with arbitrary options, but 
it is useful to run this against specific test suites.

This patch also updates the documentation at [0] to explain the new 
method and distinguish it from the previously documented methods.


[0]: 
https://www.postgresql.org/docs/devel/regress-run.html#REGRESS-RUN-CUSTOM-SETTINGS

Вложения

Re: Allow passing extra options to initdb for tests

От
Ian Lawrence Barwick
Дата:
2024年2月6日(火) 19:54 Peter Eisentraut <peter@eisentraut.org>:
>
> I'm proposing here a way to pass extra options to initdb when run
> internally during test setup in pg_regress or
> PostgreSQL::Test::Cluster's init (which covers just about all test
> suites other than initdb's own tests).
>
> For example:
>
>      make check PG_TEST_INITDB_EXTRA_OPTS='-k -c work_mem=50MB'
>
> We currently document at [0] a way to pass custom server settings to the
> tests via PGOPTIONS.  But this only works for pg_regress and only for
> options that can be changed at run time.  My proposal can set initdb
> options, and since initdb has the -c option now, it can set any GUC
> parameter as well.
>
> I think this can be useful for a wide variety of uses, like running all
> tests with checksums enabled, or with JIT enabled, or with different GUC
> settings, or with different locale settings.  (The existing pg_regress
> --no-locale option is essentially a special case of this, but it only
> provides one particular locale setting, not things like changing the
> default provider etc.)
>
> Of course, not all tests are going to pass with arbitrary options, but
> it is useful to run this against specific test suites.
>
> This patch also updates the documentation at [0] to explain the new
> method and distinguish it from the previously documented methods.

+1 for this, I recently ran into an issue with the regression tests for an
extension where it would have been very useful to provide some initdb
options.

Patch works as expected after a quick initial test.

Regards

Ian Barwick



Re: Allow passing extra options to initdb for tests

От
Ian Lawrence Barwick
Дата:
2024年2月7日(水) 12:51 Ian Lawrence Barwick <barwick@gmail.com>:
>
> 2024年2月6日(火) 19:54 Peter Eisentraut <peter@eisentraut.org>:
> >
> > I'm proposing here a way to pass extra options to initdb when run
> > internally during test setup in pg_regress or
> > PostgreSQL::Test::Cluster's init (which covers just about all test
> > suites other than initdb's own tests).
> >
> > For example:
> >
> >      make check PG_TEST_INITDB_EXTRA_OPTS='-k -c work_mem=50MB'
> >
> > We currently document at [0] a way to pass custom server settings to the
> > tests via PGOPTIONS.  But this only works for pg_regress and only for
> > options that can be changed at run time.  My proposal can set initdb
> > options, and since initdb has the -c option now, it can set any GUC
> > parameter as well.
> >
> > I think this can be useful for a wide variety of uses, like running all
> > tests with checksums enabled, or with JIT enabled, or with different GUC
> > settings, or with different locale settings.  (The existing pg_regress
> > --no-locale option is essentially a special case of this, but it only
> > provides one particular locale setting, not things like changing the
> > default provider etc.)
> >
> > Of course, not all tests are going to pass with arbitrary options, but
> > it is useful to run this against specific test suites.
> >
> > This patch also updates the documentation at [0] to explain the new
> > method and distinguish it from the previously documented methods.
>
> +1 for this, I recently ran into an issue with the regression tests for an
> extension where it would have been very useful to provide some initdb
> options.
>
> Patch works as expected after a quick initial test.

I had a longer look at this and can't find any issues with the code or
documentation changes.

I did wonder whether it would be worth mentioning that any initdb
options set in "PG_TEST_INITDB_EXTRA_OPTS" will override those
which can be set by pg_regress, but of the four ("--no-clean", "--no-sync",
"--debug" and "--no-locale"), only the optional  "--no-locale" can actually
be overridden, so it doesn't seem important.

Regards

Ian Barwick



Re: Allow passing extra options to initdb for tests

От
Robert Haas
Дата:
On Tue, Feb 6, 2024 at 4:24 PM Peter Eisentraut <peter@eisentraut.org> wrote:
> I think this can be useful for a wide variety of uses, like running all
> tests with checksums enabled, or with JIT enabled, or with different GUC
> settings, or with different locale settings.  (The existing pg_regress
> --no-locale option is essentially a special case of this, but it only
> provides one particular locale setting, not things like changing the
> default provider etc.)
>
> Of course, not all tests are going to pass with arbitrary options, but
> it is useful to run this against specific test suites.

+1.

--
Robert Haas
EDB: http://www.enterprisedb.com



Re: Allow passing extra options to initdb for tests

От
Peter Eisentraut
Дата:
On 14.02.24 06:22, Ian Lawrence Barwick wrote:
> I had a longer look at this and can't find any issues with the code or
> documentation changes.

Thanks, committed.

> I did wonder whether it would be worth mentioning that any initdb
> options set in "PG_TEST_INITDB_EXTRA_OPTS" will override those
> which can be set by pg_regress, but of the four ("--no-clean", "--no-sync",
> "--debug" and "--no-locale"), only the optional  "--no-locale" can actually
> be overridden, so it doesn't seem important.

I thought about this.  We don't have a man page for pg_regress, so there 
is no place to comprehensively document all the options and their 
interactions.  The documentation in regress.sgml works on a makefile 
level.  AFAICT, the --debug option is not exposed via the makefiles at 
all, while --no-locale can be requested by the environment variable 
NOLOCALE, but that is not documented, and also not ported to meson.  I 
think if you want tweaks on this level, there is some expectations right 
now that you might need to study the source code a bit.

One thing that might be interesting would be to allow running initdb 
without the --no-sync option, to exercise fsync a bit.  But there is no 
"--yes-sync" option to override --no-sync, otherwise you could do it 
with the just-committed feature.




Re: Allow passing extra options to initdb for tests

От
Daniel Gustafsson
Дата:
> On 15 Feb 2024, at 11:38, Peter Eisentraut <peter@eisentraut.org> wrote:

> We don't have a man page for pg_regress, so there is no place to comprehensively document all the options and their
interactions.

This comes up every now and again, just yesterday there was a question on
-general [0] about alternate output files which also are undocumented.  Maybe
it's time to add documentation for pg_regress?

--
Daniel Gustafsson

[0] CACX+KaPOzzRHEt4w_=iqKbTpMKjyrUGVng1C749yP3r6dprtcg@mail.gmail.com


Re: Allow passing extra options to initdb for tests

От
Tom Lane
Дата:
Daniel Gustafsson <daniel@yesql.se> writes:
> On 15 Feb 2024, at 11:38, Peter Eisentraut <peter@eisentraut.org> wrote:
>> We don't have a man page for pg_regress, so there is no place to comprehensively document all the options and their
interactions.

> This comes up every now and again, just yesterday there was a question on
> -general [0] about alternate output files which also are
> undocumented.

Really?

https://www.postgresql.org/docs/devel/regress-variant.html

> Maybe it's time to add documentation for pg_regress?

I'm inclined to think that a formal man page wouldn't be that useful,
since nobody ever invokes pg_regress directly; as Peter says, what
is of interest is the "make check" targets and the corresponding meson
behaviors.  I think 32.1 is already reasonably thorough about the
make targets; but the complete lack of equivalent info about what
to do in a meson build clearly needs to be rectified.

            regards, tom lane



Re: Allow passing extra options to initdb for tests

От
Daniel Gustafsson
Дата:
> On 15 Feb 2024, at 16:21, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Daniel Gustafsson <daniel@yesql.se> writes:
>> On 15 Feb 2024, at 11:38, Peter Eisentraut <peter@eisentraut.org> wrote:
>>> We don't have a man page for pg_regress, so there is no place to comprehensively document all the options and their
interactions.
>
>> This comes up every now and again, just yesterday there was a question on
>> -general [0] about alternate output files which also are
>> undocumented.
>
> Really?
>
> https://www.postgresql.org/docs/devel/regress-variant.html

Doh, I missed that when looking.

>> Maybe it's time to add documentation for pg_regress?
>
> I'm inclined to think that a formal man page wouldn't be that useful,
> since nobody ever invokes pg_regress directly;

Right, I was mostly thinking of a chapter along the lines of "I am an extension
author, what features does this tool have to help me write good tests".

> the complete lack of equivalent info about what
> to do in a meson build clearly needs to be rectified.

Indeed.

--
Daniel Gustafsson