On 2026-04-07 Tu 9:15 AM, Peter Eisentraut wrote:
On 02.04.26 14:25, Andrew Dunstan wrote:
On 2026-03-20 Fr 2:13 PM, Corey Huinker wrote:
Shortened it using your example run.
Glad I was able to help.
> I can't wait to use this.
Me too :) I've also added the Author/Reviewed-By/Discussion
footers to
the commits to make the committers job easier.
I've held off on doing that in my proposed commits so as not to be
presumptuous, but I can see where having it available would be a
convenience for the committer. This will be a good test of that.
Applies clean to master, passes tests. Ship it.
Committed with minor tidy up. The main change was to add a @CARP_NOT
setting in Utils.pm, so that croak() would look back past Cluster.pm
to the TAP script caller.
I would like to register a vote against this new behavior:
pg_regress: Include diffs in TAP output
When pg_regress fails it is often tedious to find the actual diffs,
especially in CI where you must navigate a file browser. Emit the
first
80 lines of the combined regression.diffs as TAP diagnostics so the
failure reason is visible directly in the test output.
I find this annoying.
What happens, and this is admittedly my particular experience, is that
the diff lines are wider than the terminal width, and so 80 lines in
the file might end up being 200 lines on screen, and then the summary
of the test failure disappears from the screen and the diff output is
garbled and useless, and so the whole output is now less useful than
before.
I could see this maybe being useful if the entire diff file is, say,
less than 50 lines. But I don't see how seeing a truncated diff by
default can be useful.
The commit message makes reference to "especially on CI". Maybe this
new behavior should be triggered by being on CI, or the output not
being a terminal, or something like that.
How about something like this, which would only trigger the behaviour if
an environment variable is set. Also adds that env setting to
cirrus.tasks.yml.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml
index a22cef063f3..c02dbae8e30 100644
--- a/.cirrus.tasks.yml
+++ b/.cirrus.tasks.yml
@@ -30,6 +30,7 @@ env:
PGCTLTIMEOUT: 120 # avoids spurious failures during parallel tests
TEMP_CONFIG: ${CIRRUS_WORKING_DIR}/src/tools/ci/pg_ci_base.conf
PG_TEST_EXTRA: kerberos ldap ssl libpq_encryption load_balance oauth
+ PG_REGRESS_DIFF_EXTRACT: 1
# Postgres config args for the meson builds, shared between all meson tasks
# except the 'SanityCheck' task
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 9a918156437..a5e9d25b1fb 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -1549,16 +1549,9 @@ results_differ(const char *testname, const char *resultsfile, const char *defaul
"diff %s \"%s\" \"%s\" >> \"%s\"",
pretty_diff_opts, best_expect_file, resultsfile, difffilename);
run_diff(cmd, difffilename);
-
- /*
- * Reopen the file for reading to emit the diff as TAP diagnostics. We
- * can't keep the file open while diff appends to it, because on
- * Windows the file lock prevents diff from writing.
- */
- difffile = fopen(difffilename, "r");
}
- if (difffile)
+ if (difffile && getenv("PG_REGRESS_DIFF_EXTRACT"))
{
/*
* In case of a crash the diff can be huge and all of the subsequent
@@ -1574,6 +1567,9 @@ results_differ(const char *testname, const char *resultsfile, const char *defaul
const int max_diff_lines = 80;
char line[1024];
+ /* Reopen the file for reading to emit the diff as TAP diagnostics. */
+ difffile = fopen(difffilename, "r");
+
fseek(difffile, startpos, SEEK_SET);
while (nlines < max_diff_lines &&
fgets(line, sizeof(line), difffile))
