pg.ddx.io  pgsql-committers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Andres Freund <andres@anarazel.de>
To: pgsql-committers@lists.postgresql.org
Subject: pgsql: tests: BackgroundPsql: Fix potential for lost errors on windows
Date: Wed, 19 Feb 2025 17:45:55 +0000
Message-ID: <E1tko8y-000Adj-0g@gemulon.postgresql.org> (raw)

tests: BackgroundPsql: Fix potential for lost errors on windows

This addresses various corner cases in BackgroundPsql:

- On windows stdout and stderr may arrive out of order, leading to errors not
  being reported, or attributed to the wrong statement.

  To fix, emit the "query-separation banner" on both stdout and stderr and
  wait for both.

- Very occasionally the "query-separation banner" would not get removed, because
  we waited until the banner arrived, but then replaced the banner plus
  newline.

  To fix, wait for banner and newline.

- For interactive psql replacing $banner\n is not sufficient, interactive psql
  outputs \r\n.

- For interactive psql, where commands are echoed to stdout, the \echo
  command, rather than its output, would be matched.

  This would sometimes lead to output from the prior query, or wait_connect(),
  being returned in the next command.

  This also affected wait_connect(), leading to sometimes sending queries to
  psql before the connection actually was established.

While debugging these issues I also found that it's hard to know whether a
query separation banner was attributed to the right query. Make that easier by
counting the queries each BackgroundPsql instance has emitted and include the
number in the banner.

Also emit psql stdout/stderr in query() and wait_connect() as Test::More
notes, without that it's rather hard to debug some issues in CI and buildfarm.

As this can cause issues not just to-be-added tests, but also existing ones,
backpatch the fix to all supported versions.

Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/wmovm6xcbwh7twdtymxuboaoarbvwj2haasd3sikzlb3dkgz76@n45rzycluzft
Backpatch-through: 13

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/8b886a4e34886bf2d1dd4f5e7d95f4ddadc3afa8

Modified Files
--------------
src/test/perl/PostgreSQL/Test/BackgroundPsql.pm | 75 ++++++++++++++++++++-----
1 file changed, 60 insertions(+), 15 deletions(-)



view thread (6+ messages)  latest in thread

Message-ID: <E1tko8y-000Adj-0g@gemulon.postgresql.org>
Permalink:  ../E1tko8y-000Adj-0g@gemulon.postgresql.org/
Also on:    postgresql.org/message-id/E1tko8y-000Adj-0g@gemulon.postgresql.org

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-committers@postgresql.org
  Cc: andres@anarazel.de, pgsql-committers@lists.postgresql.org
  Subject: Re: pgsql: tests: BackgroundPsql: Fix potential for lost errors on windows
  In-Reply-To: <E1tko8y-000Adj-0g@gemulon.postgresql.org>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox