Обсуждение: making pg_regress less noisy by removing boilerplate

Поиск
Список
Период
Сортировка

making pg_regress less noisy by removing boilerplate

От
Andres Freund
Дата:
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



Re: making pg_regress less noisy by removing boilerplate

От
Tom Lane
Дата:
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



Re: making pg_regress less noisy by removing boilerplate

От
Andres Freund
Дата:
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



Re: making pg_regress less noisy by removing boilerplate

От
Tom Lane
Дата:
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



Re: making pg_regress less noisy by removing boilerplate

От
Andres Freund
Дата:
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



Re: making pg_regress less noisy by removing boilerplate

От
Andrew Dunstan
Дата:
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




Re: making pg_regress less noisy by removing boilerplate

От
Andres Freund
Дата:
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



Re: making pg_regress less noisy by removing boilerplate

От
Andrew Dunstan
Дата:
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