Re: [PATCH] Honor PG_TEST_NOCLEAN for tempdirs

On 6/26/23 22:47, Michael Paquier wrote:
> src/test/perl/README and regress.sgml both describe what
> PG_TEST_NOCLEAN does, and it seems to me that these should be updated
> to tell that temporary files are not removed on top of the data
> folders?

I've added a couple of quick lines to the docs in v2; see what you think.

On 6/26/23 23:10, Daniel Gustafsson wrote:
> I think it simply got lost in that thread which had a lot of moving
> parts as it was.

I'll make sure to register it for the CF. :D

On 6/27/23 08:20, Andrew Dunstan wrote:
> This doesn't look quite right. If PG_TEST_CLEAN had a value of 0 we
would still do the cleanup.

That's how it currently works for the data directories, but Dagfinn beat
me to the punch:

On 6/27/23 08:54, Dagfinn Ilmari Mannsåker wrote:
> However, the docs
> (https://www.postgresql.org/docs/16/regress-tap.html#REGRESS-TAP-VARS)
> say "If the environment variable PG_TEST_NOCLEAN is set", not "is set to
> a true value", and the existing test in PostgreSQL::Test::Cluster's END
> block is:
>
> # skip clean if we are requested to retain the basedir
> next if defined $ENV{'PG_TEST_NOCLEAN'};
>
> So the original `not defined` test is consistent with that.

Right. The second patch in v2 now changes that behavior across the
board, so we handle false values. I'm ambivalent on changing the wording
of the docs, but I can do that too if needed. (I'm pretty used to the
phrase "setting an environment variable" implying some sort of
true/false handling, when the envvar is a boolean toggle.)

Thanks all!
--Jacob