Re: Adding CI to our tree

Hi,

On 2022-01-10 16:07:48 -0600, Justin Pryzby wrote:
> On Sun, Jan 09, 2022 at 11:57:44AM -0800, Andres Freund wrote:
> > On 2022-01-09 13:16:50 -0600, Justin Pryzby wrote:
> > > 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?
>
> Because otherwise vcregress runs pg_regress --temp-config test1 test2 [...]
> ..which means test1 gets eaten as the argument to --temp-config

Ah. I see you changed that globally, good...

I'll probably apply that part and 0002 separately.

> > 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?
>
> It needs to run the tests with separate instance. Maybe you're suggesting to
> use --temp-instance.

Yes.

> It needs to avoid running on the buildfarm, right ?

I guess so. It currently appears to have its own logic for finding contrib
(and other) tap tests:

foreach my $testdir (glob("$pgsql/contrib/*"))
{
next unless -d "$testdir/t";
my $testname = basename($testdir);
next unless step_wanted("contrib-$testname");
print time_str(), "running contrib test $testname ...\n" if $verbose;
run_tap_test("$testdir", "contrib-$testname", undef);
}

but does run vcregress contribcheck, modulescheck.

Andrew, do you see a better way than what Justin is proposing here?

Greetings,

Andres Freund