Обсуждение: making pg_regress less noisy by removing boilerplate
Hi, When running check-world, a good chunk of the output is just pg_regress boilerplate. It doesn't matter when running tests individually or for tests with a lot of individual tests like the main regression tests. But for lots of the rest it is noisy. These days there are many more regression tests than there used to be when the output was designed... ============== creating temporary instance ============== ============== initializing database system ============== ============== starting postmaster ============== running on port 51696 with PID 1156405 ============== creating database "contrib_regression" ============== CREATE DATABASE ALTER DATABASE ============== installing hstore ============== CREATE EXTENSION ============== running regression test queries ============== test python3/hstore_plpython ... ok 50 ms ============== shutting down postmaster ============== ============== removing temporary instance ============== ===================== All 1 tests passed. ===================== ============== creating temporary instance ============== [...] It seems we could - combine "creating temporary instance", "initializing database system" and "starting postmaster" - combine "creating database" and "installing *" sections - combine "shutting down postmaster" and "removing temporary instance" - put CREATE DATABASE, ALTER DATABASE, CREATE EXTENSION into a single line without loosing much information, but reducing the vertical space used a lot? It might even be worth further collapsing "starting postmaster" and "creating database" etc into one. There's also Daniel's patch of adding a tap compatible output mode [1]. I still like that idea a lot - being able to parse test successes / failures in a uniform way across tests would be great. The meson testrunner for example would benefit from that. But if I understood that patch, it'd also benefit from the reduction in unnecessary headers, albeit not as crucially. Greetings, Andres Freund [1] https://postgr.es/m/62A6B9F0-C3D3-45CD-8E8B-90A8E5B08DFA%40yesql.se
Andres Freund <andres@anarazel.de> writes: > When running check-world, a good chunk of the output is just pg_regress > boilerplate. It doesn't matter when running tests individually or for tests > with a lot of individual tests like the main regression tests. But for lots of > the rest it is noisy. These days there are many more regression tests than > there used to be when the output was designed... Also, those steps typically run a lot faster than they did then (both software speedups, and most people use better hardware). We no longer need that output to reassure ourselves that progress is being made. > It seems we could [ combine printouts ] How about going further, and just not print *any* of this overhead stuff, except maybe the "running on port 51696 with PID 1156405" line (and I'm not too wedded to that)? It is only interesting if something fails. For that, maybe if there's a failure we could print "while running text-of-command-here" after the fact. It'd also be a good idea to start using "make -s" by default for the preparatory steps. regards, tom lane
Hi, On 2022-02-21 12:05:42 -0500, Tom Lane wrote: > Also, those steps typically run a lot faster than they did then > (both software speedups, and most people use better hardware). > We no longer need that output to reassure ourselves that progress > is being made. Indeed. > > It seems we could [ combine printouts ] > > How about going further, and just not print *any* of this overhead > stuff +1 > except maybe the "running on port 51696 with PID 1156405" line (and I'm not > too wedded to that)? We still have a few issues with ports conflicts on windows. We should really consider just desupporting all windows versions without unix socket support. But until then it seems useful to be able to figure out random failures. > It'd also be a good idea to start using "make -s" by default for the > preparatory steps. For the temp-install and checkprep targets, or for something else? That'd probably just be a few @'s in front of rm, mkdir? One thing with temp-install that's been bothering me is that it often hides compilation failures inside install.log, and that it ends up building contrib with -j1. It'd be a bit of redundant work, but perhaps we should make it depend on a top-directory world-bin? This reminds me that a while ago I added the below to my Makefile.custom, to avoid all the "Nothing to be done for xyz" noise. Not sure if it bothers others as much as me... # Targets that don't require building anything cause make to say "make: # Nothing to be done for 'target'." That's OK if it happens once at the # top-level, but distracting for recursive make invocations of targets like # distprep, generated-headers-symlinks etc. # # The message can be silenced by depending on another target that always does # something. That works better than adding recipes to each of these common # targets, because in some makefiles they have their own recipses, triggering # warnings. # # To avoid repetition, use foreach/eval/call to create the dependencies, and a prefix # rule for the recipe. # # @ silences, : is the shell do-nothing command. %.silence: @: define create_silence_target $(1): | $(1).silence endef $(foreach t,$(standard_targets) $(standard_always_targets) generated-header-symlinks,$(eval $(call create_silence_target,$(t)))) Greetings, Andres Freund
Andres Freund <andres@anarazel.de> writes: > On 2022-02-21 12:05:42 -0500, Tom Lane wrote: >> except maybe the "running on port 51696 with PID 1156405" line (and I'm not >> too wedded to that)? > We still have a few issues with ports conflicts on windows. We should really > consider just desupporting all windows versions without unix socket > support. But until then it seems useful to be able to figure out random > failures. Yeah, also it seems useful to distinguish installcheck and check cases. With context removed, it might not make as much sense as it does today, so I'd vote for adding a word or two. Perhaps the output could look like test postmaster started on port 51696 with PID 1156405 "testname ... ok" lines here test postmaster stopped "all tests passed" or "n tests passed" here >> It'd also be a good idea to start using "make -s" by default for the >> preparatory steps. > For the temp-install and checkprep targets, or for something else? That'd > probably just be a few @'s in front of rm, mkdir? Well, all that stuff is interesting only in case of a failure. > One thing with temp-install that's been bothering me is that it often hides > compilation failures inside install.log, Yeah, I've run into that too --- even if there's no failure, you'll never see any compiler warnings. Perhaps if we started using "make -s" we'd not need install.log at all, and could just let errors/warnings spew to stderr where the user would see 'em. regards, tom lane
Hi, On 2022-02-21 12:40:18 -0500, Tom Lane wrote: > Andres Freund <andres@anarazel.de> writes: > > On 2022-02-21 12:05:42 -0500, Tom Lane wrote: > >> except maybe the "running on port 51696 with PID 1156405" line (and I'm not > >> too wedded to that)? > > > We still have a few issues with ports conflicts on windows. We should really > > consider just desupporting all windows versions without unix socket > > support. But until then it seems useful to be able to figure out random > > failures. > > Yeah, also it seems useful to distinguish installcheck and check cases. > With context removed, it might not make as much sense as it does today, > so I'd vote for adding a word or two. Perhaps the output could > look like > > test postmaster started on port 51696 with PID 1156405 > "testname ... ok" lines here > test postmaster stopped > "all tests passed" or "n tests passed" here I'm all for something minimal like this. I guess we could just try to go for a tap compatible output, but it'd probably be too annoying to not see the test name when it starts. Perhaps we could still take a page out of tap's book, and denote non-test output lines by starting with an #, so it's visually obvious what are lines about tests? It'd probably not be a good idea to backpatch this, even if it'd be nice to have at least semi-consistent formatting across branches? Too likely somebody built scripts depending on the old format? > >> It'd also be a good idea to start using "make -s" by default for the > >> preparatory steps. > > > For the temp-install and checkprep targets, or for something else? That'd > > probably just be a few @'s in front of rm, mkdir? > > Well, all that stuff is interesting only in case of a failure. > > One thing with temp-install that's been bothering me is that it often hides > > compilation failures inside install.log, > > Yeah, I've run into that too --- even if there's no failure, you'll > never see any compiler warnings. Perhaps if we started using > "make -s" we'd not need install.log at all, and could just let > errors/warnings spew to stderr where the user would see 'em. WFM. I'd still like to address the issue of building contrib with -j1 (due to checkprep ending up building contrib). But we can do that separately too. I assume we'd want to do this in all branches? Greetings, Andres Freund
On 2/21/22 12:31, Andres Freund wrote: > > We still have a few issues with ports conflicts on windows. We should really > consider just desupporting all windows versions without unix socket > support. But until then it seems useful to be able to figure out random > failures. > I'm pretty sure all my Windows machines with buildfarm animals are sufficiently modern except the XP machine, which only builds release 10 nowadays. What's involved in moving to require Unix socket support? cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Hi, On 2022-02-22 08:55:23 -0500, Andrew Dunstan wrote: > I'm pretty sure all my Windows machines with buildfarm animals are > sufficiently modern except the XP machine, which only builds release 10 > nowadays. Cool. > What's involved in moving to require Unix socket support? It works today (the CI scripts use it on windows for example). But it's awkward because make_temp_sockdir() defaults to /tmp/ if TMPDIR isn't set. Which it is not by default on windows. There's PG_REGRESS_SOCK_DIR, which kind of works for the msvc build, because pg_regress tests aren't run concurrently (whereas tap tests can be run concurrently with PROVE_FLAGS-j). I think we just make make_temp_sockdir() a tad smarter. Perhaps by lifting the code in src/bin/psql/command.c:do_edit() to src/port/path.c or such? Greetings, Andres Freund
On 2/22/22 17:06, Andres Freund wrote: >> What's involved in moving to require Unix socket support? > It works today (the CI scripts use it on windows for example). > > But it's awkward because make_temp_sockdir() defaults to /tmp/ if TMPDIR isn't > set. Which it is not by default on windows. There's PG_REGRESS_SOCK_DIR, which > kind of works for the msvc build, because pg_regress tests aren't run > concurrently (whereas tap tests can be run concurrently with > PROVE_FLAGS-j). > > I think we just make make_temp_sockdir() a tad smarter. Perhaps by lifting the > code in src/bin/psql/command.c:do_edit() to src/port/path.c or such? > +1 for doing that. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com