On Thursday 24 February 2011, Ralf Wildenhues wrote: > Hi Stefano, > > * Stefano Lattarini wrote on Wed, Feb 23, 2011 at 10:20:03PM CET: > > Helper subroutines, variables and other pieces of code defined > > in the `tests/defs' and used by many testcases are non-obvious, > > and tricky to get to work portably; but until now, they weren't > > tested at all in a clear and self-contained way. > > This change should remedy to the situation. > > This patch is fine with me, with some nits below. I expect some > fallout on some systems. > Me too, but that seems acceptable, as it won't introduce spurious failures in the "real" tetcases.
> Pretty nifty stuff in this patch by the way. > Thanks. > What does the S_ prefix stand for? Sanity? > Yes (or also "self-check"). It also ensures (when LC_COLLATE=C or equivalent) that the new testcases are listed earlier by e.g. ls or shell wildcarding, and that are run early, (mostly) before all the other testcases. > Well, we can keep (or lose) our sanity over all kinds of things. ;-) > How about 'defs-' as prefix, or maybe 'suite-'? > Weren't you the one fond of shorter test names? ;-) Kidding aside, I'd rather keep the new testcases standing out more clearly (as they do with the current `S_' prefix). So for the moment I haven't renamed them. But feel free to advance other proposals, or to override my decision if the current naming really bothers you. > Thank you, > Ralf > > > * tests/S_cleanup.test: New test, check removal of temporary > > test working directory by `./defs'. > > * tests/S_dir.test: New test, check that tests using `./defs' > > create a proper temporary directory, and run in it. > > * tests/S_exit.test: New test, check that, in case of failing > > commands, the correct exit status is passed to the exit trap > > installed by the `./defs' script. > > * tests/S_is_newest.test: New test, checking the `is_newest' > > subroutine. > > * tests/S_me.test: New test, checking that $me gets defined > > automatically by `tests/defs' if not set, and that it can be > > overridden from either the shell or the environment. > > * tests/S_sanity.test: New test, check that the sanity checks > > performed by the `tests/defs' script works correctly. > > * tests/S_unindent.test: New test, checking the `unindent' > > subroutine. > > * tests/Makefile.am (TESTS): Update. > > --- /dev/null > > +++ b/tests/S_cleanup.test > > > +# Sanity check for the automake testsuite. > > +# Check creation/removal of temporary test working directory by `./defs'. > > + > > +. ./defs || Exit 1 > > + > > +# We still need a little hack to make ./defs work outside automake's > > +# tree `tests' subdirectory. Not a big deal. > > +sed "s|^testbuilddir=.*|testbuilddir='`pwd`'|" ../defs-static >defs-static > > +cp ../defs . > > + > > +have_simlinks=false > > typo symlinks here and in some but not all places below > Oops *blush*. Fixed. > > +ln -s defs foo && test -h foo && have_simlinks=: > > I'm not sure if this test works that way. > MSYS emulates ln -s with cp -p, might also emulate test -h. > You can find out for sure by something like this: > echo foo > a > ln -s a b > echo bar > a > grep bar b > > But then again, I wonder, why you even care: on systems that emulate > symlinks, you can just go along with that. So I guess all you'd need > to test here is whether ln -s apparently succeeds. > Agreed. > > +export have_simlinks # Is used also by spawned shells. > > [CUT] > > > +# Check that pre-test cleanup do not unduly change the permissions of > > s/do/does/ > Fixed, sorry for the noise. > > +# files to which symlinks in the temporary test directory point to. > > +if $have_simlinks; then > > [CUT] > > > + > > +fi # $have_symlinks > > + > > +# Check that the cleanup trap does not remove the temporary > > +# test directory in case of test failure, skip, hard-error, > > +# or upon the receiving of a signal. > > or when receiving a signal. > OK. > > +for bailout_command in \ > > + 'Exit 1' \ > > + 'Exit 2' \ > > + 'Exit 10' \ > > + 'Exit 77' \ > > + 'Exit 99' \ > > + 'Exit 126' \ > > + 'Exit 127' \ > > + 'Exit 255' \ > > + 'kill -1 $$' \ > > + 'kill -2 $$' \ > > + 'kill -9 $$' \ > > + 'kill -13 $$' \ > > + 'kill -15 $$' \ > > +; do > > + $SHELL -c ". ./defs; : > foo; $bailout_command" dummy.test && Exit 1 > > + test -f dummy.dir/foo > > +done > > --- /dev/null > > +++ b/tests/S_exit.test > > > +# Sanity check for the automake testsuite. > > +# Check that. in case of failing commands, the correct exit status is > > +# passed to the exit trap installed by the `./defs' script. > > +# Also check that the `errexit' shell flag is active. > > + > > +for st in 1 2 3 4 5 77 99 126 127 128 129 130 255; do > > + > > + echo "* Try: Exit $st" > > ramping up output markup again? ;-> > This time it seems pretty innocent, and it helps to clearly distinguish logs/traces from different subshell runs, IMHO without cluttering the script's own log. Excerpt: $ sh S_exit.test * Try: Exit 1 ~/src/automake/tests:...:/usr/sbin:/bin:/sbin ++ set -e ++ pwd /home/stefano/src/automake/tests/bash.dir + Exit 1 + set +e + exit 1 + exit 1 + exit_status=1 + set +e + cd /home/stefano/src/automake/tests + case $exit_status,$keep_testdirs in + test 0 '!=' 0 + echo 'bash: exit 1' bash: exit 1 + exit 1 * rc=1 * Try: sh -c 'exit 1' ~/src/automake/tests:...:/usr/sbin:/bin:/sbin ++ set -e ++ pwd /home/stefano/src/automake/tests/bash.dir + sh -c 'exit 1' + exit_status=1 + set +e + cd /home/stefano/src/automake/tests + case $exit_status,$keep_testdirs in + test 0 '!=' 0 + echo 'bash: exit 1' bash: exit 1 + exit 1 * rc=1 * Try: Exit 2 ~/src/automake/tests:...:/usr/sbin:/bin:/sbin ++ set -e ++ pwd /home/stefano/src/automake/tests/bash.dir + Exit 2 + set +e + exit 2 + exit 2 + exit_status=2 + set +e + cd /home/stefano/src/automake/tests + case $exit_status,$keep_testdirs in + test 0 '!=' 0 + echo 'bash: exit 2' bash: exit 2 + exit 2 * rc=2 ... > [CUT] > > --- /dev/null > > +++ b/tests/S_is_newest.test > > > +# Sanity check for the automake testsuite. > > +# Check the `is_newest' subroutine. > > + > > +. ./defs || Exit 1 > > + > > +: > a > > +$sleep > > +: > b > > +: > c > > + > > +stat a b c || : # for debugging > > stat is not portable. You knew that already, I guess. > Yes, but on the other hand ls does not always have a useful timestamp granularity; so having the above working on at least Linux and FreeBSD is better than nothing. > > +is_newest c a > > +is_newest b a > > +is_newest a b && Exit 1 > > +is_newest c b > > +is_newest c c > > +is_newest c a b c > > + > > +touch -r c d || Exit 77 > > Why the || Exit 77 here? > In case we encounter a non POSIX-compliant; but I agree it would be better to handle that situation only if it really arises. So I've removed the `|| Exit 77'. > > +stat c d || : # for debugging > > + > > +is_newest c d > > +is_newest d c > > You cannot be sure about the latter one. See 'info Autoconf --index > touch'. > Ouch (I didn't know that BTW, thanks for the pointer). Latest line removed. > > --- /dev/null > > +++ b/tests/S_sanity.test > > > +# Sanity check for the automake testsuite. > > +# Test the sanity checks performed by the `defs' script. Also check that > > +# it can be used by duplicating some of infrastructure of the automake's > > +# tree `tests' subdirectory. > > I have a hard time understanding this sentence. How about > > Also check that we can use `defs' elsewhere, when we duplicate some > of the infrastructure from the automake/tests subdirectory. > Agreed. > > +. ./defs || Exit 1 > > + > > +# Avoid to confuse traces from children processed with our own traces. > > Do not confuse traces from child processes ... > Sigh, again :-( Fixed now. > > +show_stderr() > > space before paren > Oops. Fixed, and sorry for the noise. > > +{ > > + sed 's/^/ | /' stderr >&2 > > +} > > + > > [CUT] > I'll push in 72 hours if there are no further inputs or objections. Thanks, Stefano