diff mbox series

[Openvpn-devel] t_client.sh: correctly report all failed instances in summary

Message ID 20200626082743.15397-1-gert@greenie.muc.de
State Accepted
Headers
Series [Openvpn-devel] t_client.sh: correctly report all failed instances in summary |

Commit Message

Gert Doering June 25, 2020, 10:27 p.m. UTC 
  t_client.sh reports a summary at the end:

  Test sets succeeded: none.
  Test sets failed: 1 2 3 4 5.

for tests that are skipped due to the pre-test ping check ("vpn target
IP must not ping before VPN ist started") the script forgot to add
the instance number to the summary line.  Fixed.

Signed-off-by: Gert Doering <gert@greenie.muc.de>
---
 tests/t_client.sh.in | 1 +
 1 file changed, 1 insertion(+)

Comments

Antonio Quartulli July 3, 2020, 7:11 a.m. UTC | #1 
Hi,

On 26/06/2020 10:27, Gert Doering wrote:
> t_client.sh reports a summary at the end:
> 
>   Test sets succeeded: none.
>   Test sets failed: 1 2 3 4 5.
> 
> for tests that are skipped due to the pre-test ping check ("vpn target
> IP must not ping before VPN ist started") the script forgot to add
> the instance number to the summary line.  Fixed.
> 
> Signed-off-by: Gert Doering <gert@greenie.muc.de>

Simple enough and pretty clear. This is the only FAIL case where we set
the exit code, print a message, but don't append the test to the
SUMMARY_FAIL.

This is fixed now.

I can't easily come up with a t_client instance that verifies this
failure, but I trust Gert to have done so after having spotted the issue.

Acked-by: Antonio Quartulli <a@unstable.cc>
Gert Doering July 3, 2020, 8:50 a.m. UTC | #2 
Hi,

On Fri, Jul 03, 2020 at 07:11:04PM +0200, Antonio Quartulli wrote:
> On 26/06/2020 10:27, Gert Doering wrote:
> > t_client.sh reports a summary at the end:
> > 
> >   Test sets succeeded: none.
> >   Test sets failed: 1 2 3 4 5.
[..]
> I can't easily come up with a t_client instance that verifies this
> failure, but I trust Gert to have done so after having spotted the issue.

Actually I was wondering why my failed tests did not show up in the
summary in some cases :-) - so I had the perfect test case up front.

(How to reproduce is easy: just run one of the test VPN instances
manually before starting t_client.sh - it will fail all/most of the
test due to "these addresse must not ping before we start openvpn!")

gert
Gert Doering July 3, 2020, 11:51 a.m. UTC | #3 
Patch has been applied to the master branch.

commit ec33bae311e7f3549b05de4a4c92fa7bc7144d29
Author: Gert Doering
Date:   Fri Jun 26 10:27:43 2020 +0200

     t_client.sh: correctly report all failed instances in summary

     Signed-off-by: Gert Doering <gert@greenie.muc.de>
     Acked-by: Antonio Quartulli <antonio@openvpn.net>
     Message-Id: <20200626082743.15397-1-gert@greenie.muc.de>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20130.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/tests/t_client.sh.in b/tests/t_client.sh.in
index 69866fb5..294546be 100755
--- a/tests/t_client.sh.in
+++ b/tests/t_client.sh.in
@@ -298,6 +298,7 @@  do
         echo -e "OK.\n"
     else
 	echo -e "FAIL: make sure that ping hosts are ONLY reachable via VPN, SKIP test $SUF".
+	SUMMARY_FAIL="$SUMMARY_FAIL $SUF"
 	exit_code=31
 	continue
     fi