Time to change pg_regress diffs to unified by default?
Hi,
I personally, and I know of a bunch of other regular contributors, find
context diffs very hard to read. Besides general dislike, for things
like regression test output context diffs are just not well suited.
E.g. in
https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=prairiedog&dt=2017-04-06%2021%3A10%3A56&stg=check
the salient point (ERROR: 50 is outside the valid range for parameter "effective_io_concurrency" (0 .. 0))
is 130 lines into the diff, whereas it's right at the start in a unified diff.
Issues with one error that causes a lot of followup output changes are
really common in our regression suite.
I personally have PG_REGRESS_DIFF_OPTS set to -dU10, but that doesn't
help much analyzing buildfarm output.
Therefore I propose changing the defaults in pg_regress.c.
Greetings,
Andres Freund
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Apr 7, 2017 at 10:31 AM, Andres Freund <andres@anarazel.de> wrote:
Hi,
I personally, and I know of a bunch of other regular contributors, find
context diffs very hard to read. Besides general dislike, for things
like regression test output context diffs are just not well suited.
E.g. in
https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=prairiedog&dt=2017-04-06%2021%3A10%3A56&stg=check
the salient point (ERROR: 50 is outside the valid range for parameter "effective_io_concurrency" (0 .. 0))
is 130 lines into the diff, whereas it's right at the start in a unified diff.
Issues with one error that causes a lot of followup output changes are
really common in our regression suite.I personally have PG_REGRESS_DIFF_OPTS set to -dU10, but that doesn't
help much analyzing buildfarm output.Therefore I propose changing the defaults in pg_regress.c.
+1
So much easier to read.
--
Thomas Munro
http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 04/06/2017 06:31 PM, Andres Freund wrote:
Hi,
I personally, and I know of a bunch of other regular contributors, find
context diffs very hard to read. Besides general dislike, for things
like regression test output context diffs are just not well suited.
E.g. in
https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=prairiedog&dt=2017-04-06%2021%3A10%3A56&stg=check
the salient point (ERROR: 50 is outside the valid range for parameter "effective_io_concurrency" (0 .. 0))
is 130 lines into the diff, whereas it's right at the start in a unified diff.
Issues with one error that causes a lot of followup output changes are
really common in our regression suite.I personally have PG_REGRESS_DIFF_OPTS set to -dU10, but that doesn't
help much analyzing buildfarm output.Therefore I propose changing the defaults in pg_regress.c.
+1
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Andres Freund <andres@anarazel.de> writes:
I personally, and I know of a bunch of other regular contributors, find
context diffs very hard to read. Besides general dislike, for things
like regression test output context diffs are just not well suited.
Personally, I disagree completely. Unified diffs are utterly unreadable
for anything beyond trivial cases of small well-separated changes.
It's possible that regression failure diffs will usually fall into that
category, but I'm not convinced.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 4/6/17 18:31, Andres Freund wrote:
I personally have PG_REGRESS_DIFF_OPTS set to -dU10, but that doesn't
help much analyzing buildfarm output.Therefore I propose changing the defaults in pg_regress.c.
I think one problem is that diff -u is not as portable as diff -c. For
example, the HP-UX 11 man page of diff doesn't list it.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 04/06/2017 09:17 PM, Peter Eisentraut wrote:
On 4/6/17 18:31, Andres Freund wrote:
I personally have PG_REGRESS_DIFF_OPTS set to -dU10, but that doesn't
help much analyzing buildfarm output.Therefore I propose changing the defaults in pg_regress.c.
I think one problem is that diff -u is not as portable as diff -c. For
example, the HP-UX 11 man page of diff doesn't list it.
Ugh. I suppose we could run a test to see if it was available. If it
comes to that, I guess I could do a similar test in the buildfarm
client. Seems a bit like overkill, though.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 7 April 2017 at 10:31, Andres Freund <andres@anarazel.de> wrote:
Hi,
I personally, and I know of a bunch of other regular contributors, find
context diffs very hard to read. Besides general dislike, for things
like regression test output context diffs are just not well suited.
E.g. in
https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=prairiedog&dt=2017-04-06%2021%3A10%3A56&stg=check
the salient point (ERROR: 50 is outside the valid range for parameter "effective_io_concurrency" (0 .. 0))
is 130 lines into the diff, whereas it's right at the start in a unified diff.
Issues with one error that causes a lot of followup output changes are
really common in our regression suite.I personally have PG_REGRESS_DIFF_OPTS set to -dU10, but that doesn't
help much analyzing buildfarm output.Therefore I propose changing the defaults in pg_regress.c.
You mean people actually use those diffs? I've never done anything
apart from using <my favourite text comparison tool>. That way I can
reject and accept the changes as I wish, just by kicking the results
over to the expected results, or not, if there's a genuine mistake.
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Apr 06, 2017 at 07:01:32PM -0400, Tom Lane wrote:
Andres Freund <andres@anarazel.de> writes:
I personally, and I know of a bunch of other regular contributors, find
context diffs very hard to read. Besides general dislike, for things
like regression test output context diffs are just not well suited.Personally, I disagree completely. Unified diffs are utterly unreadable
for anything beyond trivial cases of small well-separated changes.It's possible that regression failure diffs will usually fall into that
category, but I'm not convinced.
For reading patches, I frequently use both formats. Overall, I perhaps read
unified 3/4 of the time and context 1/4 of the time.
For regression diffs, I use PG_REGRESS_DIFF_OPTS=-u and have never converted a
regression diff to context form. Hence, +1 for the proposed change.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
I think one problem is that diff -u is not as portable as diff -c. For
example, the HP-UX 11 man page of diff doesn't list it.
FWIW, I can confirm that HPUX 10.20's diff hasn't got it. That would
not affect gaur/pademelon, if we make this change, because I installed
GNU diffutils on that machine a decade or two ago. It might be a bigger
issue for the other HPUX critters though.
Some other data points:
* POSIX 2008 requires diff -u.
* SUS v2 (POSIX 1997) does not.
* My other pet dinosaur, prairiedog (macOS 10.4 something), has it.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: Noah Misch 2017-04-07 <20170407021431.GB2658646@tornado.leadboat.com>
I personally, and I know of a bunch of other regular contributors, find
context diffs very hard to read. Besides general dislike, for things
like regression test output context diffs are just not well suited.Personally, I disagree completely. Unified diffs are utterly unreadable
for anything beyond trivial cases of small well-separated changes.It's possible that regression failure diffs will usually fall into that
category, but I'm not convinced.For reading patches, I frequently use both formats. Overall, I perhaps read
unified 3/4 of the time and context 1/4 of the time.For regression diffs, I use PG_REGRESS_DIFF_OPTS=-u and have never converted a
regression diff to context form. Hence, +1 for the proposed change.
I've just had another case of horrible context diff from pg_regress.
I'd claim that regression diffs are particularly bad for context diffs
because one error will often trigger a whole chain of failures, which
will essentially make the whole file appear twice in the output,
whereas unified diffs would just put the original output and the error
right next to each other at the top. Having to scroll through a whole
.out file just to find "create extension; file not found" is very
inefficient.
It's nice that PG_REGRESS_DIFF_OPTS exists, but the diffs are often
coming from automated build logs where setting the variable after the
fact doesn't help.
Please consider the attached patch, extension packagers will thank
you.
Christoph
Attachments:
0001-Switch-pg_regress-to-output-unified-diffs-by-default.patchtext/x-diff; charset=us-asciiDownload
From a9b1ef089bbcc36dc65e4acff60ae3b83ecd06e3 Mon Sep 17 00:00:00 2001
From: Christoph Berg <christoph.berg@credativ.de>
Date: Thu, 22 Nov 2018 13:33:42 +0100
Subject: [PATCH] Switch pg_regress to output unified diffs by default
---
doc/src/sgml/regress.sgml | 2 +-
src/test/regress/pg_regress.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/doc/src/sgml/regress.sgml b/doc/src/sgml/regress.sgml
index 673a8c2164..a33e2482f2 100644
--- a/doc/src/sgml/regress.sgml
+++ b/doc/src/sgml/regress.sgml
@@ -410,7 +410,7 @@ make standbycheck
If you don't
like the <command>diff</command> options that are used by default, set the
environment variable <envar>PG_REGRESS_DIFF_OPTS</envar>, for
- instance <literal>PG_REGRESS_DIFF_OPTS='-u'</literal>. (Or you
+ instance <literal>PG_REGRESS_DIFF_OPTS='-C'</literal>. (Or you
can run <command>diff</command> yourself, if you prefer.)
</para>
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 3248603da1..bb06624936 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -62,10 +62,10 @@ static char *shellprog = SHELLPROG;
*/
#ifndef WIN32
const char *basic_diff_opts = "";
-const char *pretty_diff_opts = "-C3";
+const char *pretty_diff_opts = "-U3";
#else
const char *basic_diff_opts = "-w";
-const char *pretty_diff_opts = "-w -C3";
+const char *pretty_diff_opts = "-w -U3";
#endif
/* options settable from command line */
--
2.19.1
On Thu, Nov 22, 2018 at 02:10:59PM +0100, Christoph Berg wrote:
Re: Noah Misch 2017-04-07 <20170407021431.GB2658646@tornado.leadboat.com>
I personally, and I know of a bunch of other regular contributors, find
context diffs very hard to read. Besides general dislike, for things
like regression test output context diffs are just not well suited.Personally, I disagree completely. Unified diffs are utterly unreadable
for anything beyond trivial cases of small well-separated changes.It's possible that regression failure diffs will usually fall into that
category, but I'm not convinced.For reading patches, I frequently use both formats. Overall, I perhaps read
unified 3/4 of the time and context 1/4 of the time.For regression diffs, I use PG_REGRESS_DIFF_OPTS=-u and have never converted a
regression diff to context form. Hence, +1 for the proposed change.I've just had another case of horrible context diff from pg_regress.
I'd claim that regression diffs are particularly bad for context diffs
because one error will often trigger a whole chain of failures, which
will essentially make the whole file appear twice in the output,
whereas unified diffs would just put the original output and the error
right next to each other at the top. Having to scroll through a whole
.out file just to find "create extension; file not found" is very
inefficient.It's nice that PG_REGRESS_DIFF_OPTS exists, but the diffs are often
coming from automated build logs where setting the variable after the
fact doesn't help.Please consider the attached patch, extension packagers will thank
you.Christoph
+1 for pushing this. Regression diffs can get pretty noisy even with
it in place.
Best,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
On 22/11/2018 14:10, Christoph Berg wrote:
It's nice that PG_REGRESS_DIFF_OPTS exists, but the diffs are often
coming from automated build logs where setting the variable after the
fact doesn't help.Please consider the attached patch, extension packagers will thank
you.
Committed.
While we're considering the pg_regress output, what do you think about
replacing the ======... separator with a standard diff separator like
"diff %s %s %s\n". This would make the file behave more like a proper
diff file, for use with other tools. And it shows the diff options
used, for clarity. See attached patch.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
0001-Use-standard-diff-separator-for-regression.diffs.patchtext/plain; charset=UTF-8; name=0001-Use-standard-diff-separator-for-regression.diffs.patch; x-mac-creator=0; x-mac-type=0Download
From 00380be627fd1df6dd0474380d31fd76fcb644de Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Wed, 2 Jan 2019 21:24:51 +0100
Subject: [PATCH] Use standard diff separator for regression.diffs
Instead of ======..., use the standard separator for a multi-file
diff, which is, per POSIX,
"diff %s %s %s\n", <diff_options>, <filename1>, <filename2>
This makes regression.diffs behave more like a proper diff file, for
use with other tools. And it shows the diff options used, for
clarity.
---
src/test/regress/pg_regress.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 2c469413a3..9786e86211 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -1453,20 +1453,23 @@ results_differ(const char *testname, const char *resultsfile, const char *defaul
* Use the best comparison file to generate the "pretty" diff, which we
* append to the diffs summary file.
*/
- snprintf(cmd, sizeof(cmd),
- "diff %s \"%s\" \"%s\" >> \"%s\"",
- pretty_diff_opts, best_expect_file, resultsfile, difffilename);
- run_diff(cmd, difffilename);
- /* And append a separator */
+ /* Write diff header */
difffile = fopen(difffilename, "a");
if (difffile)
{
fprintf(difffile,
- "\n======================================================================\n\n");
+ "diff %s %s %s\n",
+ pretty_diff_opts, best_expect_file, resultsfile);
fclose(difffile);
}
+ /* Run diff */
+ snprintf(cmd, sizeof(cmd),
+ "diff %s \"%s\" \"%s\" >> \"%s\"",
+ pretty_diff_opts, best_expect_file, resultsfile, difffilename);
+ run_diff(cmd, difffilename);
+
unlink(diff);
return true;
}
--
2.20.1
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
While we're considering the pg_regress output, what do you think about
replacing the ======... separator with a standard diff separator like
"diff %s %s %s\n". This would make the file behave more like a proper
diff file, for use with other tools. And it shows the diff options
used, for clarity. See attached patch.
I'm confused by this patch. Doesn't moving the diff call like that
break the logic completely?
regards, tom lane
Re: Peter Eisentraut 2019-01-02 <70440c81-37bb-76dd-e48b-b5a9550d5613@2ndquadrant.com>
Committed.
\o/
While we're considering the pg_regress output, what do you think about
replacing the ======... separator with a standard diff separator like
"diff %s %s %s\n". This would make the file behave more like a proper
diff file, for use with other tools. And it shows the diff options
used, for clarity. See attached patch.
It will especially say which _alternate.out file was used, which seems
like a big win. So +1.
Christoph
On 3 Jan 2019, at 10:39, Christoph Berg <myon@debian.org> wrote:
Re: Peter Eisentraut 2019-01-02 <70440c81-37bb-76dd-e48b-b5a9550d5613@2ndquadrant.com>
While we're considering the pg_regress output, what do you think about
replacing the ======... separator with a standard diff separator like
"diff %s %s %s\n". This would make the file behave more like a proper
diff file, for use with other tools. And it shows the diff options
used, for clarity. See attached patch.It will especially say which _alternate.out file was used, which seems
like a big win. So +1.
That has bitten more times than I’d like to admit, so definately +1 on being
explicit about that.
cheers ./daniel
On 02/01/2019 21:44, Tom Lane wrote:
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
While we're considering the pg_regress output, what do you think about
replacing the ======... separator with a standard diff separator like
"diff %s %s %s\n". This would make the file behave more like a proper
diff file, for use with other tools. And it shows the diff options
used, for clarity. See attached patch.I'm confused by this patch. Doesn't moving the diff call like that
break the logic completely?
For clarification, I have attached a "before" and "after".
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
regression.diffs.newtext/plain; charset=UTF-8; name=regression.diffs.new; x-mac-creator=0; x-mac-type=0Download
diff -u /Users/peter/devel/postgresql/postgresql/src/test/regress/expected/int4.out /Users/peter/devel/postgresql/postgresql/src/test/regress/results/int4.out
--- /Users/peter/devel/postgresql/postgresql/src/test/regress/expected/int4.out 2018-12-07 15:10:58.000000000 +0100
+++ /Users/peter/devel/postgresql/postgresql/src/test/regress/results/int4.out 2019-01-03 11:49:53.000000000 +0100
@@ -1,4 +1,4 @@
---
+--x
-- INT4
--
CREATE TABLE INT4_TBL(f1 int4);
diff -u /Users/peter/devel/postgresql/postgresql/src/test/regress/expected/int8.out /Users/peter/devel/postgresql/postgresql/src/test/regress/results/int8.out
--- /Users/peter/devel/postgresql/postgresql/src/test/regress/expected/int8.out 2018-12-07 15:10:58.000000000 +0100
+++ /Users/peter/devel/postgresql/postgresql/src/test/regress/results/int8.out 2019-01-03 11:49:53.000000000 +0100
@@ -1,4 +1,4 @@
---
+--x
-- INT8
-- Test int8 64-bit integers.
--
regression.diffs.oldtext/plain; charset=UTF-8; name=regression.diffs.old; x-mac-creator=0; x-mac-type=0Download
--- /Users/peter/devel/postgresql/postgresql/src/test/regress/expected/int4.out 2018-12-07 15:10:58.000000000 +0100
+++ /Users/peter/devel/postgresql/postgresql/src/test/regress/results/int4.out 2019-01-03 11:43:48.000000000 +0100
@@ -1,4 +1,4 @@
---
+--x
-- INT4
--
CREATE TABLE INT4_TBL(f1 int4);
======================================================================
--- /Users/peter/devel/postgresql/postgresql/src/test/regress/expected/int8.out 2018-12-07 15:10:58.000000000 +0100
+++ /Users/peter/devel/postgresql/postgresql/src/test/regress/results/int8.out 2019-01-03 11:43:48.000000000 +0100
@@ -1,4 +1,4 @@
---
+--x
-- INT8
-- Test int8 64-bit integers.
--
======================================================================
On 03/01/2019 10:39, Christoph Berg wrote:
It will especially say which _alternate.out file was used, which seems
like a big win. So +1.
It already shows that in the existing diff output header.
Although if you have a really long absolute path, it might be hard to
find it. So perhaps the attached patch to make it more readable.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
0001-pg_regress-Don-t-use-absolute-paths-for-the-diff.patchtext/plain; charset=UTF-8; name=0001-pg_regress-Don-t-use-absolute-paths-for-the-diff.patch; x-mac-creator=0; x-mac-type=0Download
From ac586a4fa2186b4cf85c4b7c7e0269fac1af402e Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Thu, 3 Jan 2019 12:12:21 +0100
Subject: [PATCH] pg_regress: Don't use absolute paths for the diff
Don't expand inputfile and outputfile to absolute paths globally, just
where needed. In particular, pass them as is to the file name
arguments of the diff command, so that we don't see the full absolute
path in the diff header, which makes the diff unnecessarily verbose
and harder to read.
---
src/test/regress/pg_regress.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 9786e86211..4970b74f1b 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -488,7 +488,7 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
/* Error logged in pgfnames */
exit(2);
- snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
+ snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", make_absolute_path(outputdir));
#ifdef WIN32
@@ -552,10 +552,10 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
}
while (fgets(line, sizeof(line), infile))
{
- replace_string(line, "@abs_srcdir@", inputdir);
- replace_string(line, "@abs_builddir@", outputdir);
+ replace_string(line, "@abs_srcdir@", make_absolute_path(inputdir));
+ replace_string(line, "@abs_builddir@", make_absolute_path(outputdir));
replace_string(line, "@testtablespace@", testtablespace);
- replace_string(line, "@libdir@", dlpath);
+ replace_string(line, "@libdir@", make_absolute_path(dlpath));
replace_string(line, "@DLSUFFIX@", DLSUFFIX);
fputs(line, outfile);
}
@@ -2220,10 +2220,6 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc
*/
port = 0xC000 | (PG_VERSION_NUM & 0x3FFF);
- inputdir = make_absolute_path(inputdir);
- outputdir = make_absolute_path(outputdir);
- dlpath = make_absolute_path(dlpath);
-
/*
* Initialization
*/
@@ -2569,7 +2565,7 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc
printf(_("The differences that caused some tests to fail can be viewed in the\n"
"file \"%s\". A copy of the test summary that you see\n"
"above is saved in the file \"%s\".\n\n"),
- difffilename, logfilename);
+ make_absolute_path(difffilename), make_absolute_path(logfilename));
}
else
{
--
2.20.1
On 2019-01-03 11:20, Daniel Gustafsson wrote:
On 3 Jan 2019, at 10:39, Christoph Berg <myon@debian.org> wrote:
Re: Peter Eisentraut 2019-01-02 <70440c81-37bb-76dd-e48b-b5a9550d5613@2ndquadrant.com>
While we're considering the pg_regress output, what do you think about
replacing the ======... separator with a standard diff separator like
"diff %s %s %s\n". This would make the file behave more like a proper
diff file, for use with other tools. And it shows the diff options
used, for clarity. See attached patch.It will especially say which _alternate.out file was used, which seems
like a big win. So +1.That has bitten more times than I’d like to admit, so definately +1 on being
explicit about that.
committed
(This might be one of those rare times where one hopes for a buildfarm
failure for verification. :-) )
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
"Peter" == Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
Peter> (This might be one of those rare times where one hopes for a
Peter> buildfarm failure for verification. :-) )
Also while we're tweaking regression test output, would it be possible
to have some indicator of whether a test passed because a variant file
in the resultmap was ignored in favor of the standard result?
The current system of silently ignoring the resultmap means that nobody
ever notices when resultmap entries become obsolete....
--
Andrew (irc:RhodiumToad)
Re: Andrew Gierth 2019-02-15 <874l95m8w7.fsf@news-spur.riddles.org.uk>
Also while we're tweaking regression test output, would it be possible
to have some indicator of whether a test passed because a variant file
in the resultmap was ignored in favor of the standard result?The current system of silently ignoring the resultmap means that nobody
ever notices when resultmap entries become obsolete....
By the same argument, it should always print which variant file was
used so determining which _N.out files are still in use is possible.
Christoph
On 2019-01-03 12:16, Peter Eisentraut wrote:
On 03/01/2019 10:39, Christoph Berg wrote:
It will especially say which _alternate.out file was used, which seems
like a big win. So +1.It already shows that in the existing diff output header.
Although if you have a really long absolute path, it might be hard to
find it. So perhaps the attached patch to make it more readable.
Any objections to this change?
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2019-02-15 16:05, Christoph Berg wrote:
Re: Andrew Gierth 2019-02-15 <874l95m8w7.fsf@news-spur.riddles.org.uk>
Also while we're tweaking regression test output, would it be possible
to have some indicator of whether a test passed because a variant file
in the resultmap was ignored in favor of the standard result?The current system of silently ignoring the resultmap means that nobody
ever notices when resultmap entries become obsolete....By the same argument, it should always print which variant file was
used so determining which _N.out files are still in use is possible.
I would rather not overload the test output even more. A test passes or
it doesn't. If we need to collect additional information, let's think
about ways to do that separately.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: Peter Eisentraut 2019-02-20 <40c5c12f-adad-fc86-7d43-ff7c535356ed@2ndquadrant.com>
By the same argument, it should always print which variant file was
used so determining which _N.out files are still in use is possible.I would rather not overload the test output even more. A test passes or
it doesn't. If we need to collect additional information, let's think
about ways to do that separately.
Aye. No objections.
Christoph