Re: Add extension options to control TAP and isolation tests

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Arthur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>
Cc: Nikolay Shaplov <dhyan(at)nataraj(dot)su>, pgsql-hackers(at)lists(dot)postgresql(dot)org, Adam Berlin <berlin(dot)ab(at)gmail(dot)com>
Subject: Re: Add extension options to control TAP and isolation tests
Date: 2018-11-24 01:15:30
Message-ID: 20181124011530.GA22038@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Nov 23, 2018 at 05:29:00PM +0300, Arthur Zakirov wrote:
> The patch is very useful. Using TAP_TESTS is more convenient and clearer
> than adding wal-check target. Every time I was adding TAP tests for a
> extension I had to remember that I should add wal-check.

wal-check is a custom option part of contrib/bloom/ which is not aimed
at spreading around.

> After applying the patch all tests pass, there wasn't any error.
>
> Also I tested it in one of our extension which has TAP tests. installcheck
> and check work as expected.

Thanks.

> But there is a problem that you need to copy your extension to the contrib
> directory if you want to run TAP tests. I tried to run TAP test of the
> extension outside of PostgreSQL source directory. And it failed to run the
> test. It is because `prove_installcheck` redefines `top_builddir` and
> `PG_REGRESS`:

I have tested that as well with one of my custom extensions, which has
some TAP tests, and the following Makefile additions:
TAP_TESTS = 1
ifdef USE_PGXS
PG_CONFIG = pg_config
PGXS := $(shell $(PG_CONFIG) --pgxs)
include $(PGXS)
else
subdir = contrib/EXTENSION_NAME_HERE
top_builddir = ../..
include $(top_builddir)/src/Makefile.global
include $(top_srcdir)/contrib/contrib-global.mk
endif

Running make clean at the root of the tree, then running make check from
contrib/EXTENSION_NAME_HERE works for me.

> Unfortunately I didn't find the way to run it, maybe I miss something. It
> can be fixed by an additional patch I attached. I think I can create an
> entry in the future commitfest or it can be joined into your patch.

The previous version of the patch I sent make the build of
src/test/regress dependent on if REGRESS is set, but I missed the fact
that TAP tests also call pg_regress, which is the error you are seeing.
The attached patch will be able to work.

Thanks all for the reviews, I'll do a last lookup on Monday my time and
I'll try to get that committed by then. That's a nice cleanup of the
tree.
--
Michael

Attachment Content-Type Size
pgxs-isolation-tap-v3.patch text/x-diff 13.0 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2018-11-24 01:18:17 Re: Should new partitions inherit their tablespace from their parent?
Previous Message Edmund Horner 2018-11-24 00:56:33 Re: Tid scan improvements