Another usability issue with our TAP tests

On Mon, Jul 16, 2018 at 01:13:36PM -0400, Tom Lane wrote:

Since "make check-world" is rather chatty, if you get a failure while
running it under high parallelism, the location of the failure has often
scrolled off the terminal window by the time all the other subjobs
exit.

Yes, I have pested about that myself a bit lately.

This is not a huge problem for tests using our traditional infrastructure,
because you can just run "git status" to look for regression.diffs files.
But a TAP test failure leaves nothing behind that git will consider
unusual. I've repeatedly had to run check-world with no parallelism
(wasting many minutes) in order to locate which test actually failed.

Even for tests currently running regression.diffs is created but remains
empty.

I'm not sure about a good way to improve this. One idea that comes
to mind is to tweak the "make check" rules so that the tmp_check
subdirectories are automatically deleted on successful completion,
but not on failure, and then remove tmp_check from the .gitignore lists.
But the trouble with that is sometimes you want to look at the test logs
afterwards, even when make thought the test succeeded.

I definitely want to be able to look at TAP test logs even if they
succeed, and only wipe them out at the next run, so I think that this
should be independent. What about an on-disk empty file then which gets
simply created in the INIT() block of TestLib.pm, and removed in the END
block only if all tests pass? We know the base directory thanks to
$ENV{TESTDIR} which should just be "." if undefined. Then we name it
say, "prove_running" or similar.
--
Michael

On 16.07.18 19:13, Tom Lane wrote:

But a TAP test failure leaves nothing behind that git will consider
unusual. I've repeatedly had to run check-world with no parallelism
(wasting many minutes) in order to locate which test actually failed.

How about something like this:

diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index 95d090e72d..12114d8427 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -396,7 +396,7 @@ endif
 PROVE = @PROVE@
 # There are common routines in src/test/perl, and some test suites have
 # extra perl modules in their own directory.
-PG_PROVE_FLAGS = -I $(top_srcdir)/src/test/perl/ -I $(srcdir)
+PG_PROVE_FLAGS = -I $(top_srcdir)/src/test/perl/ -I $(srcdir) --state=save
 # User-supplied prove flags such as --verbose can be provided in PROVE_FLAGS.
 PROVE_FLAGS =

@@ -420,12 +420,14 @@ define prove_installcheck
rm -rf '$(CURDIR)'/tmp_check
$(MKDIR_P) '$(CURDIR)'/tmp_check
cd $(srcdir) && TESTDIR='$(CURDIR)' PATH="$(bindir):$$PATH" PGPORT='6$(DEF_PGPORT)' top_builddir='$(CURDIR)/$(top_builddir)' PG_REGRESS='$(CURDIR)/$(top_builddir)/src/test/regress/pg_regress' $(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) $(if $(PROVE_TESTS),$(PROVE_TESTS),t/*.pl)
+@rm -f .prove
endef

define prove_check
rm -rf '$(CURDIR)'/tmp_check
$(MKDIR_P) '$(CURDIR)'/tmp_check
cd $(srcdir) && TESTDIR='$(CURDIR)' $(with_temp_install) PGPORT='6$(DEF_PGPORT)' PG_REGRESS='$(CURDIR)/$(top_builddir)/src/test/regress/pg_regress' $(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) $(if $(PROVE_TESTS),$(PROVE_TESTS),t/*.pl)
+@rm -f .prove
endef

else

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

How about something like this:

-PG_PROVE_FLAGS = -I $(top_srcdir)/src/test/perl/ -I $(srcdir)
+PG_PROVE_FLAGS = -I $(top_srcdir)/src/test/perl/ -I $(srcdir) --state=save

Cute idea, but it seems not to work with older versions of prove:

$ which prove
/usr/local/perl5.8.3/bin/prove
$ prove --state=save
Unknown option: s

We could just do a "touch .prove_running" beforehand and an "rm" after,
perhaps (I think Michael suggested something like that already).

regards, tom lane

On Tue, Jul 17, 2018 at 03:02:32PM -0400, Tom Lane wrote:

Cute idea, but it seems not to work with older versions of prove:

$ which prove
/usr/local/perl5.8.3/bin/prove
$ prove --state=save
Unknown option: s

I didn't know this one, and that's actually nice, but I cannot get
easily a way to track down which tests failed during a parallel run...
And I am used as well to hunt those with "git status".

We could just do a "touch .prove_running" beforehand and an "rm" after,
perhaps (I think Michael suggested something like that already).

Yes, except that in the idea of upthread TestLib.pm would be in control
of it, so as there is less code churn in prove_check and
prove_installcheck. But I am having second-thoughts about putting the
test state directly in the module as that's a four-liner in
Makefile.global.in, and that seems to work even if you put a die() to
fail hard or even if only a subset of tests is failing.

Thoughts?
--
Michael