From ebd49928215e3854d91167e798949a75b34958d0 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sat, 27 Jul 2019 15:59:57 -0400 Subject: [PATCH] Don't drop NOTICE messages in isolation tests. For its entire existence, isolationtester.c has forced client_min_messages to WARNING, but that seems like a very poor choice of test design. It should be up to individual test scripts to manage whether they emit notices and to ensure that the results are stable. (There were no NOTICE messages in the original set of isolation tests, so this was certainly dead code when committed, but perhaps it was needed at some earlier point.) It's possible that the original motivation was due to platform-dependent variations in the timing of stdout vs. stderr output. That should be moot since commits 73bcb76b7/6eda3e9c2, but just in case, adjust isotesterNoticeProcessor to print to stdout not stderr. (stderr seems like the wrong thing anyway: it should be for error printouts not expected test output.) Testing shows that the notices in insert-conflict-specconflict are indeed a bit timing-unstable on very slow machines, so hide them; maybe we can improve that later. Also, make the notices in plpgsql-toast a bit less verbose than the original code would've had them. Discussion: https://postgr.es/m/14616.1564251339@sss.pgh.pa.us --- src/test/isolation/expected/plpgsql-toast.out | 15 ++++++++++----- src/test/isolation/isolationtester.c | 14 +------------- .../specs/insert-conflict-specconflict.spec | 6 ++++++ src/test/isolation/specs/plpgsql-toast.spec | 10 +++++----- 4 files changed, 22 insertions(+), 23 deletions(-) diff --git a/src/test/isolation/expected/plpgsql-toast.out b/src/test/isolation/expected/plpgsql-toast.out index 43411533f1d..fc557da5e77 100644 --- a/src/test/isolation/expected/plpgsql-toast.out +++ b/src/test/isolation/expected/plpgsql-toast.out @@ -22,7 +22,7 @@ do $$ delete from test1; commit; perform pg_advisory_lock(1); - raise notice 'x = %', x; + raise notice 'length(x) = %', length(x); end; $$; @@ -35,6 +35,7 @@ step unlock: pg_advisory_unlock t +s1: NOTICE: length(x) = 6000 step assign1: <... completed> starting permutation: lock assign2 vacuum unlock @@ -59,7 +60,7 @@ do $$ delete from test1; commit; perform pg_advisory_lock(1); - raise notice 'x = %', x; + raise notice 'length(x) = %', length(x); end; $$; @@ -72,6 +73,7 @@ step unlock: pg_advisory_unlock t +s1: NOTICE: length(x) = 6000 step assign2: <... completed> starting permutation: lock assign3 vacuum unlock @@ -97,7 +99,7 @@ do $$ delete from test1; commit; perform pg_advisory_lock(1); - raise notice 'r = %', r; + raise notice 'length(r) = %', length(r::text); end; $$; @@ -110,6 +112,7 @@ step unlock: pg_advisory_unlock t +s1: NOTICE: length(r) = 6004 step assign3: <... completed> starting permutation: lock assign4 vacuum unlock @@ -134,7 +137,7 @@ do $$ delete from test1; commit; perform pg_advisory_lock(1); - raise notice 'r = %', r; + raise notice 'length(r) = %', length(r::text); end; $$; @@ -147,6 +150,7 @@ step unlock: pg_advisory_unlock t +s1: NOTICE: length(r) = 6004 step assign4: <... completed> starting permutation: lock assign5 vacuum unlock @@ -173,7 +177,7 @@ do $$ delete from test1; commit; perform pg_advisory_lock(1); - raise notice 'r = %', r; + raise notice 'length(r) = %', length(r::text); end; $$; @@ -186,4 +190,5 @@ step unlock: pg_advisory_unlock t +s1: NOTICE: length(r) = 6002 step assign5: <... completed> diff --git a/src/test/isolation/isolationtester.c b/src/test/isolation/isolationtester.c index 2f039b83eea..6ab19b1e597 100644 --- a/src/test/isolation/isolationtester.c +++ b/src/test/isolation/isolationtester.c @@ -187,18 +187,6 @@ main(int argc, char **argv) blackholeNoticeProcessor, NULL); - /* - * Suppress NOTIFY messages, which otherwise pop into results at odd - * places. - */ - res = PQexec(conns[i], "SET client_min_messages = warning;"); - if (PQresultStatus(res) != PGRES_COMMAND_OK) - { - fprintf(stderr, "message level setup failed: %s", PQerrorMessage(conns[i])); - exit(1); - } - PQclear(res); - /* Get the backend pid for lock wait checking. */ res = PQexec(conns[i], "SELECT pg_catalog.pg_backend_pid()"); if (PQresultStatus(res) == PGRES_TUPLES_OK) @@ -899,7 +887,7 @@ printResultSet(PGresult *res) static void isotesterNoticeProcessor(void *arg, const char *message) { - fprintf(stderr, "%s: %s", (char *) arg, message); + printf("%s: %s", (char *) arg, message); } /* notice processor, hides the message */ diff --git a/src/test/isolation/specs/insert-conflict-specconflict.spec b/src/test/isolation/specs/insert-conflict-specconflict.spec index 3a70484fc29..2d16fae1b1b 100644 --- a/src/test/isolation/specs/insert-conflict-specconflict.spec +++ b/src/test/isolation/specs/insert-conflict-specconflict.spec @@ -51,6 +51,9 @@ session "s1" setup { SET default_transaction_isolation = 'read committed'; + -- on very slow machines, the notices come out in unpredictable order, + -- so hide them + SET client_min_messages = warning; SET spec.session = 1; } step "s1_begin" { BEGIN; } @@ -61,6 +64,9 @@ session "s2" setup { SET default_transaction_isolation = 'read committed'; + -- on very slow machines, the notices come out in unpredictable order, + -- so hide them + SET client_min_messages = warning; SET spec.session = 2; } step "s2_begin" { BEGIN; } diff --git a/src/test/isolation/specs/plpgsql-toast.spec b/src/test/isolation/specs/plpgsql-toast.spec index e6228c9ef68..fe7090addbb 100644 --- a/src/test/isolation/specs/plpgsql-toast.spec +++ b/src/test/isolation/specs/plpgsql-toast.spec @@ -40,7 +40,7 @@ do $$ delete from test1; commit; perform pg_advisory_lock(1); - raise notice 'x = %', x; + raise notice 'length(x) = %', length(x); end; $$; } @@ -56,7 +56,7 @@ do $$ delete from test1; commit; perform pg_advisory_lock(1); - raise notice 'x = %', x; + raise notice 'length(x) = %', length(x); end; $$; } @@ -73,7 +73,7 @@ do $$ delete from test1; commit; perform pg_advisory_lock(1); - raise notice 'r = %', r; + raise notice 'length(r) = %', length(r::text); end; $$; } @@ -89,7 +89,7 @@ do $$ delete from test1; commit; perform pg_advisory_lock(1); - raise notice 'r = %', r; + raise notice 'length(r) = %', length(r::text); end; $$; } @@ -107,7 +107,7 @@ do $$ delete from test1; commit; perform pg_advisory_lock(1); - raise notice 'r = %', r; + raise notice 'length(r) = %', length(r::text); end; $$; }