Morning,
I've been getting some random test-suite failures for some time now so
I've started looking at diag.sh and there seems to be a bunch of things
that should be fixed.
If I don't get an outright disapproval for the following analysis, I'll
start filing PRs.
- diag.sh is sometimes sourced, sometimes not
sample[0]: $srcdir/diag.sh wait-startup $3 || . ./diag.sh error-exit $?
Looks like diag.sh is _not_ sourced for just couple of commands, but I
think that is coincidence and a bug; the commands still call error-exit,
which is intended to kill the test, which can't happen if diag.sh is not
sourced.
I.e.: diag.sh should be always sourced, right?
- redundant calls to error-exit
same sample[0]
error-exit is already called in the command whose success is tested.
wait-startup either returns 0 or error-exit is called within. The second
part of the line is redundant.
(Maybe the need for the second part was motivated by not sourcing
diag.sh in the first part?.. But there are cases even with sourced diag.sh)
- inconsistent use of $srcdir vs relative path
same sample[0] (boy, this line is bad :])
I guess all the occurrances of ./diag.sh should be changed to
$srcdir/diag.sh.
- The whole code that handles CURRENT_TEST is unnecessary
The information stored in this variable / file is only ever used in
error-exit.
The information is accessible in its scope and never modified, hence it
doesn't need to be stored beforehand. The file doesn't function as a
lock, as it is truncated before spawning additional processes that could
access it.
- spawning an interactive shell
sample[1]:
echo "ABORT! core file exists, starting interactive shell"
bash
I think this is a mistake. Michael mentioned some time ago that he
intends to run the test-suite during package builds and I'd like to do
the same. Even in other scenarios, you probably want to keep the whole
process as non-interactive as possible.
- wait-shutdown doesn't kill the running instance
sample[2]: echo "manual cleanup."
After experimenting with the test-suite, I've sometimes wondered why I
have so many rsyslog instances running... :]
I think it would be cleaner to just kill the current instance after the
timeout. Leaving the instance running can mess up all subsequent tests.
Comments?
HTH,
Tomas
[0] https://github.com/rsyslog/rsyslog/blob/master/tests/diag.sh#L106
[1] https://github.com/rsyslog/rsyslog/blob/master/tests/diag.sh#L162
[2] https://github.com/rsyslog/rsyslog/blob/master/tests/diag.sh#L156
_______________________________________________
rsyslog mailing list
http://lists.adiscon.net/mailman/listinfo/rsyslog
http://www.rsyslog.com/professional-services/
What's up with rsyslog? Follow https://twitter.com/rgerhards
NOTE WELL: This is a PUBLIC mailing list, posts are ARCHIVED by a myriad of
sites beyond our control. PLEASE UNSUBSCRIBE and DO NOT POST if you DON'T LIKE
THAT.
