Hi,

On 2022-03-26 16:24:32 -0400, Tom Lane wrote:
> Robert Haas <robertmh...@gmail.com> writes:
> > On Sat, Mar 26, 2022 at 3:35 PM Andres Freund <and...@anarazel.de> wrote:
> >> === tap tests in src/bin/pg_resetwal ===
> >> t/001_basic.pl ...... ok
> >> t/002_corrupted.pl .. ok
> >> All tests successful.
>
> > Yeah, this certainly seems like an improvement to me.

Do we want to do the same of regress and isolation tests? They're mostly a bit
easier to place, but it's still a memory retention game.  Using the above
format for all looks a tad weird, due to pg_regress' output having kinda
similar markers.

...
======================
 All 22 tests passed.
======================

=== regress tests in contrib/ltree_plpython" ===
============== creating temporary instance            ==============
============== initializing database system           ==============
============== starting postmaster                    ==============
running on port 51696 with PID 3905518
============== creating database "contrib_regression" ==============
CREATE DATABASE
ALTER DATABASE
============== installing ltree                       ==============
CREATE EXTENSION
============== running regression test queries        ==============
test ltree_plpython               ... ok           51 ms
============== shutting down postmaster               ==============
============== removing temporary instance            ==============
...


Could just use a different character. +++ doesn't look bad:
+++ tap tests in contrib/test_decoding +++
t/001_repl_stats.pl .. ok
All tests successful.
Files=1, Tests=2,  3 wallclock secs ( 0.02 usr  0.00 sys +  1.74 cusr  0.28 
csys =  2.04 CPU)
Result: PASS


Would we want to do this in all branches? I'd vote for yes, but ...

Prototype patch attached. I looked through the uses of
  pg_(isolation_)?regress_(install)?check'
and didn't find any that'd have a problem with turning the invocation into a
multi-command one.


> +1, but will it help for CI

Yes, it should make it considerably better (for everything but windows, but
that outputs separators already).


> or buildfarm cases?

Probably not much, because that largely runs tests serially with "stage" names
corresponding to the test. And when it runs multiple tests in a row it adds
something similar to the above, e.g.:
=========== Module pg_stat_statements check =============
https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=peripatus&dt=2022-03-26%2000%3A20%3A30&stg=misc-check

But I think it'll still be a tad better when it runs a single test:
https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=snapper&dt=2022-03-26%2018%3A46%3A28&stg=subscription-check


Might make it more realistic to make -s, at least to run tests? The reams of
output like:
gmake -C ../../../../src/test/regress pg_regress
gmake[1]: Entering directory 
'/home/pgbuildfarm/buildroot/HEAD/pgsql.build/src/test/regress'
gmake -C ../../../src/port all
gmake[2]: Entering directory 
'/home/pgbuildfarm/buildroot/HEAD/pgsql.build/src/port'
gmake[2]: Nothing to be done for 'all'.
gmake[2]: Leaving directory 
'/home/pgbuildfarm/buildroot/HEAD/pgsql.build/src/port'
gmake -C ../../../src/common all
gmake[2]: Entering directory 
'/home/pgbuildfarm/buildroot/HEAD/pgsql.build/src/common'
gmake[2]: Nothing to be done for 'all'.

are quite clutter-y.

Greetings,

Andres Freund
diff --git i/src/Makefile.global.in w/src/Makefile.global.in
index 0726b2020ff..4014e437fc6 100644
--- i/src/Makefile.global.in
+++ w/src/Makefile.global.in
@@ -448,6 +448,7 @@ ifeq ($(enable_tap_tests),yes)
 
 ifndef PGXS
 define prove_installcheck
+echo "+++ tap tests in $(subdir) +++" && \
 rm -rf '$(CURDIR)'/tmp_check && \
 $(MKDIR_P) '$(CURDIR)'/tmp_check && \
 cd $(srcdir) && \
@@ -458,6 +459,7 @@ cd $(srcdir) && \
 endef
 else # PGXS case
 define prove_installcheck
+echo "+++ tap tests in $(subdir) +++" && \
 rm -rf '$(CURDIR)'/tmp_check && \
 $(MKDIR_P) '$(CURDIR)'/tmp_check && \
 cd $(srcdir) && \
@@ -469,6 +471,7 @@ endef
 endif # PGXS
 
 define prove_check
+echo "+++ tap tests in $(subdir) +++" && \
 rm -rf '$(CURDIR)'/tmp_check && \
 $(MKDIR_P) '$(CURDIR)'/tmp_check && \
 cd $(srcdir) && \
@@ -663,6 +666,7 @@ pg_regress_locale_flags = $(if $(ENCODING),--encoding=$(ENCODING)) $(NOLOCALE)
 pg_regress_clean_files = results/ regression.diffs regression.out tmp_check/ tmp_check_iso/ log/ output_iso/
 
 pg_regress_check = \
+    echo "+++ regress tests in $(subdir) +++" && \
     $(with_temp_install) \
     $(top_builddir)/src/test/regress/pg_regress \
     --temp-instance=./tmp_check \
@@ -671,12 +675,14 @@ pg_regress_check = \
     $(TEMP_CONF) \
     $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS)
 pg_regress_installcheck = \
+    echo "+++ regress tests in $(subdir) +++" && \
     $(top_builddir)/src/test/regress/pg_regress \
     --inputdir=$(srcdir) \
     --bindir='$(bindir)' \
     $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS)
 
 pg_isolation_regress_check = \
+    echo "+++ isolation tests in $(subdir) +++" && \
     $(with_temp_install) \
     $(top_builddir)/src/test/isolation/pg_isolation_regress \
     --temp-instance=./tmp_check_iso \
@@ -685,6 +691,7 @@ pg_isolation_regress_check = \
     $(TEMP_CONF) \
     $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS)
 pg_isolation_regress_installcheck = \
+    echo "+++ isolation tests in $(subdir) +++" && \
     $(top_builddir)/src/test/isolation/pg_isolation_regress \
     --inputdir=$(srcdir) --outputdir=output_iso \
     --bindir='$(bindir)' \

Reply via email to