Re: pg_upgrade test writes to source directory

From: Andres Freund <andres(at)anarazel(dot)de>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, Justin Pryzby <pryzby(at)telsasoft(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: pg_upgrade test writes to source directory
Date: 2022-08-16 03:20:51
Message-ID: 20220816032051.ya7ieuteaq2h6g2m@awork3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On 2022-08-11 11:26:39 -0400, Tom Lane wrote:
> Andres Freund <andres(at)anarazel(dot)de> writes:
> > On 2022-06-01 10:55:28 -0400, Tom Lane wrote:
> >> [...] I'm definitely not happy with the proposed changes to
> >> 010_tab_completion.pl. My recollection is that those tests
> >> were intentionally written to test tab completion involving a
> >> directory name, but this change just loses that aspect entirely.
>
> > How about creating a dedicated directory for the created files, to maintain
> > that? My goal of being able to redirect the test output elsewhere can be
> > achieved with just a hunk like this:
>
> Sure, there's no need for these files to be in the exact same place that
> the output is collected. I just want to keep their same relationship
> to the test's CWD.
>
> > Of course it'd need a comment adjustment etc. It's a bit ugly to use a
> > otherwise empty tmp_check/ directory just to reduce the diff size, but it's
> > also not too bad.
>
> Given that it's no longer going to be the same tmp_check dir used
> elsewhere, maybe we could s/tmp_check/tab_comp_dir/g or something
> like that? That'd add some clarity I think.

Done in the attached patch (0001).

A bunch of changes (e.g. f4ce6c4d3a3) made since I'd first written that
TESTOUTDIR patch means that we don't need two different variables anymore. So
patch 0002 just moves the addition of /tmp_check from Utils.pm to the places
in which TESTDIR is defined.

That still "forces" tmp_check/ to exist when going through pg_regress, but
that's less annoying because pg_regress at least keeps
regression.{diffs,out}/log files/directory outside of tmp_check/.

I've also attached a 0003 that splits the log location from the data
location. That could be used to make the log file location symmetrical between
pg_regress (log/) and tap tests (tmp_check/log). But it'd break the
buildfarm's tap test log file collection, so I don't think that's something we
really can do soon-ish?

Greetings,

Andres Freund

Attachment Content-Type Size
0001-Create-test-files-for-010_tab_completion.pl-in-dedic.patch text/x-diff 3.7 KB
0002-Don-t-hardcode-tmp_check-as-test-directory-for-tap-t.patch text/x-diff 3.5 KB
0003-Split-TESTDIR-into-TESTLOGDIR-and-TESTDATADIR.patch text/x-diff 4.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrey Borodin 2022-08-16 03:57:54 Logical WAL sender unresponsive during decoding commit
Previous Message Bruce Momjian 2022-08-16 02:53:18 Re: SELECT documentation