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