Re: [HACKERS] Time to change pg_regress diffs to unified by default?

From: Christoph Berg <myon(at)debian(dot)org>
To: Noah Misch <noah(at)leadboat(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Andres Freund <andres(at)anarazel(dot)de>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: [HACKERS] Time to change pg_regress diffs to unified by default?
Date: 2018-11-22 13:10:59
Message-ID: 20181122131059.GA18687@msg.df7cb.de
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

Re: Noah Misch 2017-04-07 <20170407021431(dot)GB2658646(at)tornado(dot)leadboat(dot)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

Attachment Content-Type Size
0001-Switch-pg_regress-to-output-unified-diffs-by-default.patch text/x-diff 1.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Evgeniy Efimkin 2018-11-22 13:23:01 Re: [WIP] CREATE SUBSCRIPTION with FOR TABLES clause (table filter)
Previous Message Dmitry Dolgov 2018-11-22 12:58:50 Re: [HACKERS] Re: [COMMITTERS] pgsql: Remove pgbench "progress" test pending solution of its timing is (fwd)