Re: [HACKERS] Time to change pg_regress diffs to unified by default?
Christoph Berg <myon@debian.org>
From: Christoph Berg <myon@debian.org>
To: Noah Misch <noah@leadboat.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Andres Freund <andres@anarazel.de>, pgsql-hackers@postgresql.org
Date: 2018-11-22T13:10:59Z
Lists: pgsql-hackers
Attachments
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
Commits
-
pg_regress: Don't use absolute paths for the diff
- 1995552deb54 12.0 landed
-
Use standard diff separator for regression.diffs
- 8f27a14b1bd3 12.0 landed