Thread: pgsql: Reference test binary using TESTDIR in 001_libpq_pipeline.pl.
Reference test binary using TESTDIR in 001_libpq_pipeline.pl. The previous approach didn't really work on windows, due to the PATH separator being ';' not ':'. Instead of making the PATH change more complicated, reference the binary using the TESTDIR environment. Reported-By: Andres Freund <andres@anarazel.de> Suggested-By: Andrew Dunstan <andrew@dunslane.net> Discussion: https://postgr.es/m/20210930214040.odkdd42vknvzifm6@alap3.anarazel.de Backpatch: 14-, where the test was introduced. Branch ------ REL_14_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/0baa33da0fd123aff215c505116dc60bc04dae90 Modified Files -------------- src/test/modules/libpq_pipeline/t/001_libpq_pipeline.pl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
On 10/18/21 10:23 AM, Alvaro Herrera wrote:
> On 2021-Oct-17, Andrew Dunstan wrote:
>
>> +sub find_built_program
>> +{
>> + my $program = shift;
>> + my $path;
>> +
>> + if (defined $ENV{MSBUILDDIR})
>> + {
>> + # vcregress.pl sets MSBUILDDIR which is the root of all the build dirs
>> + my $wanted = sub { $_ eq "$program.exe" && $path = $File::Find::name;};
>> + File::Find::find($wanted,$ENV{MSBUILDDIR});
>> + }
> Hmm, it seems weird to have to use File::Find when we already know where
> the program is, right? I mean, per your previous patch, we know that
> the program is in $MSBUILDDIR/libpq_pipeline/libpq_pipeline.exe, so why
> don't we make this one be
>
> if (defined $ENV{MSBUILDDIR})
> {
> return $ENV{MSBUILDDIR} . "$program/$program.exe";
> }
Could do, but say the module had two programs? Mostly we don't do that
but Release/pg_isolation_regress has two executables, so it's more than
just theoretically possible.
File::Find on the build directory is likely to be fairly quick, but a
simpler way might be to use glob expansion:
maybe something like:
my @progs = glob("$ENV{MSBUILDDIR}/*/$program.exe");
plus some logic to disambiguate any duplicates.
>
> ?
>
> I noticed that drongo is still red, and reporting that no tests are
> being run. While this makes sense (because the list of tests is
> obtained by running libpq_pipeline), I am surprised that this isn't
> reported as such. I would have expected it to die with an error message
> starting with "oops: " ... did run_command() fail to return stderr?
>
> ... oh, run_command() seems a bit too optimistic: it doesn't check
> whether IPC::Run::run() failed. I think that's worth fixing
> independently.
>
Yeah.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
On 10/20/21 12:31 PM, Andres Freund wrote: > > ISTM that we should just make the "test invocation framework" responsible to > put the relevant directory onto PATH? I.e. Makefile.global should add the > build directory of the test to PATH, and vcregress.pl should put the relevant > $configuration/$project/ directory there? > Like this? That seems reasonable. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Attachment
On 10/20/21 3:09 PM, Andres Freund wrote: > Hi, > > On 2021-10-20 14:30:49 -0400, Andrew Dunstan wrote: >> On 10/20/21 12:31 PM, Andres Freund wrote: >>> ISTM that we should just make the "test invocation framework" responsible to >>> put the relevant directory onto PATH? I.e. Makefile.global should add the >>> build directory of the test to PATH, and vcregress.pl should put the relevant >>> $configuration/$project/ directory there? >> Like this? That seems reasonable. > Yea, that's along the lines of what I was thinking... > OK, it turns out that it's better to put the addition at the end of the PATH, otherise initdb complains that it can't find postgres in the same directory. This patch has been tested on Linux but not yet on Windows/MSVC, although I have little doubt it will work there too. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Attachment
Andrew Dunstan <andrew@dunslane.net> writes:
> OK, it turns out that it's better to put the addition at the end of the
> PATH, otherise initdb complains that it can't find postgres in the same
> directory.
That seems horribly unsafe, or at least prone to pulling in the
program from some other versuion of Postgres. How about ordering
the path like
PATH="$(abs_top_builddir)/tmp_install$(bindir):$(CURDIR):$$PATH"
regards, tom lane
On 10/20/21 5:48 PM, Tom Lane wrote: > Andrew Dunstan <andrew@dunslane.net> writes: >> OK, it turns out that it's better to put the addition at the end of the >> PATH, otherise initdb complains that it can't find postgres in the same >> directory. > That seems horribly unsafe, or at least prone to pulling in the > program from some other versuion of Postgres. How about ordering > the path like > > PATH="$(abs_top_builddir)/tmp_install$(bindir):$(CURDIR):$$PATH" > > fair point. See attached. Tested on MSVC and Linux cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com