Re: [PATCH] test-driver: optionally run in test directory

[Philipp A. Hartmann]

Hi Stefano,

On 29/10/13 01:10, Stefano Lattarini wrote:
> 
> On 10/25/2013 10:34 AM, Philipp A. Hartmann wrote:
>> ...
>> This patch adds an option '--change-dir={yes|no}' to the
>> test-driver to change to the sub-directory of the given
>> test script (directory component taken from the script
>> name, if any) before running the script.
>>
> Honestly, I'm very reluctant to add more code to the test-driver
> script for a feature that, as you noticed, is very easy to implement
> (in an even more controlled and granular fashion) with environment
> variables and/or simple LOG_COMPILER wrapper script.  In addition,
> the custom test drivers support in automake (since 1.12) means that
> you *can* easily use your own version of the script in your package,
> without any need to patch automake directly.

I agree (based on your review below) that this functionality may not be
fit for the core script.  Adding the needed change to the driver just
was the quickest way for me to solve my local problem.  Then I thought
it might be useful for others.

> Let me still do a review of your patch though; hopefully you can
> benefit from it in case you decide to keep using the patched
> script (instead of environment variables) in your project.

Thanks for the review.  :-)

[...]
>> +# Optionally switch to directory of test script before running it.
>> +if test $change_dir = yes; then
>> +  tstbin=`basename "$@"`
>>
> 'basename' only take a single argument, so you should just use "$1"
> here.  (Note that this is more a theoretical issue, since at the
> moment there is no way the test-driver script will be passed more
> than one non-option argument by the automake generated Makefile
> rules).

Agreed.

>> +  # Prefer dirname, but fall back on a substitute if dirname fails.
>>
> If you assume 'basename' exists, you could as well just assume
> 'dirname' exists as well.  And I don't think any system lacking
> basename or dirname is worth supporting or worrying about today.

Hmm, I borrowed this part from install-sh, which does the same
(unconditionally calling basename and adding dirname emulation).

[...]
>> +  # Test script is run here (in the directory of the script).
>> +  (cd "$tstdir" ; "./$tstbin") >$log_file 2>&1
>>
> This will break in VPATH rules, since it will run in the source
> directory rather than in the build directory -- and in VPATH
> builds, the source directory is allowed to be read-only (this
> actually happens in "make distcheck").  I don't know how much
> adding proper VPATH support would complicate the implementation;
> nor have I given it much thought honestly, since personally
> I'd just go with environment variables (this is what Automake's
> own testsuite does).

Here you got me.  In our case, the TESTS are all compiled/generated and
therefore not present in the source tree.  But of course in case of
preexisting test (scripts), the "change dir" functionality is just plain
wrong.

Thanks again for your feedback!

Greetings from Oldenburg,
   Philipp

[...]

-- 
Philipp A. Hartmann
Hardware/Software Design Methodology Group

OFFIS Institute for Information Technology
R&D Division Transportation · FuE-Bereich Verkehr
Escherweg 2 · 26121 Oldenburg · Germany · http://www.offis.de/
Phone/Fax: +49-441-9722-420/282 · PGP: 0x9161A5C0 · Skype: phi.har