On 06/21/2018 04:53 AM, Willy Tarreau wrote:
Hi Daniel,

On Wed, Jun 20, 2018 at 10:28:43AM -0400, Daniel Corbett wrote:
+shell -expect "used:0" {
+    echo "show table http1" |socat ${tmpdir}/h1/stats.sock -
                                 ^^^^^

This is the point where it will start to require that we organize the
reg tests better. First, socat is rarely installed by default so we'll
have to mention that it's required. Second, socat introduces half a
second delay before quitting, making it impractical for the quick
automated tests that we expect developers to run frequently.

The dependency on socat makes me think we could probably put all of such
tests in a specific sub-directory. However, I predict that we will also
create a number of other ones which will be slower than average and which
will be unrelated to the CLI.

Maybe we could simply introduce levels :
   - level 1 (the default) would contain only the immediate tests that cover
     the internal state machine and HTTP compliance (the things we break the
     most often by side effets when fixing a bug in the same area). Basically
     we should expect to be able to run 100 tests in a second there and there
     should be zero excuse for not running them before committing a patch
     affecting a sensitive area.

   - level 2 would cover some extra parts requiring a bit more time (eg:
     CLI commands, horrible stuff involving tcploop) and would probably
     be needed only when trying to ensure that a fix doesn't break
     something unexpected.

   - level 3 would be the painful one that we already know nobody will dare
     to run. They would cover timeouts, health checks, etc. All the stuff
     that takes multiple seconds per test would be there. They may occasionally
     be run by a dev during lunch time, or at night by automated bots.


Then we could issue "make reg-tests" to run level 1 by default or
"make reg-tests LEVEL=<level>" for the other ones. The idea is that I would
*really* like to encourage developers to run some basic tests before sending
patches, and we all know that none of us will accept to run them if they take
more time than what is needed to divert us (ie if you have time to switch to
reading your mails while the test runs, we won't run them because this will
create distraction).


I have attached #0003 patch for that in addition to these ones:

#0001 : as would say Olivier "Ooops, I'am an idiot etc". reg-tests/ssl/h00000.vtc did not run any https request.

#0002 : set the default value of HAPROXY_PROGRAM environment variable.

We will have to change the class of the already existing reg test files.
>From bbbef595937170b87d0707780968d031c95ca9e4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20L=C3=A9caille?= <flecai...@haproxy.com>
Date: Mon, 25 Jun 2018 11:15:43 +0200
Subject: [PATCH 3/3] REGTEST/MINOR: Add levels to reg-tests target.

With this patch we can provide LEVEL environment variable when
running reg-tests Makefile targe (reg testing) to set the execution
level of the reg-tests make target to run.

LEVEL default value is 1.

LEVEL=1 is to run all h*.vtc files which are the most important
reg testing files (to test haproxy core, HTTP compliance etc).

LEVEL=2 is to run all s*.vtc files which are a bit slow tests,
for instance tests requiring external programs (curl, socat etc).

LEVEL=3 is to run all l*.vtc files which are test files with again
more slow or with little interest.
---
 Makefile | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 9ea3e80..817161f 100644
--- a/Makefile
+++ b/Makefile
@@ -1004,6 +1004,16 @@ reg-tests:
 		echo "Please make the VARNISHTEST_PROGRAM variable point to the location of the varnishtest program."; \
 		exit 1; \
 	fi
-	@find reg-tests -type f -name "*.vtc" -print0 | \
-	   HAPROXY_PROGRAM=$${HAPROXY_PROGRAM:-$$PWD/haproxy} xargs -0 $(VARNISHTEST_PROGRAM) -l -t5
+	@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'; \
+	fi ; \
+	if [ -n "$$EXPR" ] ; then \
+	   find reg-tests -type f -name "$$EXPR" -print0 | \
+	      HAPROXY_PROGRAM=$${HAPROXY_PROGRAM:-$$PWD/haproxy} xargs -r -0 $(VARNISHTEST_PROGRAM) -l -t5 ; \
+	fi
 .PHONY: reg-tests
-- 
2.1.4

>From 820602fa642c3143c3884056775a7077bf99c2eb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20L=C3=A9caille?= <flecai...@haproxy.com>
Date: Mon, 25 Jun 2018 10:24:37 +0200
Subject: [PATCH 2/3] REGTEST/MINOR: Set HAPROXY_PROGRAM default value.

With this patch, we set HAPROXY_PROGRAM environment variable
default value to the haproxy executable of the current working directory.
So, if the current directory is the haproxy sources directory,
the reg-tests Makefile target may be run with this shorter command:

  $ VARNISTEST_PROGRAM=<...> make reg-tests

in place of

  $ VARNISTEST_PROGRAM=<...> HAPROXY_PROGRAM=<...> make reg-tests
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 69f6ebb..9ea3e80 100644
--- a/Makefile
+++ b/Makefile
@@ -1005,5 +1005,5 @@ reg-tests:
 		exit 1; \
 	fi
 	@find reg-tests -type f -name "*.vtc" -print0 | \
-	   xargs -0 $(VARNISHTEST_PROGRAM) -l -t5
+	   HAPROXY_PROGRAM=$${HAPROXY_PROGRAM:-$$PWD/haproxy} xargs -0 $(VARNISHTEST_PROGRAM) -l -t5
 .PHONY: reg-tests
-- 
2.1.4

>From d6e32684e9384edd567d3cbdeddd3f19ebb3ae55 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20L=C3=A9caille?= <flecai...@haproxy.com>
Date: Fri, 22 Jun 2018 22:55:07 +0200
Subject: [PATCH 1/3] REGTEST/MINOR: Wrong URI in a reg test for SSL/TLS.

Fix typos where http:// URIs were used in place of https://.
---
 reg-tests/ssl/h00000.vtc | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/reg-tests/ssl/h00000.vtc b/reg-tests/ssl/h00000.vtc
index 5b0a391..0765cb4 100644
--- a/reg-tests/ssl/h00000.vtc
+++ b/reg-tests/ssl/h00000.vtc
@@ -31,15 +31,11 @@ haproxy h1 -conf {
     http-request redirect location /
 } -start
 
-
-# Note that with such a HAProxy configuration and curl status is 52
-# (Empty reply from server).
-
-process p1 "curl -i -k http://${h1_frt_addr}:${h1_frt_port}"; -expect-exit 52 -start
-process p2 "curl -i -k http://${h1_frt_addr}:${h1_frt_port}"; -expect-exit 52 -start
-process p3 "curl -i -k http://${h1_frt_addr}:${h1_frt_port}"; -expect-exit 52 -start
-process p4 "curl -i -k http://${h1_frt_addr}:${h1_frt_port}"; -expect-exit 52 -start
-process p5 "curl -i -k http://${h1_frt_addr}:${h1_frt_port}"; -expect-exit 52 -start
+process p1 "curl -i -k https://${h1_frt_addr}:${h1_frt_port}"; -start
+process p2 "curl -i -k https://${h1_frt_addr}:${h1_frt_port}"; -start
+process p3 "curl -i -k https://${h1_frt_addr}:${h1_frt_port}"; -start
+process p4 "curl -i -k https://${h1_frt_addr}:${h1_frt_port}"; -start
+process p5 "curl -i -k https://${h1_frt_addr}:${h1_frt_port}"; -start
 
 process p1 -wait
 process p2 -wait
-- 
2.1.4

Reply via email to