Re: [PATCH v4] Add \warn to psql - Mailing list pgsql-hackers

From Tom Lane
Subject Re: [PATCH v4] Add \warn to psql
Date
Msg-id 11038.1562098250@sss.pgh.pa.us
Whole thread Raw
In response to Re: [PATCH v4] Add \warn to psql  (David Fetter <david@fetter.org>)
Responses Re: [PATCH v4] Add \warn to psql  (Fabien COELHO <coelho@cri.ensmp.fr>)
Re: [PATCH v4] Add \warn to psql  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
David Fetter <david@fetter.org> writes:
> [ v7-0001-Add-warn-to-psql.patch ]

I took a look at this.  I have no quibble with the proposed feature,
and the implementation is certainly simple enough.  But I'm unconvinced
about the proposed test scaffolding.  Spinning up a new PG instance is a
*hell* of a lot of overhead to pay for testing something that could be
tested as per attached.  Admittedly, the attached doesn't positively
prove which pipe each output string went down, but that does not strike
me as a concern large enough to justify adding a TAP test for.

I'd be happier about adding TAP infrastructure if it looked like it'd
be usable to test some of the psql areas that are unreachable by the
existing test methodology, particularly tab-complete.c and prompt.c.
But I don't see anything here that looks like it'll work for that.

I don't like what you did to command_checks_all, either --- it could
hardly say "bolted on after the fact" more clearly if you'd written
that in <blink><red> text.  If we need an input-stream argument,
let's just add it in a rational place and adjust the callers.
There aren't that many of 'em, nor has the subroutine been around
all that long.

            regards, tom lane

diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out
index 4bcf0cc..9b4060f 100644
--- a/src/test/regress/expected/psql.out
+++ b/src/test/regress/expected/psql.out
@@ -4180,6 +4180,29 @@ drop table psql_serial_tab;
 \pset format aligned
 \pset expanded off
 \pset border 1
+-- \echo and allied features
+\echo this is a test
+this is a test
+\echo -n this is a test without newline
+this is a test without newline\echo more test
+more test
+\set foo bar
+\echo foo = :foo
+foo = bar
+\qecho this is a test
+this is a test
+\qecho -n this is a test without newline
+this is a test without newline\qecho more test
+more test
+\qecho foo = :foo
+foo = bar
+\warn this is a test
+this is a test
+\warn -n this is a test without newline
+this is a test without newline\warn more test
+more test
+\warn foo = :foo
+foo = bar
 -- tests for \if ... \endif
 \if true
   select 'okay';
diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql
index 26f436a..2bae6c5 100644
--- a/src/test/regress/sql/psql.sql
+++ b/src/test/regress/sql/psql.sql
@@ -771,6 +771,24 @@ drop table psql_serial_tab;
 \pset expanded off
 \pset border 1

+-- \echo and allied features
+
+\echo this is a test
+\echo -n this is a test without newline
+\echo more test
+\set foo bar
+\echo foo = :foo
+
+\qecho this is a test
+\qecho -n this is a test without newline
+\qecho more test
+\qecho foo = :foo
+
+\warn this is a test
+\warn -n this is a test without newline
+\warn more test
+\warn foo = :foo
+
 -- tests for \if ... \endif

 \if true

pgsql-hackers by date:

Previous
From: Alvaro Herrera
Date:
Subject: Re: [PATCH v5] Show detailed table persistence in \dt+
Next
From: Tom Lane
Date:
Subject: Re: [PATCH v5] Show detailed table persistence in \dt+