On 2/18/21 11:01 AM, Andrew Dunstan wrote: > On 2/17/21 3:42 PM, Thomas Munro wrote: >> On Thu, Feb 18, 2021 at 9:18 AM Andrew Dunstan <and...@dunslane.net> wrote: >>> On 2/17/21 11:06 AM, Tom Lane wrote: >>>> Peter Smith <smithpb2...@gmail.com> writes: >>>>> I saw that one of our commitfest entries (32/2914) is recently >>>>> reporting a fail on the cfbot site [1]. I thought this was all ok a >>>>> few days ago. >>>>> ... >>>>> Is there any other detailed information available anywhere, e.g. >>>>> logs?, which might help us work out what was the cause of the test >>>>> failure? >>>> AFAIK the cfbot doesn't capture anything beyond the session typescript. >>>> However, this doesn't look that hard to reproduce locally ... have you >>>> tried, using similar configure options to what that cfbot run did? >>>> Once you did reproduce it, there'd be logs under >>>> contrib/test_decoding/tmp_check/. >>> yeah. The cfbot runs check-world which makes it difficult for it to know >>> which log files to show when there's an error. That's a major part of >>> the reason the buildfarm runs a much finer grained set of steps. >> Yeah, it's hard to make it print out just the right logs without >> dumping so much stuff that it's hard to see the wood for the trees; >> perhaps if the Makefile had an option to dump relevant stuff for the >> specific tests that failed, or perhaps the buildfarm is already better >> at that and cfbot should just use the buildfarm client directly. Hmm. >> Another idea would be to figure out how to make a tarball of all log >> files that you can download for inspection with better tools at home >> when things go wrong. It would rapidly blow through the 1GB limit for >> stored "artefacts" on open source/community Cirrus accounts though, so >> we'd need to figure out how to manage retention carefully. > > I did some thinking about this. How about if we have the make files and > the msvc build system create a well known file with the location(s) to > search for log files if there's an error. Each bit of testing could > overwrite this file before starting testing, and then tools like cfbot > would know where to look for files to report? To keep things clean for > other users the file would only be created if, say, > PG_NEED_ERROR_LOG_LOCATIONS is set. The well known location would be > something like "$(top_builddir)/error_log_locations.txt", and individual > Makefiles would have entries something like:, > > > override ERROR_LOG_LOCATIONS = > $(top_builddir)/contrib/test_decoding/tmp_check/log > > > If this seems like a good idea I can go and try to make that happen. > >
here's a very small and simple (and possibly naive) POC patch that demonstrates this and seems to do the right thing. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
diff --git a/src/Makefile.global.in b/src/Makefile.global.in index 74b3a6acd2..a1339d6ec9 100644 --- a/src/Makefile.global.in +++ b/src/Makefile.global.in @@ -390,6 +390,15 @@ endif endif +.PHONY: set_error_log_locations + +set_error_log_locations: +ifdef PG_NEED_ERROR_LOCATIONS +ifdef ERROR_LOG_LOCATIONS + echo "$(ERROR_LOG_LOCATIONS)" > $(top_builddir)/error_log_locations.txt +endif +endif + # Testing # In much the same way as above, these rules ensure that we build a temp diff --git a/src/bin/psql/Makefile b/src/bin/psql/Makefile index d00881163c..c325d76b52 100644 --- a/src/bin/psql/Makefile +++ b/src/bin/psql/Makefile @@ -12,6 +12,8 @@ PGFILEDESC = "psql - the PostgreSQL interactive terminal" PGAPPICON=win32 +override ERROR_LOG_LOCATIONS = $(abs_top_builddir)/$(subdir)/tmp_check/log + subdir = src/bin/psql top_builddir = ../../.. include $(top_builddir)/src/Makefile.global @@ -83,7 +85,7 @@ clean distclean: maintainer-clean: distclean rm -f sql_help.h sql_help.c psqlscanslash.c -check: +check: set_error_log_locations $(prove_check) installcheck: diff --git a/src/test/regress/GNUmakefile b/src/test/regress/GNUmakefile index 3d85857bfa..5fbcb5e34d 100644 --- a/src/test/regress/GNUmakefile +++ b/src/test/regress/GNUmakefile @@ -13,6 +13,8 @@ PGFILEDESC = "pg_regress - test driver" PGAPPICON = win32 +override ERROR_LOG_LOCATIONS = $(abs_top_builddir)/$(subdir)/log + subdir = src/test/regress top_builddir = ../../.. include $(top_builddir)/src/Makefile.global @@ -128,10 +130,10 @@ tablespace-setup: REGRESS_OPTS = --dlpath=. --max-concurrent-tests=20 $(EXTRA_REGRESS_OPTS) -check: all tablespace-setup +check: all set_error_log_locations tablespace-setup $(pg_regress_check) $(REGRESS_OPTS) --schedule=$(srcdir)/parallel_schedule $(MAXCONNOPT) $(EXTRA_TESTS) -check-tests: all tablespace-setup | temp-install +check-tests: all set_error_log_locations tablespace-setup | temp-install $(pg_regress_check) $(REGRESS_OPTS) $(MAXCONNOPT) $(TESTS) $(EXTRA_TESTS) installcheck: all tablespace-setup
