Re: [PATCH] REGTEST/MINOR: script: add run-regtests.sh script
On 11/29/18 10:04 PM, Frederic Lecaille wrote:
On 11/29/18 5:36 AM, Willy Tarreau wrote:
Hi guys,
On Wed, Nov 28, 2018 at 11:17:22AM +0100, Frederic Lecaille wrote:
Perhaps we should "chmod +x" this script.
Good point, done here.
However I'm now seeing this when starting it :
## Starting varnishtest
##
Testing with haproxy version: 1.8-dev0-3b0a6d-66
Assert error in start_test(), vtc_main.c line 283:
Condition((mkdir(tmpdir, 0711)) == 0) not true.
errno = 13 (Permission denied)
./scripts/run-regtests.sh: line 299: 21484 Aborted
$VARNISHTEST_PROGRAM $varnishtestparams $verbose $jobcount -l -k -t 10
$testlist
## Gathering failed results
##
find: `/proc/tty/driver': Permission denied
find: `/proc/1/task/1/fd': Permission denied
find: `/proc/1/task/1/fdinfo': Permission denied
find: `/proc/1/task/1/ns': Permission denied
find: `/proc/1/fd': Permission denied
find: `/proc/1/map_files': Permission denied
(...)
Then I stopped it using Ctrl-C.
I'm seeing a few minor issues we must still address :
- haproxy is started from the path, which means that on all systems
where it's installed, it will be the one provided by the system and
not the just built one which will be tested (as happened above),
which is confusing. I think we shouldn't search for haproxy in the
path but use $PWD/haproxy or ./haproxy or something like this.
- it seems there are some issues in the way TMPDIR/TESTDIR are
created,
as it ended up with an empty TESTDIR. In my case, TMPDIR is not set,
so TESTDIR was set to /tmp/varnishtest_haproxy. mkdir worked (I now
have this directory), but a test is missing on this mkdir.
- the way the sub-directory is created is problematic on shared
development machines, as only the first user will own the directory
and will thus prevent other users from creating their own overthere,
thus it's probably preferable not to create an intermediary
directory
in the end.
- in my case it's mktemp which failed :
++ date +%Y-%m-%d_%H-%M-%S
+ TESTRUNDATETIME=2018-11-29_05-03-43
+ TESTDIR=/tmp/varnishtest_haproxy
+ mkdir -p /tmp/varnishtest_haproxy
++ mktemp -d /tmp/varnishtest_haproxy/2018-11-29_05-03-43.
mktemp: cannot make temp dir
/tmp/varnishtest_haproxy/2018-11-29_05-03-43.: Invalid argument
+ TESTDIR=
I haven't checked why yet, but we definitely need to test the status
code for success as well.
- in my opinion the script is still missing a number of quotes when
using
string variables, especially in the directory names. If someone
is crazy
enough to have TMPDIR="/tmp/Temp Dir", then he'll have some fun.
- I'm seeing a linux-specific "rm -d" at the end, it's be better to
replace it with rmdir.
- there's "/usr/bin/env sh" at the top of the shell script. /bin/sh is
the portable one, I've spent lots of time in the past editing
scripts
where env was forced to /usr/bin while it was placed in /bin on some
systems, so I'm pretty certain that this one is not portable at
all :-)
Here is a patch for haproxy (named 0001-REGTEST*) to fix these issues.
Note the other patch for varnishtest which is also required (sent to PHK).
Fred.
A remaining patch for varnishtest.
Fred.
>From bdcdde3744f26059dd2151f2c30ed2151500098e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20L=C3=A9caille?=
Date: Fri, 30 Nov 2018 07:41:13 +0100
Subject: [PATCH 2/2] Support spaces in remaining filenames.
---
bin/varnishtest/vtc_haproxy.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/bin/varnishtest/vtc_haproxy.c b/bin/varnishtest/vtc_haproxy.c
index e361aad78..ec3006505 100644
--- a/bin/varnishtest/vtc_haproxy.c
+++ b/bin/varnishtest/vtc_haproxy.c
@@ -477,7 +477,7 @@ haproxy_new(const char *name)
h->cli = haproxy_cli_new(h);
AN(h->cli);
- bprintf(buf, "rm -rf %s ; mkdir -p \"%s\"", h->workdir, h->workdir);
+ bprintf(buf, "rm -rf \"%s\" ; mkdir -p \"%s\"", h->workdir, h->workdir);
AZ(system(buf));
VTAILQ_INSERT_TAIL(, h, list);
@@ -498,7 +498,7 @@ haproxy_delete(struct haproxy *h)
vtc_logclose(h->vl);
if (!leave_temp) {
- bprintf(buf, "rm -rf %s", h->workdir);
+ bprintf(buf, "rm -rf \"%s\"", h->workdir);
AZ(system(buf));
}
@@ -548,7 +548,7 @@ haproxy_start(struct haproxy *h)
vsb = VSB_new_auto();
AN(vsb);
- VSB_printf(vsb, "exec %s", h->filename);
+ VSB_printf(vsb, "exec \"%s\"", h->filename);
if (h->opt_check_mode)
VSB_printf(vsb, " -c");
else if (h->opt_daemon)
@@ -567,7 +567,7 @@ haproxy_start(struct haproxy *h)
bprintf(buf, "%s/pid", h->workdir);
h->pid_fn = strdup(buf);
AN(h->pid_fn);
- VSB_printf(vsb, " -p %s", h->pid_fn);
+ VSB_printf(vsb, " -p \"%s\"", h->pid_fn);
}
AZ(VSB_finish(vsb));
--
2.11.0
Re: [PATCH] REGTEST/MINOR: script: add run-regtests.sh script
On Thu, Nov 29, 2018 at 10:04:29PM +0100, Frederic Lecaille wrote: > Here is a patch for haproxy (named 0001-REGTEST*) to fix these issues. Works like a charm, thank you Fred! Now applied. Willy
Re: [PATCH] REGTEST/MINOR: script: add run-regtests.sh script
On 11/29/18 5:36 AM, Willy Tarreau wrote:
Hi guys,
On Wed, Nov 28, 2018 at 11:17:22AM +0100, Frederic Lecaille wrote:
Perhaps we should "chmod +x" this script.
Good point, done here.
However I'm now seeing this when starting it :
## Starting varnishtest ##
Testing with haproxy version: 1.8-dev0-3b0a6d-66
Assert error in start_test(), vtc_main.c line 283:
Condition((mkdir(tmpdir, 0711)) == 0) not true.
errno = 13 (Permission denied)
./scripts/run-regtests.sh: line 299: 21484 Aborted
$VARNISHTEST_PROGRAM $varnishtestparams $verbose $jobcount -l -k -t 10 $testlist
## Gathering failed results ##
find: `/proc/tty/driver': Permission denied
find: `/proc/1/task/1/fd': Permission denied
find: `/proc/1/task/1/fdinfo': Permission denied
find: `/proc/1/task/1/ns': Permission denied
find: `/proc/1/fd': Permission denied
find: `/proc/1/map_files': Permission denied
(...)
Then I stopped it using Ctrl-C.
I'm seeing a few minor issues we must still address :
- haproxy is started from the path, which means that on all systems
where it's installed, it will be the one provided by the system and
not the just built one which will be tested (as happened above),
which is confusing. I think we shouldn't search for haproxy in the
path but use $PWD/haproxy or ./haproxy or something like this.
- it seems there are some issues in the way TMPDIR/TESTDIR are created,
as it ended up with an empty TESTDIR. In my case, TMPDIR is not set,
so TESTDIR was set to /tmp/varnishtest_haproxy. mkdir worked (I now
have this directory), but a test is missing on this mkdir.
- the way the sub-directory is created is problematic on shared
development machines, as only the first user will own the directory
and will thus prevent other users from creating their own overthere,
thus it's probably preferable not to create an intermediary directory
in the end.
- in my case it's mktemp which failed :
++ date +%Y-%m-%d_%H-%M-%S
+ TESTRUNDATETIME=2018-11-29_05-03-43
+ TESTDIR=/tmp/varnishtest_haproxy
+ mkdir -p /tmp/varnishtest_haproxy
++ mktemp -d /tmp/varnishtest_haproxy/2018-11-29_05-03-43.
mktemp: cannot make temp dir
/tmp/varnishtest_haproxy/2018-11-29_05-03-43.: Invalid argument
+ TESTDIR=
I haven't checked why yet, but we definitely need to test the status
code for success as well.
- in my opinion the script is still missing a number of quotes when using
string variables, especially in the directory names. If someone is crazy
enough to have TMPDIR="/tmp/Temp Dir", then he'll have some fun.
- I'm seeing a linux-specific "rm -d" at the end, it's be better to
replace it with rmdir.
- there's "/usr/bin/env sh" at the top of the shell script. /bin/sh is
the portable one, I've spent lots of time in the past editing scripts
where env was forced to /usr/bin while it was placed in /bin on some
systems, so I'm pretty certain that this one is not portable at all :-)
Here is a patch for haproxy (named 0001-REGTEST*) to fix these issues.
Note the other patch for varnishtest which is also required (sent to PHK).
Fred.
>From 7d316b74e65be4d75802feefe3d17b37afddf04e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20L=C3=A9caille?=
Date: Thu, 29 Nov 2018 21:51:55 +0100
Subject: [PATCH] Add the support for spaces in filenames for the temporaty
working directory.
---
bin/varnishtest/vtc_haproxy.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/bin/varnishtest/vtc_haproxy.c b/bin/varnishtest/vtc_haproxy.c
index 9d12ed671..e361aad78 100644
--- a/bin/varnishtest/vtc_haproxy.c
+++ b/bin/varnishtest/vtc_haproxy.c
@@ -477,7 +477,7 @@ haproxy_new(const char *name)
h->cli = haproxy_cli_new(h);
AN(h->cli);
- bprintf(buf, "rm -rf %s ; mkdir -p %s", h->workdir, h->workdir);
+ bprintf(buf, "rm -rf %s ; mkdir -p \"%s\"", h->workdir, h->workdir);
AZ(system(buf));
VTAILQ_INSERT_TAIL(, h, list);
@@ -561,7 +561,7 @@ haproxy_start(struct haproxy *h)
VSB_printf(vsb, " %s", VSB_data(h->args));
- VSB_printf(vsb, " -f %s ", h->cfg_fn);
+ VSB_printf(vsb, " -f \"%s\" ", h->cfg_fn);
if (h->opt_worker || h->opt_daemon) {
bprintf(buf, "%s/pid", h->workdir);
@@ -754,7 +754,7 @@ haproxy_write_conf(const struct haproxy *h, const char *cfg, int auto_be)
vsb2 = VSB_new_auto();
AN(vsb2);
- VSB_printf(vsb, "global\n\tstats socket %s "
+ VSB_printf(vsb, "global\n\tstats socket \"%s\" "
"level admin mode 600\n", h->cli_fn);
VSB_printf(vsb, "stats socket \"fd@${cli}\" level admin\n");
AZ(VSB_cat(vsb, cfg));
--
2.11.0
>From bd6bc4956f3bd005af7c27cf83e78c8cd14aea02 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20L=C3=A9caille?=
Date: Thu, 29 Nov 2018 21:41:42 +0100
Subject: [PATCH] REGTEST: Fix several
Re: [PATCH] REGTEST/MINOR: script: add run-regtests.sh script
Hi Frederic, Op 29-11-2018 om 19:18 schreef Frederic Lecaille: On 11/29/18 8:47 AM, Willy Tarreau wrote: On Thu, Nov 29, 2018 at 05:36:35AM +0100, Willy Tarreau wrote: However I'm well aware that it's easier to work on improvements once the script is merged, so what I've done now is to merge it and create a temporary "reg-tests2" target in the makefile to use it without losing the existing one. This way everyone can work in parallel, and once the few issues seem reliably addressed, we can definitely replace the make target. Unfortunately ENOCOFFEE struck me this morning and I forgot to commit my local changes so I merged the unmodified version which replaces the "reg-test" target. Thus now we're condemned to quickly fix these small issues :-) Pieter, I am having a look at all these issues. Regards, Fred. If that means i don't have to do anything at this moment, thank you! (i suppose your turnaround time from issue>fix will also be shorter than waiting for my spare evening hour..) Ill start checking some requirements of existing .vtc . And perhaps writing new one's. Regards, PiBa-NL (Pieter)
Re: [PATCH] REGTEST/MINOR: script: add run-regtests.sh script
On 11/29/18 8:47 AM, Willy Tarreau wrote: On Thu, Nov 29, 2018 at 05:36:35AM +0100, Willy Tarreau wrote: However I'm well aware that it's easier to work on improvements once the script is merged, so what I've done now is to merge it and create a temporary "reg-tests2" target in the makefile to use it without losing the existing one. This way everyone can work in parallel, and once the few issues seem reliably addressed, we can definitely replace the make target. Unfortunately ENOCOFFEE struck me this morning and I forgot to commit my local changes so I merged the unmodified version which replaces the "reg-test" target. Thus now we're condemned to quickly fix these small issues :-) Pieter, I am having a look at all these issues. Regards, Fred.
Re: [PATCH] REGTEST/MINOR: script: add run-regtests.sh script
On Thu, Nov 29, 2018 at 05:36:35AM +0100, Willy Tarreau wrote: > However I'm well aware that it's easier to work on improvements once the > script is merged, so what I've done now is to merge it and create a > temporary "reg-tests2" target in the makefile to use it without losing > the existing one. This way everyone can work in parallel, and once the > few issues seem reliably addressed, we can definitely replace the make > target. Unfortunately ENOCOFFEE struck me this morning and I forgot to commit my local changes so I merged the unmodified version which replaces the "reg-test" target. Thus now we're condemned to quickly fix these small issues :-) I've found the problem related to mktemp, it uses "" as the suffix. On my distro the man page says : The mktemp utility takes the given filename template and overwrites a portion of it to create a unique filename. The template may be any filename with six (6) `Xs' appended to it, for example /tmp/tfile.XX. I've found that other systems demand at least 3 or at least 1. And the glibc's mktemp() calll wants 6 anyway or returns EINVAL (the one I got). Thus we should add 2 more "X" to make this work everywhere mktemp is present (and test for the output as well). Willy
Re: [PATCH] REGTEST/MINOR: script: add run-regtests.sh script
Hi guys, On Wed, Nov 28, 2018 at 11:17:22AM +0100, Frederic Lecaille wrote: > Perhaps we should "chmod +x" this script. Good point, done here. However I'm now seeing this when starting it : ## Starting varnishtest ## Testing with haproxy version: 1.8-dev0-3b0a6d-66 Assert error in start_test(), vtc_main.c line 283: Condition((mkdir(tmpdir, 0711)) == 0) not true. errno = 13 (Permission denied) ./scripts/run-regtests.sh: line 299: 21484 Aborted $VARNISHTEST_PROGRAM $varnishtestparams $verbose $jobcount -l -k -t 10 $testlist ## Gathering failed results ## find: `/proc/tty/driver': Permission denied find: `/proc/1/task/1/fd': Permission denied find: `/proc/1/task/1/fdinfo': Permission denied find: `/proc/1/task/1/ns': Permission denied find: `/proc/1/fd': Permission denied find: `/proc/1/map_files': Permission denied (...) Then I stopped it using Ctrl-C. I'm seeing a few minor issues we must still address : - haproxy is started from the path, which means that on all systems where it's installed, it will be the one provided by the system and not the just built one which will be tested (as happened above), which is confusing. I think we shouldn't search for haproxy in the path but use $PWD/haproxy or ./haproxy or something like this. - it seems there are some issues in the way TMPDIR/TESTDIR are created, as it ended up with an empty TESTDIR. In my case, TMPDIR is not set, so TESTDIR was set to /tmp/varnishtest_haproxy. mkdir worked (I now have this directory), but a test is missing on this mkdir. - the way the sub-directory is created is problematic on shared development machines, as only the first user will own the directory and will thus prevent other users from creating their own overthere, thus it's probably preferable not to create an intermediary directory in the end. - in my case it's mktemp which failed : ++ date +%Y-%m-%d_%H-%M-%S + TESTRUNDATETIME=2018-11-29_05-03-43 + TESTDIR=/tmp/varnishtest_haproxy + mkdir -p /tmp/varnishtest_haproxy ++ mktemp -d /tmp/varnishtest_haproxy/2018-11-29_05-03-43. mktemp: cannot make temp dir /tmp/varnishtest_haproxy/2018-11-29_05-03-43.: Invalid argument + TESTDIR= I haven't checked why yet, but we definitely need to test the status code for success as well. - in my opinion the script is still missing a number of quotes when using string variables, especially in the directory names. If someone is crazy enough to have TMPDIR="/tmp/Temp Dir", then he'll have some fun. - I'm seeing a linux-specific "rm -d" at the end, it's be better to replace it with rmdir. - there's "/usr/bin/env sh" at the top of the shell script. /bin/sh is the portable one, I've spent lots of time in the past editing scripts where env was forced to /usr/bin while it was placed in /bin on some systems, so I'm pretty certain that this one is not portable at all :-) However I'm well aware that it's easier to work on improvements once the script is merged, so what I've done now is to merge it and create a temporary "reg-tests2" target in the makefile to use it without losing the existing one. This way everyone can work in parallel, and once the few issues seem reliably addressed, we can definitely replace the make target. Thanks! Willy
Re: [PATCH] REGTEST/MINOR: script: add run-regtests.sh script
On 11/27/18 11:17 PM, PiBa-NL wrote: Hi Frederic, Willy, Op 27-11-2018 om 15:00 schreef Frederic Lecaille: On 11/27/18 10:44 AM, Frederic Lecaille wrote: On 11/27/18 9:52 AM, Willy Tarreau wrote: Hi guys, On Tue, Nov 27, 2018 at 09:45:25AM +0100, Frederic Lecaille wrote: I put the script in the /reg-tests/ folder. Maybe it should have been besides the Makefile in the / root ? Yes I think it should be placed at the same level as the Makefile. Well, we already have a "scripts" directory with the stuff used for release and backport management. I think it perfectly has its place there. /scripts/ sounds good. Note that the reg tests must be run from the Makefile with "reg-tests" target and possibly other arguments/variables. Willy recently added REG_TEST_FILES variable. I've changed the the script to include the LEVEL parameter almost the way the Makefile used it, changed behavior though so without the parameter it it runs all tests. Ok. I am sorry Pieter a remaining detail I should have mentioned before: + for i in $(find $TESTDIR/ -type d -name "vtc.*"); + do + echo "## $(cat $i/INFO) ##" + echo "## test results in: $i" + grep -- $i/LOG + + echo "## $(cat $i/INFO) ##" >> $TESTDIR/failedtests.log + echo "## test results in: $i" >> $TESTDIR/failedtests.log + grep -- $i/LOG >> $TESTDIR/failedtests.log + echo >> $TESTDIR/failedtests.log + done may be shortened thanks to tee command like that: cat <<- EOF | tee $TESDIR/failedtests.log . . EOF Removed some spaces for indentation which became part of the output. To make <<- operator work (I think it is portable) we must add TABS to align the code. And indeed the remaining spaces are taken into an account. Let's forget that, it is ok with your modifications. I have tested you script. For me it is OK. Good job! Thank you a lot Pieter. OK just let me know what to do with this, should I merge it as-is and expect minor updates later, or do you or Pieter want to resend an updated version ? I can adapt, let me know. I have modified Pieter's patch for the modification mentioned above. Seems to work ;) Willy, Here is a better patch which takes into an account the modification above and yours (the script is added in "tests" directory). I think Willy mentioned a 'scripts' directory? Changed patch to include that as well. Aww yes, sorry. You can merge it as-is. Regards, Fred New path attached, which includes a LEVEL check. And a modification of the Makefile to call the ./scripts/run-regtests.sh Please can someone check it again before merging.?. Thanks guys :). Perhaps we should "chmod +x" this script. It is OK on my side. Thank you Pieter. Fred.
Re: [PATCH] REGTEST/MINOR: script: add run-regtests.sh script
Hi Pieter, On Tue, Nov 27, 2018 at 11:17:42PM +0100, PiBa-NL wrote: > I've changed the the script to include the LEVEL parameter almost the way > the Makefile used it, changed behavior though so without the parameter it it > runs all tests. (...) > New path attached, which includes a LEVEL check. > And a modification of the Makefile to call the ./scripts/run-regtests.sh Thank you. > Please can someone check it again before merging.?. Thanks guys :). I've run a quick check and am mostly OK with it, though I'll wait for Fred to re-check this morning before applying it. I'll just perform a very minor modification below : > diff --git a/Makefile b/Makefile (...) > + ./scripts/run-regtests.sh --LEVEL "$$LEVEL" $$REG_TEST_FILES I'll reuse $(REG_TEST_FILES) instead of $$REG_TEST_FILES. The difference is that the former is resolved by make and will take care of makefile variables, while the latter is resolved by the shell and will only see exported environment variables. This is fairly minor, no need to respin a patch for this ;-) Thanks, Willy
Re: [PATCH] REGTEST/MINOR: script: add run-regtests.sh script
Hi Frederic, Willy,
Op 27-11-2018 om 15:00 schreef Frederic Lecaille:
On 11/27/18 10:44 AM, Frederic Lecaille wrote:
On 11/27/18 9:52 AM, Willy Tarreau wrote:
Hi guys,
On Tue, Nov 27, 2018 at 09:45:25AM +0100, Frederic Lecaille wrote:
I put the script in the /reg-tests/ folder. Maybe it should have been
besides the Makefile in the / root ?
Yes I think it should be placed at the same level as the Makefile.
Well, we already have a "scripts" directory with the stuff used for
release and backport management. I think it perfectly has its place
there.
/scripts/ sounds good.
Note that the reg tests must be run from the Makefile with
"reg-tests" target and possibly other arguments/variables.
Willy recently added REG_TEST_FILES variable.
I've changed the the script to include the LEVEL parameter almost the
way the Makefile used it, changed behavior though so without the
parameter it it runs all tests.
I am sorry Pieter a remaining detail I should have mentioned before:
+ for i in $(find $TESTDIR/ -type d -name "vtc.*");
+ do
+ echo "## $(cat $i/INFO) ##"
+ echo "## test results in: $i"
+ grep -- $i/LOG
+
+ echo "## $(cat $i/INFO) ##" >> $TESTDIR/failedtests.log
+ echo "## test results in: $i" >> $TESTDIR/failedtests.log
+ grep -- $i/LOG >> $TESTDIR/failedtests.log
+ echo >> $TESTDIR/failedtests.log
+ done
may be shortened thanks to tee command like that:
cat <<- EOF | tee $TESDIR/failedtests.log
.
.
EOF
Removed some spaces for indentation which became part of the output.
I have tested you script. For me it is OK. Good job!
Thank you a lot Pieter.
OK just let me know what to do with this, should I merge it as-is and
expect minor updates later, or do you or Pieter want to resend an
updated version ? I can adapt, let me know.
I have modified Pieter's patch for the modification mentioned above.
Seems to work ;)
Willy,
Here is a better patch which takes into an account the modification
above and yours (the script is added in "tests" directory).
I think Willy mentioned a 'scripts' directory? Changed patch to include
that as well.
You can merge it as-is.
Regards,
Fred
New path attached, which includes a LEVEL check.
And a modification of the Makefile to call the ./scripts/run-regtests.sh
Please can someone check it again before merging.?. Thanks guys :).
Regards,
PiBa-NL (Pieter)
From 989bf7ccbfd849deed450291121cdcc68796ba64 Mon Sep 17 00:00:00 2001
From: PiBa-NL
Date: Tue, 27 Nov 2018 22:26:38 +0100
Subject: [PATCH] REGTEST/MINOR: script: add run-regtests.sh script
Some tests require a minimal haproxy version or compilation options to be
able to run successfully. This script allows to add 'requirements' to tests
to check so they will automatically be skipped if a requirement is not met.
The script supports several parameters to slightly modify its behavior
including the directories to search for tests.
Also some features are not available for certain OS's these can also
be 'excluded', this should allow for the complete set of test cases to be
run on any OS against any haproxy release without 'expected failures'.
The test .vtc files will need to be modified to include their 'requirements'
by listing including text options as shown below:
#EXCLUDE_TARGETS=dos,freebsd,windows
#REQUIRE_OPTIONS=ZLIB,OPENSSL,LUA
#REQUIRE_VERSION=0.0
#REQUIRE_VERSION_BELOW=99.9,
When excluding a OS by its TARGET, please do make a comment why the test
can not succeed on that TARGET.
---
Makefile| 25 +---
scripts/run-regtests.sh | 317
2 files changed, 318 insertions(+), 24 deletions(-)
create mode 100644 scripts/run-regtests.sh
diff --git a/Makefile b/Makefile
index 6d7a0159..aa6d66b9 100644
--- a/Makefile
+++ b/Makefile
@@ -1094,28 +1094,5 @@ opts:
# LEVEL 3 scripts are low interest scripts (prefixed with 'l' letter).
# LEVEL 4 scripts are in relation with bugs they help to reproduce (prefixed
with 'b' letter).
reg-tests:
- $(Q)if [ ! -x "$(VARNISHTEST_PROGRAM)" ]; then \
- echo "Please make the VARNISHTEST_PROGRAM variable point to the
location of the varnishtest program."; \
- exit 1; \
- fi
- $(Q)export LEVEL=$${LEVEL:-1}; \
- if [ $$LEVEL = 1 ] ; then \
- EXPR='h*.vtc'; \
- elif [ $$LEVEL = 2 ] ; then \
- EXPR='s*.vtc'; \
- elif [ $$LEVEL = 3 ] ; then \
- EXPR='l*.vtc'; \
- elif [ $$LEVEL = 4 ] ; then \
- EXPR='b*.vtc'; \
- fi ; \
- if [ -n "$(REG_TEST_FILES)" ] ; then \
- err=0; \
- for n in $(REG_TEST_FILES); do \
- HAPROXY_PROGRAM=$${HAPROXY_PROGRAM:-$$PWD/haproxy}
$(VARNISHTEST_PROGRAM) -l -t5 $$n || ((err++)); \
- done; \
- exit $$err; \
- elif [ -n "$$EXPR" ] ; then \
- find reg-tests -type f -name "$$EXPR" -print0 | \
-
Re: [PATCH] REGTEST/MINOR: script: add run-regtests.sh script
On 11/27/18 10:44 AM, Frederic Lecaille wrote:
On 11/27/18 9:52 AM, Willy Tarreau wrote:
Hi guys,
On Tue, Nov 27, 2018 at 09:45:25AM +0100, Frederic Lecaille wrote:
I put the script in the /reg-tests/ folder. Maybe it should have been
besides the Makefile in the / root ?
Yes I think it should be placed at the same level as the Makefile.
Well, we already have a "scripts" directory with the stuff used for
release and backport management. I think it perfectly has its place
there.
I am sorry Pieter a remaining detail I should have mentioned before:
+ for i in $(find $TESTDIR/ -type d -name "vtc.*");
+ do
+ echo "## $(cat $i/INFO) ##"
+ echo "## test results in: $i"
+ grep -- $i/LOG
+
+ echo "## $(cat $i/INFO) ##" >> $TESTDIR/failedtests.log
+ echo "## test results in: $i" >> $TESTDIR/failedtests.log
+ grep -- $i/LOG >> $TESTDIR/failedtests.log
+ echo >> $TESTDIR/failedtests.log
+ done
may be shortened thanks to tee command like that:
cat <<- EOF | tee $TESDIR/failedtests.log
.
.
EOF
I have tested you script. For me it is OK. Good job!
Thank you a lot Pieter.
OK just let me know what to do with this, should I merge it as-is and
expect minor updates later, or do you or Pieter want to resend an
updated version ? I can adapt, let me know.
I have modified Pieter's patch for the modification mentioned above.
Seems to work ;)
Willy,
Here is a better patch which takes into an account the modification
above and yours (the script is added in "tests" directory).
You can merge it as-is.
Regards,
Fred
>From 4432c10a0a822619c152aa187f18b2f6478ac565 Mon Sep 17 00:00:00 2001
From: PiBa-NL
Date: Sun, 25 Nov 2018 16:46:44 +0100
Subject: [PATCH] REGTEST/MINOR: script: add run-regtests.sh script
Some tests require a minimal haproxy version or compilation options to be
able to run successfully. This script allows to add 'requirements' to tests
to check so they will automatically be skipped if a requirement is not met.
The script supports several parameters to slightly modify its behavior
including the directories to search for tests.
Also some features are not available for certain OS's these can also
be 'excluded', this should allow for the complete set of test cases to be
run on any OS against any haproxy release without 'expected failures'.
The test .vtc files will need to be modified to include their 'requirements'
by listing including text options as shown below:
#EXCLUDE_TARGETS=dos,freebsd,windows
#REQUIRE_OPTIONS=ZLIB,OPENSSL,LUA
#REQUIRE_VERSION=0.0
#REQUIRE_VERSION_BELOW=99.9,
When excluding a OS by its TARGET, please do make a comment why the test
can not succeed on that TARGET.
---
tests/run-regtests.sh | 300 ++
1 file changed, 300 insertions(+)
create mode 100644 tests/run-regtests.sh
diff --git a/tests/run-regtests.sh b/tests/run-regtests.sh
new file mode 100644
index ..1094117f
--- /dev/null
+++ b/tests/run-regtests.sh
@@ -0,0 +1,300 @@
+#!/usr/bin/env sh
+
+if [ "$1" = "--help" ]; then
+ cat << EOF
+### run-regtests.sh ###
+ Running run-regtests.sh --help shows this information about how to use it
+
+ Run without parameters to run all tests in the current folder (including subfolders)
+run-regtests.sh
+
+ Provide paths to run tests from (including subfolders):
+run-regtests.sh ./tests1 ./tests2
+
+ Parameters:
+--j , To run varnishtest with multiple jobs / threads for a faster overall result
+ run-regtests.sh ./fasttest --j 16
+
+--v, to run verbose
+ run-regtests.sh --v, disables the default varnishtest 'quiet' parameter
+
+--varnishtestparams , passes custom ARGS to varnishtest
+ run-regtests.sh --varnishtestparams "-n 10"
+
+ Including text below into a .vtc file will check for its requirements
+ related to haproxy's target and compilation options
+# Below targets are not capable of completing this test succesfully
+#EXCLUDE_TARGET=freebsd, abns sockets are not available on freebsd
+
+#EXCLUDE_TARGETS=dos,freebsd,windows
+
+# Below option is required to complete this test succesfully
+#REQUIRE_OPTION=OPENSSL, this test needs OPENSSL compiled in.
+
+#REQUIRE_OPTIONS=ZLIB,OPENSSL,LUA
+
+# To define a range of versions that a test can run with:
+#REQUIRE_VERSION=0.0
+#REQUIRE_VERSION_BELOW=99.9
+
+ Configure environment variables to set the haproxy and varnishtest binaries to use
+setenv HAPROXY_PROGRAM /usr/local/sbin/haproxy
+setenv VARNISHTEST_PROGRAM /usr/local/bin/varnishtest
+EOF
+ return
+fi
+
+_startswith() {
+ _str="$1"
+ _sub="$2"
+ echo "$_str" | grep "^$_sub" >/dev/null 2>&1
+}
+
+_findtests() {
+ set -f
+ for i in $( find "$1" -name "*.vtc" ); do
+skiptest=
+require_version="$(grep "#REQUIRE_VERSION=" "$i" | sed -e 's/.*=//')"
+require_version_below="$(grep "#REQUIRE_VERSION_BELOW=" "$i" | sed -e 's/.*=//')"
+require_options="$(grep
Re: [PATCH] REGTEST/MINOR: script: add run-regtests.sh script
On 11/27/18 9:52 AM, Willy Tarreau wrote:
Hi guys,
On Tue, Nov 27, 2018 at 09:45:25AM +0100, Frederic Lecaille wrote:
I put the script in the /reg-tests/ folder. Maybe it should have been
besides the Makefile in the / root ?
Yes I think it should be placed at the same level as the Makefile.
Well, we already have a "scripts" directory with the stuff used for
release and backport management. I think it perfectly has its place
there.
I am sorry Pieter a remaining detail I should have mentioned before:
+ for i in $(find $TESTDIR/ -type d -name "vtc.*");
+ do
+echo "## $(cat $i/INFO) ##"
+echo "## test results in: $i"
+grep -- $i/LOG
+
+echo "## $(cat $i/INFO) ##" >> $TESTDIR/failedtests.log
+echo "## test results in: $i" >> $TESTDIR/failedtests.log
+grep -- $i/LOG >> $TESTDIR/failedtests.log
+echo >> $TESTDIR/failedtests.log
+ done
may be shortened thanks to tee command like that:
cat <<- EOF | tee $TESDIR/failedtests.log
.
.
EOF
I have tested you script. For me it is OK. Good job!
Thank you a lot Pieter.
OK just let me know what to do with this, should I merge it as-is and
expect minor updates later, or do you or Pieter want to resend an
updated version ? I can adapt, let me know.
I have modified Pieter's patch for the modification mentioned above.
Seems to work ;)
Fred.
>From 4432c10a0a822619c152aa187f18b2f6478ac565 Mon Sep 17 00:00:00 2001
From: PiBa-NL
Date: Sun, 25 Nov 2018 16:46:44 +0100
Subject: [PATCH] REGTEST/MINOR: script: add run-regtests.sh script
Some tests require a minimal haproxy version or compilation options to be
able to run successfully. This script allows to add 'requirements' to tests
to check so they will automatically be skipped if a requirement is not met.
The script supports several parameters to slightly modify its behavior
including the directories to search for tests.
Also some features are not available for certain OS's these can also
be 'excluded', this should allow for the complete set of test cases to be
run on any OS against any haproxy release without 'expected failures'.
The test .vtc files will need to be modified to include their 'requirements'
by listing including text options as shown below:
#EXCLUDE_TARGETS=dos,freebsd,windows
#REQUIRE_OPTIONS=ZLIB,OPENSSL,LUA
#REQUIRE_VERSION=0.0
#REQUIRE_VERSION_BELOW=99.9,
When excluding a OS by its TARGET, please do make a comment why the test
can not succeed on that TARGET.
---
reg-tests/run-regtests.sh | 300 ++
1 file changed, 300 insertions(+)
create mode 100644 reg-tests/run-regtests.sh
diff --git a/reg-tests/run-regtests.sh b/reg-tests/run-regtests.sh
new file mode 100644
index ..1094117f
--- /dev/null
+++ b/reg-tests/run-regtests.sh
@@ -0,0 +1,300 @@
+#!/usr/bin/env sh
+
+if [ "$1" = "--help" ]; then
+ cat << EOF
+### run-regtests.sh ###
+ Running run-regtests.sh --help shows this information about how to use it
+
+ Run without parameters to run all tests in the current folder (including subfolders)
+run-regtests.sh
+
+ Provide paths to run tests from (including subfolders):
+run-regtests.sh ./tests1 ./tests2
+
+ Parameters:
+--j , To run varnishtest with multiple jobs / threads for a faster overall result
+ run-regtests.sh ./fasttest --j 16
+
+--v, to run verbose
+ run-regtests.sh --v, disables the default varnishtest 'quiet' parameter
+
+--varnishtestparams , passes custom ARGS to varnishtest
+ run-regtests.sh --varnishtestparams "-n 10"
+
+ Including text below into a .vtc file will check for its requirements
+ related to haproxy's target and compilation options
+# Below targets are not capable of completing this test succesfully
+#EXCLUDE_TARGET=freebsd, abns sockets are not available on freebsd
+
+#EXCLUDE_TARGETS=dos,freebsd,windows
+
+# Below option is required to complete this test succesfully
+#REQUIRE_OPTION=OPENSSL, this test needs OPENSSL compiled in.
+
+#REQUIRE_OPTIONS=ZLIB,OPENSSL,LUA
+
+# To define a range of versions that a test can run with:
+#REQUIRE_VERSION=0.0
+#REQUIRE_VERSION_BELOW=99.9
+
+ Configure environment variables to set the haproxy and varnishtest binaries to use
+setenv HAPROXY_PROGRAM /usr/local/sbin/haproxy
+setenv VARNISHTEST_PROGRAM /usr/local/bin/varnishtest
+EOF
+ return
+fi
+
+_startswith() {
+ _str="$1"
+ _sub="$2"
+ echo "$_str" | grep "^$_sub" >/dev/null 2>&1
+}
+
+_findtests() {
+ set -f
+ for i in $( find "$1" -name "*.vtc" ); do
+skiptest=
+require_version="$(grep "#REQUIRE_VERSION=" "$i" | sed -e 's/.*=//')"
+require_version_below="$(grep "#REQUIRE_VERSION_BELOW=" "$i" | sed -e 's/.*=//')"
+require_options="$(grep "#REQUIRE_OPTIONS=" "$i" | sed -e 's/.*=//')"
+exclude_targets=",$(grep "#EXCLUDE_TARGETS=" "$i" | sed -e 's/.*=//'),"
+
+if [ -n "$require_version" ]; then
+ if [ $(_version
Re: [PATCH] REGTEST/MINOR: script: add run-regtests.sh script
Hi guys, On Tue, Nov 27, 2018 at 09:45:25AM +0100, Frederic Lecaille wrote: > > I put the script in the /reg-tests/ folder. Maybe it should have been > > besides the Makefile in the / root ? > > Yes I think it should be placed at the same level as the Makefile. Well, we already have a "scripts" directory with the stuff used for release and backport management. I think it perfectly has its place there. > I am sorry Pieter a remaining detail I should have mentioned before: > > + for i in $(find $TESTDIR/ -type d -name "vtc.*"); > + do > +echo "## $(cat $i/INFO) ##" > +echo "## test results in: $i" > +grep -- $i/LOG > + > +echo "## $(cat $i/INFO) ##" >> $TESTDIR/failedtests.log > +echo "## test results in: $i" >> $TESTDIR/failedtests.log > +grep -- $i/LOG >> $TESTDIR/failedtests.log > +echo >> $TESTDIR/failedtests.log > + done > > may be shortened thanks to tee command like that: > > cat <<- EOF | tee $TESDIR/failedtests.log > . > . > EOF > > I have tested you script. For me it is OK. Good job! > Thank you a lot Pieter. OK just let me know what to do with this, should I merge it as-is and expect minor updates later, or do you or Pieter want to resend an updated version ? I can adapt, let me know. thanks! Willy
Re: [PATCH] REGTEST/MINOR: script: add run-regtests.sh script
On 11/25/18 4:59 PM, PiBa-NL wrote: Hi Frederic, Willy, Hi Pieter, Added the varnishtest script we have been discussing as a .patch this time. I put the script in the /reg-tests/ folder. Maybe it should have been besides the Makefile in the / root ? Yes I think it should be placed at the same level as the Makefile. Note that the reg tests must be run from the Makefile with "reg-tests" target and possibly other arguments/variables. Willy recently added REG_TEST_FILES variable. Also i put a bit of comments into the commit. I hope it is okay like this? If not, feel free to comment on them or change them as required. I am sorry Pieter a remaining detail I should have mentioned before: + for i in $(find $TESTDIR/ -type d -name "vtc.*"); + do +echo "## $(cat $i/INFO) ##" +echo "## test results in: $i" +grep -- $i/LOG + +echo "## $(cat $i/INFO) ##" >> $TESTDIR/failedtests.log +echo "## test results in: $i" >> $TESTDIR/failedtests.log +grep -- $i/LOG >> $TESTDIR/failedtests.log +echo >> $TESTDIR/failedtests.log + done may be shortened thanks to tee command like that: cat <<- EOF | tee $TESDIR/failedtests.log . . EOF I have tested you script. For me it is OK. Good job! Thank you a lot Pieter. Once this one is 'accepted' ill create a few patches for the existing .vtc files to include their requirements. (at least the more obvious ones..) Regards, PiBa-NL (Pieter)
