Your patch has been applied to the master branch.
Since this is not depending on 5/7, no reason to stall - so, here we
are.
Comments...
- it's sourcing t_client.rc, but only for the SUDO definition - this is
a bit confusing, and might warrant a comment in the script
- maybe make this a bit more verbose what it does?
make check-TESTS
make[4]: Entering directory
'/rhome/gert/src/openvpn-maint/test-build-master-ossl111/tests'
PASS: t_net.sh
(all our other tests print "test this, ok, test that, ok", which I find
good feedback)
- maybe feed the list of things to test from a "t_net.rc" file, so you
can more easily extend the test set (when developing)? Right now,
the test is hardcoded in the C source, but the script needs to know
how many tests there are to call them in sequence...
- "make check" in an --enable-iproute2 build fails like this:
./../openvpn/tests/t_net.sh: eval: line 150: syntax error near unexpected token
`;'
./../openvpn/tests/t_net.sh: eval: line 150: `;'
FAIL: t_net.sh
I expected it to fail in some interesting way, but this looks like
it at least needs more sanity checking on setting of $OUT and $CMD
(or maybe fail more explicitly for non-sitnl builds).
- code wise:
t_scripts = t_client.sh
+test_scripts = t_net.sh
+test_scripts += t_client.sh
test_scripts += t_lpback.sh t_cltsrv.sh
... why are we having two lines that add one script each and one
line that adds two scripts here? Why not just "one line for all"?
- if you want bash, ask for bash...
+#!/bin/sh
..
+# Note: statements in the rest of the script may not even pass syntax check on
+# solaris/bsd. It uses /bin/bash
/bin/sh on Linux will not always be bash (dash on Debian, for example),
so either do not use bash features, or ask for /bin/bash.
- comments taken over verbatim from t_client.sh are more confusing
than useful here:
+# make sure we have permissions to run ifconfig/route from OpenVPN
+# can't use "id -u" here - doesn't work on Solaris
- all the KILL_EXEC stuff should be removed from t_net.sh - you never
run "sudo kill <something>", so don't carry over that code
(we need to explicitely test this in t_client.sh because otherwise
we might end up with openvpn binaries that we have started in the
background and cannot kill anymore - "sudo openvpn" allowed, but
"sudo kill" not allowed)
- in the tests: good practice is to always use 2001:db8::/32 for
"demo and testing", not addresses that might be assigned to someone
else one day. But also use an ULA network (fdxx:xxxx:xxxx::/48, with
"xx" selected by rolling a dice) in case "something" handles these
differently - ULA is the new RFC1918 stuff, globally unique but still
not routed.
commit c4d5bcd7c90abbab2378ac865e326933b0ff1e1c
Author: Antonio Quartulli
Date: Wed Dec 19 15:01:17 2018 +1000
unit tests: implement test for sitnl
Signed-off-by: Antonio Quartulli <[email protected]>
Acked-by: Arne Schwabe <[email protected]>
Message-Id: <[email protected]>
URL:
https://www.mail-archive.com/[email protected]/msg18027.html
Signed-off-by: Gert Doering <[email protected]>
--
kind regards,
Gert Doering
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
