On 10/28/18 8:01 PM, PiBa-NL wrote:
Hi Frederic,
Hello Pieter,
Sorry again for this late reply.
Op 19-10-2018 om 11:51 schreef Frederic Lecaille:
The idea of the script sounds good to me. About the script itself it
is a nice work which could be a good start.
Thanks.
Just a few details below.
Note that "cat $file | grep something" may be shortened by "grep
something $file". It would also be interesting to avoid creating
temporary files as most as possible (at least testflist.lst is not
necessary I think).
TARGET=$(haproxy -vv | grep "TARGET = " | sed 's/.*= //')
OPTIONS=$(haproxy -vv | grep "OPTIONS = " | sed 's/.*= //')
may be shortened by these lines:
{ read TARGET; read OPTIONS; } << EOF
$(./haproxy -vv |grep 'TARGET\|OPTIONS' | sed 's/.* = //')
EOF
Thanks, I've changed this.
which is portable, or something similar.
sed 's/.*=//'| sed 's/,.*//'
may be shortened by
sed -e 's/.*=//' -e 's/,.*//'
Thanks, I've changed this as well.
Also note, there are some cases where options are enabled without
appearing in OPTIONS variable value.
For instance if you compile haproxy like that:
$ make TARGET=linux2628
the support for the thread is enabled without appearing in OPTIONS
variable value. I am not sure this is an issue at this time.
That could become an issue. but should be easy to solve adding a 'fake'
option perhaps to check against.. Or adding a separate check perhaps I'm
not sure yet.
With one test like that after having parsed OPTIONS, it would suffice at
this time :
if [ $TARGET = linux2628 ] ; then
OPTIONS="$OPTIONS USE_xxx USE_xxx"
fi
with USE_xxx the listed compilation options found in the Makefile file:
ifeq ($(TARGET),linux2628)
# This is for standard Linux >= 2.6.28 with netfilter, epoll, tproxy
and splice
USE_NETFILTER = implicit
USE_POLL = implicit
USE_EPOLL = implicit
USE_TPROXY = implicit
USE_CRYPT_H = implicit
USE_LIBCRYPT = implicit
USE_LINUX_SPLICE= implicit
USE_LINUX_TPROXY= implicit
USE_ACCEPT4 = implicit
USE_FUTEX = implicit
USE_CPU_AFFINITY= implicit
ASSUME_SPLICE_WORKS= implicit
USE_DL = implicit
USE_RT = implicit
USE_THREAD = implicit
else
Perhaps we could could only one line for the required options and
excluded targets like that:
#EXCLUDED_TARGETS=freebsd,dos,windows ;)
#REQUIRED_OPTIONS=OPENSSL,LUA,ZLIB
Added this option, but then i like the option of excluding single
targets and having some comment behind it explaining the reason.. But i
guess if one would want to know some comments above the setting could
also 'explain' why that target is currently not 'valid' to run the test
on. Should i remove the settings for the 'single' option/target.?
No. It is ok at this time.
New 'version' of the script attached.
It now supports a set of parameters to modify its behavior a little. And
also checking for a 'version requirement'. So a H2 test doesn't have to
fail on 1.7.
Very good work.
Should i 'ask' to delete old test result.? Or would there be a other
better way to keep previous results separated from the current run?
No.
Rename the old vtc files like that before running the tests:
bname=$(basename $i)
mv /tmp/$bname /tmp/old.$bname
with $i in $(find /tmp/ -type d -name "vtc.*")
or something like that.
please *add -type d* to the find command to list the temporary
varnishtest directories.
And we should only keep temporaries directory of tests which have failed
with -l varnishtest option currently enabled option. They are all the
/tmp/vtc.* directories found after having run the tests.
So the script should list the remaining temporary directories after
having run the tests. There is no need to parse the log file of this
directories. We cannot rely on the fact there is no bug in varnishtest
logging code ;)
If you could give it another look i would appreciate that. Are there any
things that need to be added/changed about it before considering to add
it to haproxy sources branch?
Yes a few details.
It seems there are remaining useless piped sed processes (sed | sed) or
perhaps I have missed something.
There is no need to use awk to remove sections of lines. Have a look to
cut command which is more lightweight.
tr command accepts escaped characters as '\n' for new line character
which is more readable than '
' ;)
For me with these remaining modifications/cleanup this script could be
added to haproxy sources.
Thanks again for this good work Pieter.
Regards,
Fred.
