Andres Freund <andres@anarazel.de> writes:
> On 2022-08-11 11:26:39 -0400, Tom Lane wrote:
>> 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).
I was confused by 0001, because with the present test setup that will
result in creating an extra tab_comp_dir that isn't inside tmp_check,
leading to needing cleanup infrastructure that isn't there. However,
0002 clarifies that: you're redefining TESTDIR. I think 0001 is OK
as long as you apply it after, or integrate it into, 0002.
> patch 0002 just moves the addition of /tmp_check from Utils.pm to the places
> in which TESTDIR is defined.
I see some references to TESTDIR in src/tools/msvc/ecpg_regression.proj.
It looks like those are not references to this variable but uses of the
<PropertyGroup>
<TESTDIR>..\..\interfaces\ecpg\test</TESTDIR>
thingy at the top of the file. Still, it's a bit confusing --- should
we rename that? Maybe not worth the trouble given the short expected
lifespan of the MSVC test scripts. 0002 seems fine otherwise.
> 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?
No particular opinion about 0003 -- as you say, that's going to be
gated by the buildfarm.
regards, tom lane