Re: pgsql: Make VACUUM accept 1 and 0 as a boolean value.

Andres Freund <andres@anarazel.de> writes:
> On 2019-05-20 14:09:40 -0400, Tom Lane wrote:
>> What I'd like, for both prove and pg_regress, is to print something
>> about failing tests and otherwise be quiet.  Simple redirection won't
>> do that.  Plus it'd be hard to fit that in with the case where you
>> don't want it to be quiet.

> The most annoying noise imo is the pg_upgrade test. The set -x and the
> fact that it resets MAKEFLAGS makes that pretty annoying.

Yeah.  I just experimented with running "make -s check-world >/dev/null"
and found that *all* of the useless chatter on stderr is the pg_upgrade
test's fault.  With the attached quick-hack patch, a successful run is
silent --- and if it fails, make tells you which directory it failed
in, which is enough to go find the problem.  So that's already a huge
usability improvement over where we are.

> 1) we still do an "open coded" install, regardless of dcae5faccab64 And
>    that's *in addition* to to the install from dcae5faccab64. That seems
>    pretty unnecessary.

Agreed, that could be worth improving.

> 2) We overwrite MAKELEVEL, MAKEFLAGS, breaking jobserver etc - that
>    shouldn't be required anymore if test.sh were to be changed to behave
>    like dcae5faccab64

Hmm.  AFAICS, that commit removed that code without any explanation of
why it was safe to remove it, so I'm unclear on whether it would be
safe to do likewise in test.sh.

> 3) The fact that src/bin/pg_upgrade/Makefile invokes test.sh with
>    MAKE=$(MAKE) triggers make, for reasons I do not yet understand, to
>    *disable* output synchronization. Which annoys the heck out of me,
>    because it makes parallel check output neigh unparsable.

Probably the same thing as your 2)?

> 4) set -x spews a lot of things

Yeah.  I'm definitely in favor of losing the set -x.  The other thing
I had to do below was to suppress "NOTICE:  database "regression" does
not exist, skipping".  The added createdb is a mighty expensive and
grotty way to do that, but I didn't immediately see a better one.

            regards, tom lane

diff --git a/src/bin/pg_upgrade/test.sh b/src/bin/pg_upgrade/test.sh
index 66e69b946b..15f98e29bc 100644
--- a/src/bin/pg_upgrade/test.sh
+++ b/src/bin/pg_upgrade/test.sh
@@ -22,7 +22,8 @@ unset MAKELEVEL
 standard_initdb() {
     # To increase coverage of non-standard segment size and group access
     # without increasing test runtime, run these tests with a custom setting.
-    "$1" -N --wal-segsize 1 -g
+    # Also, specify "-A trust" explicitly to suppress initdb's warning.
+    "$1" -N --wal-segsize 1 -g -A trust
     if [ -n "$TEMP_CONFIG" -a -r "$TEMP_CONFIG" ]
     then
         cat "$TEMP_CONFIG" >> "$PGDATA/postgresql.conf"
@@ -150,9 +151,6 @@ done
 EXTRA_REGRESS_OPTS="$EXTRA_REGRESS_OPTS --port=$PGPORT"
 export EXTRA_REGRESS_OPTS

-# enable echo so the user can see what is being executed
-set -x
-
 standard_initdb "$oldbindir"/initdb
 "$oldbindir"/pg_ctl start -l "$logdir/postmaster1.log" -o "$POSTMASTER_OPTS" -w

@@ -169,6 +167,9 @@ createdb "$dbname1" || createdb_status=$?
 createdb "$dbname2" || createdb_status=$?
 createdb "$dbname3" || createdb_status=$?

+# Create regression database explicitly to avoid noise from pg_regress
+createdb regression
+
 if "$MAKE" -C "$oldsrc" installcheck-parallel; then
     oldpgversion=`psql -X -A -t -d regression -c "SHOW server_version_num"`

@@ -256,10 +257,6 @@ esac
 pg_dumpall --no-sync -f "$temp_root"/dump2.sql || pg_dumpall2_status=$?
 pg_ctl -m fast stop

-# no need to echo commands anymore
-set +x
-echo
-
 if [ -n "$pg_dumpall2_status" ]; then
     echo "pg_dumpall of post-upgrade database cluster failed"
     exit 1