Thread: [HACKERS] [PATCH] Reduce src/test/recovery verbosity
Hi all Trivial patch to change 'diag' to 'note' in TAP tests in src/test/recovery attached. It'll reduce the test output a little. Those lines are there so you can tell what the test was doing before it does something if it fails at a setup step, or to provide extra diagnostic data, so they're retained, just muted to logs. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Attachment
On 3/22/17 03:18, Craig Ringer wrote: > Trivial patch to change 'diag' to 'note' in TAP tests in > src/test/recovery attached. > > It'll reduce the test output a little. Committed, and also done the same in src/test/ssl/. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 29 March 2017 at 08:57, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote: > On 3/22/17 03:18, Craig Ringer wrote: >> Trivial patch to change 'diag' to 'note' in TAP tests in >> src/test/recovery attached. >> >> It'll reduce the test output a little. > > Committed, and also done the same in src/test/ssl/. Thanks. It won't do a ton of good unless we reduce the default TAP verbosity level too, but it's a start and will make things work better if/when we do that. -- Craig Ringer http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On Wed, Mar 29, 2017 at 12:37 PM, Craig Ringer <craig@2ndquadrant.com> wrote: > On 29 March 2017 at 08:57, Peter Eisentraut > <peter.eisentraut@2ndquadrant.com> wrote: >> On 3/22/17 03:18, Craig Ringer wrote: >>> Trivial patch to change 'diag' to 'note' in TAP tests in >>> src/test/recovery attached. >>> >>> It'll reduce the test output a little. >> >> Committed, and also done the same in src/test/ssl/. > > Thanks. It won't do a ton of good unless we reduce the default TAP > verbosity level too, but it's a start and will make things work better > if/when we do that. [hijack] src/bin/pg_dump and src/test/modules/test_pgdump generate too much output. If we could get tests to only print the final result, like how many tests done and how many have passed, things would be much friendlier. [/hijack] -- Michael
On 3/28/17 23:42, Michael Paquier wrote: > src/bin/pg_dump and src/test/modules/test_pgdump generate too much > output. If we could get tests to only print the final result, like how > many tests done and how many have passed, things would be much > friendlier. There are options to change the verbosity of things. I don't think I would like changing the default verbosity. At least the precedent from the pg_regress type tests is that there should be some action on the screen as things run. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: [PATCH] Reduce src/test/recovery verbosity
From
ilmari@ilmari.org (Dagfinn Ilmari Mannsåker)
Date:
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes: > On 3/28/17 23:42, Michael Paquier wrote: >> src/bin/pg_dump and src/test/modules/test_pgdump generate too much >> output. If we could get tests to only print the final result, like how >> many tests done and how many have passed, things would be much >> friendlier. > > There are options to change the verbosity of things. > > I don't think I would like changing the default verbosity. At least the > precedent from the pg_regress type tests is that there should be some > action on the screen as things run. Non-verbose prove still lists each test script, it just doesn't list each individual test within the script. -- "A disappointingly low fraction of the human race is,at any given time, on fire." - Stig Sandbeck Mathisen
* Dagfinn Ilmari Mannsåker (ilmari@ilmari.org) wrote: > Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes: > > > On 3/28/17 23:42, Michael Paquier wrote: > >> src/bin/pg_dump and src/test/modules/test_pgdump generate too much > >> output. If we could get tests to only print the final result, like how > >> many tests done and how many have passed, things would be much > >> friendlier. > > > > There are options to change the verbosity of things. > > > > I don't think I would like changing the default verbosity. At least the > > precedent from the pg_regress type tests is that there should be some > > action on the screen as things run. > > Non-verbose prove still lists each test script, it just doesn't list > each individual test within the script. I agree that it'd be better to just show the per-script results rather than every little test result because it's just too much. If there's a way to change the verbosity for just those scripts, I'd be happy to do that, if we're unable to agree on reducing it across the board.. Thanks! Stephen
Stephen Frost wrote: > * Dagfinn Ilmari Mannsåker (ilmari@ilmari.org) wrote: > > Non-verbose prove still lists each test script, it just doesn't list > > each individual test within the script. > > I agree that it'd be better to just show the per-script results rather > than every little test result because it's just too much. Well, for some test files a single test could take half a minute. A dozen lines taking in total about a minute seems fine to me. The problem I see is a (pg_dump I think) script that has over 500 tests. > If there's a way to change the verbosity for just those scripts, I'd be > happy to do that, if we're unable to agree on reducing it across the > board.. I'd rather silence only scripts that are overly verbose. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
* Alvaro Herrera (alvherre@2ndquadrant.com) wrote: > Stephen Frost wrote: > > * Dagfinn Ilmari Mannsåker (ilmari@ilmari.org) wrote: > > > > Non-verbose prove still lists each test script, it just doesn't list > > > each individual test within the script. > > > > I agree that it'd be better to just show the per-script results rather > > than every little test result because it's just too much. > > Well, for some test files a single test could take half a minute. A > dozen lines taking in total about a minute seems fine to me. The > problem I see is a (pg_dump I think) script that has over 500 tests. Not sure that I see that as a "problem". :) > > If there's a way to change the verbosity for just those scripts, I'd be > > happy to do that, if we're unable to agree on reducing it across the > > board.. > > I'd rather silence only scripts that are overly verbose. I'm fine with that, but how? Thanks! Stephen
On Thu, Mar 30, 2017 at 4:08 AM, Stephen Frost <sfrost@snowman.net> wrote: > * Alvaro Herrera (alvherre@2ndquadrant.com) wrote: >> Stephen Frost wrote: >> > * Dagfinn Ilmari Mannsåker (ilmari@ilmari.org) wrote: >> >> > > Non-verbose prove still lists each test script, it just doesn't list >> > > each individual test within the script. >> > >> > I agree that it'd be better to just show the per-script results rather >> > than every little test result because it's just too much. >> >> Well, for some test files a single test could take half a minute. A >> dozen lines taking in total about a minute seems fine to me. The >> problem I see is a (pg_dump I think) script that has over 500 tests. > > Not sure that I see that as a "problem". :) > >> > If there's a way to change the verbosity for just those scripts, I'd be >> > happy to do that, if we're unable to agree on reducing it across the >> > board.. >> >> I'd rather silence only scripts that are overly verbose. > > I'm fine with that, but how? The important point to me is not to show the tests that passed, it is to show the tests that are failing when running them. So I would suggest to just remove the --verbose flag in PROVE_FLAGS. If you do so, the test of pg_dump would show up like that, printing as well a count number while running: t/001_basic.pl ......... ok t/002_pg_dump.pl ....... ok t/010_dump_connstr.pl .. ok And failures would still show up, here is an example breaking manually a recovery test: t/001_stream_rep.pl .................. 1/28 # Failed test 'read-only queries on standby 2' # at t/001_stream_rep.pl line 59. # got: '3' # expected: '0' # testing connection parameter "target_session_attrs" # switching to physical replication slot t/001_stream_rep.pl .................. 11/28 # enabling hot_standby_feedback t/001_stream_rep.pl .................. 15/28 # doing some work to advance xmin # new xmin 1442, old xmin 558 t/001_stream_rep.pl .................. 19/28 # new xmin 1553, old xmin 558 # disabling hot_standby_feedback t/001_stream_rep.pl .................. 23/28 # re-enabling hot_standby_feedback and disabling while stopped t/001_stream_rep.pl .................. 28/28 # Looks like you failed 1 test of 28. t/001_stream_rep.pl .................. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/28 subtests t/002_archiving.pl ................... ok t/003_recovery_targets.pl ............ ok t/004_timeline_switch.pl ............. ok Attached is a patch. -- Michael
Attachment
Michael, * Michael Paquier (michael.paquier@gmail.com) wrote: > On Thu, Mar 30, 2017 at 4:08 AM, Stephen Frost <sfrost@snowman.net> wrote: > >> > If there's a way to change the verbosity for just those scripts, I'd be > >> > happy to do that, if we're unable to agree on reducing it across the > >> > board.. > >> > >> I'd rather silence only scripts that are overly verbose. > > > > I'm fine with that, but how? > > The important point to me is not to show the tests that passed, it is > to show the tests that are failing when running them. So I would > suggest to just remove the --verbose flag in PROVE_FLAGS. If you do > so, the test of pg_dump would show up like that, printing as well a > count number while running: Right, but you can't do that for just pg_dump. I'd be fine with removing --verbose globally, as your patch does, but there was some argument that we then would have long 'quiet' periods. I haven't had a chance to go test if that's really the case yet though. Thanks! Stephen
On Thu, Mar 30, 2017 at 9:52 PM, Stephen Frost <sfrost@snowman.net> wrote: > Michael, > > * Michael Paquier (michael.paquier@gmail.com) wrote: >> On Thu, Mar 30, 2017 at 4:08 AM, Stephen Frost <sfrost@snowman.net> wrote: >> >> > If there's a way to change the verbosity for just those scripts, I'd be >> >> > happy to do that, if we're unable to agree on reducing it across the >> >> > board.. >> >> >> >> I'd rather silence only scripts that are overly verbose. >> > >> > I'm fine with that, but how? >> >> The important point to me is not to show the tests that passed, it is >> to show the tests that are failing when running them. So I would >> suggest to just remove the --verbose flag in PROVE_FLAGS. If you do >> so, the test of pg_dump would show up like that, printing as well a >> count number while running: > > Right, but you can't do that for just pg_dump. > > I'd be fine with removing --verbose globally, as your patch does, but > there was some argument that we then would have long 'quiet' periods. > I haven't had a chance to go test if that's really the case yet though. I don't see much the point to have --verbose enabled by default, but if that's the consensus I have nothing better to propose than adding something like that to the command launching the prove command: $(if $(PG_PROVE_FLAGS),$(PG_PROVE_FLAGS),--verbose) And then it is possible to enforce the flag for noisy tests. -- Michael
Stephen Frost wrote: > I'd be fine with removing --verbose globally, as your patch does, but > there was some argument that we then would have long 'quiet' periods. > I haven't had a chance to go test if that's really the case yet though. Michael said that a running counter was displayed, which perhaps is enough of a progress indicator. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 30 March 2017 at 20:57, Michael Paquier <michael.paquier@gmail.com> wrote: > I don't see much the point to have --verbose enabled by default, but > if that's the consensus I have nothing better to propose than adding > something like that to the command launching the prove command: $(if > $(PG_PROVE_FLAGS),$(PG_PROVE_FLAGS),--verbose) > And then it is possible to enforce the flag for noisy tests. That's what I'd like to go for. Even default to it, so long as I can override on command line. -- Craig Ringer http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Stephen Frost <sfrost@snowman.net> writes:
> I'd be fine with removing --verbose globally, as your patch does, but
> there was some argument that we then would have long 'quiet' periods.
> I haven't had a chance to go test if that's really the case yet though.
I've been running it like this lately:
make -s check-world -j4 PROVE_FLAGS='-j4 --quiet --nocolor --nocount'
and it is still pretty noisy ;-). The only place where it sits for
more than a couple of seconds without printing anything is at the very
start, which I believe to be the initial "make temp-install" step,
which would be unaffected by prove verbosity anyway.
So I'd be +1 for just removing --verbose by default. Anybody who really
wants it can put it back via PROVE_FLAGS.
regards, tom lane
Tom, all, * Tom Lane (tgl@sss.pgh.pa.us) wrote: > Stephen Frost <sfrost@snowman.net> writes: > > I'd be fine with removing --verbose globally, as your patch does, but > > there was some argument that we then would have long 'quiet' periods. > > I haven't had a chance to go test if that's really the case yet though. > > I've been running it like this lately: > > make -s check-world -j4 PROVE_FLAGS='-j4 --quiet --nocolor --nocount' > > and it is still pretty noisy ;-). The only place where it sits for > more than a couple of seconds without printing anything is at the very > start, which I believe to be the initial "make temp-install" step, > which would be unaffected by prove verbosity anyway. > > So I'd be +1 for just removing --verbose by default. Anybody who really > wants it can put it back via PROVE_FLAGS. Yeah, I'm feeling the same way on this, and it'd reduce the size of the logs on the buildfarm too, which I believe is a good thing. Unless people wish to object, I'll use Michael's patch to remove --verbose from the top level tomorrow. Thanks! Stephen
On 31 March 2017 at 04:29, Stephen Frost <sfrost@snowman.net> wrote: > Unless people wish to object, I'll use Michael's patch to remove > --verbose from the top level tomorrow. Sounds good. Maybe add To get more detailed output from tests set PROVE_FLAGS='--verbose' or examine the detailed test output in tmp_check/logs. to src/test/perl/README too? -- Craig Ringer http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Greetings, * Craig Ringer (craig@2ndquadrant.com) wrote: > On 31 March 2017 at 04:29, Stephen Frost <sfrost@snowman.net> wrote: > > > Unless people wish to object, I'll use Michael's patch to remove > > --verbose from the top level tomorrow. > > Sounds good. > > Maybe add > > To get more detailed output from tests set PROVE_FLAGS='--verbose' or examine > the detailed test output in tmp_check/logs. > > to src/test/perl/README too? Committed, with those additions. Thanks! Stephen
On Wed, Apr 5, 2017 at 12:39 AM, Stephen Frost <sfrost@snowman.net> wrote: > Committed, with those additions. Thanks for the commit. The final result is nice. -- Michael