Re: Updating the test suite (Was: commit: ldap/tests run.in)

[Hallvard B Furuseth <h.b.furuseth@usit.uio.no>]

Howard Chu writes:
>Hallvard B Furuseth wrote:
>> * Run each LDAP client and daemon as something like
>>     $LDAP_TESTER <program> <args>...
>>   where the user can set the (normally unset) $LDAP_TESTER
>>   environment variable to e.g. valgrind or "xterm -e gdb --args".
>>   Actually, we could use 4 variables:
>>   $LDAP_RETCODE_TESTER for clients whose return code (other than
>>   success/failure?) is important, defaulting to:
>>   $LDAP_FG_TESTER for foreground processes, defaulting to:
>>   $LDAP_TESTER for any process, and
>>   $LDAP_BG_TESTER for background processes (default $LDAP_TESTER).
>
> I've gotten pretty used to manually invoking valgrind when I want it. 
> Dunno...

Hmm.  Your 'sleep 1' commit includes:

> --- scripts/test008-concurrency	26 Sep 2005 12:52:02 -0000	1.37
> +++ scripts/test008-concurrency	30 Sep 2005 05:32:49 -0000	1.38
>  echo "Using tester for concurrent server access..."
> -$SLAPDTESTER -P "$PROGDIR" -d "$DATADIR" -h $LOCALHOST -p $PORT1 -D "$MANAGERDN" -w $PASSWD -l 50
> +time $SLAPDTESTER -P "$PROGDIR" -d "$DATADIR" -h $LOCALHOST -p $PORT1 -D "$MANAGERDN" -w $PASSWD -l 50
> +#$SLAPDTESTER -P "$PROGDIR" -d "$DATADIR" -h $LOCALHOST -p $PORT1 -D "$MANAGERDN" -w $PASSWD -l 50
>  RC=$?

and

> --- scripts/test021-certificate	29 Sep 2005 20:26:44 -0000	1.14
> +++ scripts/test021-certificate	30 Sep 2005 05:32:50 -0000	1.15
>  
>  echo "Starting slapd on TCP/IP port $PORT1..."
> +#valgrind -v --gdb-attach=yes --logfile=info --num-callers=16 --leak-check=yes --leak-resolution=high $SLAPD -f $CONF1 -h $URI1 -d $LVL $TIMING </dev/tty > $LOG1 2>&1 &
>  $SLAPD -f $CONF1 -h $URI1 -d $LVL $TIMING > $LOG1 2>&1 &

_That's_ what I do too.  Edit the test suite to invoke some program.
Though I put the program and args in an env variable.
BTW, was that 'time' call intended to reach CVS?

>> * Give ./run an "-ignore" (-i) argument, to ignore some errors and
>>   keep running the tests.  scripts/all would not abort if a test
>>   fails, and specific checks in each script could be marked as
>>   "soft" (keep going under ./run -i) or "hard" (always abort).
>
> Perhaps.

Actually that --soft argument to my Demand_RC() causes a lot of problems
when I look at it closely.  For the time being I think this would at most
implement that 'run all' does not abort if one test fails.

>> * The tests should wait for daemons to exit after killing them, and
>>   fail if they failed.
> (...)
>
> We should also refuse the -k (KILLSERVERS) option if more than one
> test is being run.

Right.

>> * While waiting for slapd to start,
>>   - sleep briefly after first failure and longer in each iteration.
>>   - If ldapsearch fails, abort the loop if slapd is not running.
>>   - Do not sleep 5 seconds before exiting the loop and failing.
>
> Yes. The excessive sleeps here were getting annoying. I don't know
> about waiting for up to 30 or more seconds; if the server doesn't
> start up right away it probably isn't going to start up at all. Even
> on the slowest dev system I used (an OS/390 beast) it usually started
> within 10 seconds.

I agree - with the caveat that it gets a lot slower with valgrind
--memcheck on an already slow box.  I remember _some_ sleep was too
fast for me in that case, but I don't remember if it was this one.
Anyway, we could reduce the default sleep time but take the max sleep
time from an optional environment variable.

>> * "./run all" should only run tests matching "scripts/*[a-zA-Z0-9]"
>>   or something, so Emacs backup files "testxyz-foo.~19~" are not run.
>>
>>   What are the backup file names to avoid from other editors?
>>
>>   Does Cygwin support filename expansion like *[a-zA-Z0-9], or must
>>   this be handled with a case statement in scripts/all?
>
> Filename expansion is implemented by the shell, not the operating
> system. "Does Cygwin support it" is a meaningless question.  Does
> Cygwin support Unix-style command shells - of course, that's one of
> its reasons for existence.

Fine.  I didn't even know that much about Cygwin.

>> * Allow "./run scripts/scriptname" and not just "./run scriptname",
>>   so one  can use filename expansion when typing a test command:
>>   "./run scripts/<start-of-filename><tab>"
>
> This seems unnecessary. Since run already knows to look for arg*, and
> you have to supply a unique prefix anyway, this is more work to invoke
> than the existing functionality.
>
> i.e. "./run test033" is less to type than "./run scripts/test033<tab>"
> so what's the point?

Heh.  I had to think a bit do figure out what I wanted with that one.
Once in a while I type

  ./run scripts/t<tab>

to find the test I want.  Then after completing the test name I have to
go back and delete "scripts/" after writing the test name.  That's three
extra keystrokes - horrible!  (As opposed to - how many? - in this part
of the discussion?:-)

>> Kill_named () {
>> 	eval "fV_name=\${fV_PIDNAME$1-child} fV_PID$1="
>> 	echo "Stopping and waiting for $fV_name."
>> 	kill -HUP $1 && wait $1 && return 0
>> 	echo "$fV_name (pid $1) failed ($?)!" >&2
>> 	Kill_all
>> 	return 99
>> }
>
> This function must unset the particular PID from fv_PIDVARS, otherwise
> Kill_all will complain.

It does, indirectly.  For pid 345, Fv_PIDVARS contains '$fV_PID345',
where fV_PID345 originally is '345'.  Kill_named sets fV_PID345=''.

-- 
Hallvard