Re: Adding CI to our tree

Hi,

On 2022-01-09 13:16:50 -0600, Justin Pryzby wrote:
> I noticed a patch failing in cfbot everywhere except windows:
>
> https://commitfest.postgresql.org/36/3476/
> | Invalid relcache when ADD PRIMARY KEY USING INDEX
>
> It's because vcregress skips tests which have NO_INSTALLCHECK=1.

> Is it desirable to enable more module/contrib tests for windows CI ?

Yes. I think the way we run windows tests is pretty bad - it's not reasonable
that each developer needs to figure out 20 magic incantations to run all tests
on windows.

> This does a few, but there's a few others which would require the server to
> be restarted to set shared_preload_libraries for each module.
>
> diff --git a/.cirrus.yml b/.cirrus.yml
> index 19b3737fa11..c427b468334 100644
> --- a/.cirrus.yml
> +++ b/.cirrus.yml
> @@ -390,7 +390,7 @@ task:
> - perl src/tools/msvc/vcregress.pl check parallel
> startcreate_script:
> # paths to binaries need backslashes
> - - tmp_install\bin\pg_ctl.exe initdb -D tmp_check/db -l tmp_check/initdb.log
> + - tmp_install\bin\pg_ctl.exe initdb -D tmp_check/db -l tmp_check/initdb.log --options=--no-sync
> - echo include '%TEMP_CONFIG%' >> tmp_check/db/postgresql.conf
> - tmp_install\bin\pg_ctl.exe start -D tmp_check/db -l tmp_check/postmaster.log
> test_pl_script:

> diff --git a/contrib/test_decoding/Makefile b/contrib/test_decoding/Makefile
> index 9a31e0b8795..14fd847ba7f 100644
> --- a/contrib/test_decoding/Makefile
> +++ b/contrib/test_decoding/Makefile
> @@ -10,7 +10,7 @@ ISOLATION = mxact delayed_startup ondisk_startup concurrent_ddl_dml \
> oldest_xmin snapshot_transfer subxact_without_top concurrent_stream \
> twophase_snapshot
>
> -REGRESS_OPTS = --temp-config $(top_srcdir)/contrib/test_decoding/logical.conf
> +REGRESS_OPTS = --temp-config=$(top_srcdir)/contrib/test_decoding/logical.conf
> ISOLATION_OPTS = --temp-config $(top_srcdir)/contrib/test_decoding/logical.conf
>

Not sure why these are part of the diff?

> diff --git a/src/tools/ci/pg_ci_base.conf b/src/tools/ci/pg_ci_base.conf
> index d8faa9c26c1..52cdb697a57 100644
> --- a/src/tools/ci/pg_ci_base.conf
> +++ b/src/tools/ci/pg_ci_base.conf
> @@ -12,3 +12,24 @@ log_connections = true
> log_disconnections = true
> log_line_prefix = '%m [%p][%b] %q[%a][%v:%x] '
> log_lock_waits = true
> +
> +# test_decoding
> +wal_level = logical
> +max_replication_slots = 4
> +logical_decoding_work_mem = 64kB
> [ more ]

This doesn't really seem like a scalable path forward - duplicating
configuration in more places doesn't seem sane. It seems it'd make more sense
to teach vcregress.pl to run NO_INSTALLCHECK targets properly? ISTM that
changing the options passed to pg_regress based on fetchTests() return value
wouldn't be too hard?

Greetings,

Andres Freund