Merge lp:~brendan-donegan/checkbox/bug944176_wireless_connection_failpass into lp:checkbox

[Brendan Donegan]

This merge updates the commands used by the wireless_connection tests to always remove the SSID file from the system-connections directory, but fail if internet_test fails and pass if it passes.

[Marc Tardif (cr3)]

The commands seem helluva complicated. How about this:

trap "rm -f /etc/NetworkManager/system-connections/$WPA_BG_SSID" EXIT
create_connection $WPA_BG_SSID --security=wpa --key=$WPA_BG_PSK
internet_test --interface=`nmcli dev status | awk '/802-11-wireless/ {print $1}'`

This will always remove the SSID file and exit with the status of the last command, ie internet_test. What do you think?

review: Needs Information

[Brendan Donegan (brendan-donegan)]

That seems to work as well. I guess much of the complication comes from having things on one line though - are we allowed to have commands on multiple lines in Checkbox?

[Brendan Donegan (brendan-donegan)]

Used the trap command. I kept everything on one line, just in case

review: Needs Resubmitting

[lp:~brendan-donegan/checkbox/bug944176_wireless_connection_failpass]

1292. By Brendan Donegan on 2012-03-02

Made the command simpler using trap according to cr3's feedback.

[Brendan Donegan (brendan-donegan)]

This wasn't working when I tested it, but I realise now that the EXIT signal will only be issued if this command is run inside a seperate shell session. If you run it from the same shell then no exit will ever be issued. So it will work fine in Checkbox, just not where it is copied and run on the same shell. I think a few of our job commands are like that.

[Marc Tardif (cr3)]

The last two diffs seem to remove the wrong file:

-command: create_connection $WPA_BG_SSID --security=wpa --key=$WPA_BG_PSK && ifconfig eth0 down; iperf -c $SERVER_IPERF -t 300 -i 30; ifconfig eth0 up; rm -f /etc/NetworkManager/system-connections/$WPA_BG_SSID
+command: trap "rm -f /etc/NetworkManager/system-connections/$OPEN_N_SSID; ifconfig eth0 up" EXIT; create_connection $WPA_BG_SSID --security=wpa --key=$WPA_BG_PSK; ifconfig eth0 down; iperf -c $SERVER_IPERF -t 300 -i 30

Before, $WPA_BG_SSID was being removed. After, $OPEN_N_SSID is now being removed. Is that intentional?

review: Needs Information

[Marc Tardif (cr3)]

To answer your previous question: yes, you are allowed to have commands on multiple lines in checkbox.

[Daniel Manrique (roadmr)]

I fixed the last two commands (were apparently cut-pasted and they indeed were deleting the wrong file). Thanks for catching that!

I'll merge this change now (with that minor fix).

review: Approve

=== modified file 'debian/changelog'
--- debian/changelog	2012-03-01 22:35:44 +0000
+++ debian/changelog	2012-03-02 08:21:18 +0000
@@ -7,6 +7,8 @@
     (LP: #942548)
   * Updated command fields in composite disk jobs to address the ! in 
     some disk paths (LP: #942769)
+  * Fixed command run by wireless_connection tests so that they fail if the
+    internet_test fails, but still clean up the connection file (LP: #944176)
 
   [Daniel Manrique]
   * Tweaks to internet_test: don't try to ping an IP that's unreachable from 

=== modified file 'jobs/wireless.txt.in'
--- jobs/wireless.txt.in	2012-02-01 16:28:33 +0000
+++ jobs/wireless.txt.in	2012-03-02 08:21:18 +0000
@@ -24,7 +24,7 @@
 requires: device.category == 'WIRELESS'
 user: root
 environ: WPA_BG_SSID WPA_BG_PSK
-command: create_connection $WPA_BG_SSID --security=wpa --key=$WPA_BG_PSK; internet_test --interface=`nmcli dev status | awk '/802-11-wireless/ {print $1}'`; rm -f /etc/NetworkManager/system-connections/$WPA_BG_SSID
+command: trap "rm -f /etc/NetworkManager/system-connections/$WPA_BG_SSID" EXIT; create_connection $WPA_BG_SSID --security=wpa --key=$WPA_BG_PSK; internet_test --interface=`nmcli dev status | awk '/802-11-wireless/ {print $1}'`
 _description:
  Tests that the systems wireless hardware can connect to a router using WPA
  security and the 802.11b/g protocols.
@@ -34,7 +34,7 @@
 requires: device.category == 'WIRELESS'
 user: root
 environ: OPEN_BG_SSID
-command: create_connection $OPEN_BG_SSID; internet_test --interface=`nmcli dev status | awk '/802-11-wireless/ {print $1}'`; rm -f /etc/NetworkManager/system-connections/$OPEN_BG_SSID
+command: trap "rm -f /etc/NetworkManager/system-connections/$OPEN_BG_SSID" EXIT; create_connection $OPEN_BG_SSID --security=wpa --key=$OPEN_BG_PSK; internet_test --interface=`nmcli dev status | awk '/802-11-wireless/ {print $1}'`
 _description:
  Tests that the systems wireless hardware can connect to a router using no
  security and the 802.11b/g protocols.
@@ -44,7 +44,7 @@
 requires: device.category == 'WIRELESS'
 user: root
 environ: WPA_N_SSID WPA_N_PSK
-command: create_connection $WPA_N_SSID --security=wpa --key=$WPA_N_PSK; internet_test --interface=`nmcli dev status | awk '/802-11-wireless/ {print $1}'`; rm -f /etc/NetworkManager/system-connections/$WPA_N_SSID
+command: trap "rm -f /etc/NetworkManager/system-connections/$WPA_N_SSID" EXIT; create_connection $WPA_N_SSID --security=wpa --key=$WPA_N_PSK; internet_test --interface=`nmcli dev status | awk '/802-11-wireless/ {print $1}'`
 _description:
  Tests that the systems wireless hardware can connect to a router using WPA
  security and the 802.11n protocol.
@@ -54,7 +54,7 @@
 requires: device.category == 'WIRELESS'
 user: root
 environ: OPEN_N_SSID
-command: create_connection $OPEN_N_SSID; internet_test --interface=`nmcli dev status | awk '/802-11-wireless/ {print $1}'`; rm -f /etc/NetworkManager/system-connections/$OPEN_N_SSID
+command: trap "rm -f /etc/NetworkManager/system-connections/$OPEN_N_SSID" EXIT; create_connection $OPEN_N_SSID --security=wpa --key=$OPEN_N_PSK; internet_test --interface=`nmcli dev status | awk '/802-11-wireless/ {print $1}'`
 _description:
  Tests that the systems wireless hardware can connect to a router using no
  security and the 802.11n protocol.
@@ -66,7 +66,7 @@
  device.category == 'WIRELESS'
 user: root
 environ: WPA_BG_SSID WPA_BG_PSK SERVER_IPERF
-command: create_connection $WPA_BG_SSID --security=wpa --key=$WPA_BG_PSK && ifconfig eth0 down; iperf -c $SERVER_IPERF -t 300 -i 30; ifconfig eth0 up; rm -f /etc/NetworkManager/system-connections/$WPA_BG_SSID
+command: trap "rm -f /etc/NetworkManager/system-connections/$OPEN_N_SSID; ifconfig eth0 up" EXIT; create_connection $WPA_BG_SSID --security=wpa --key=$WPA_BG_PSK; ifconfig eth0 down; iperf -c $SERVER_IPERF -t 300 -i 30
 _description:
  Tests the performance of a systems wireless connection through the iperf tool.
 
@@ -77,6 +77,6 @@
  device.category == 'WIRELESS'
 user: root
 environ: WPA_BG_SSID WPA_BG_PSK SERVER_IPERF
-command: create_connection $WPA_BG_SSID --security=wpa --key=$WPA_BG_PSK && ifconfig eth0 down; iperf -c $SERVER_IPERF -t 300 -i 30 -u -b 100m -p 5050; ifconfig eth0 up; rm -f /etc/NetworkManager/system-connections/$WPA_BG_SSID
+command: trap "rm -f /etc/NetworkManager/system-connections/$OPEN_N_SSID; ifconfig eth0 up" EXIT; create_connection $WPA_BG_SSID --security=wpa --key=$WPA_BG_PSK && ifconfig eth0 down; iperf -c $SERVER_IPERF -t 300 -i 30 -u -b 100m -p 5050
 _description:
  Tests the performance of a systems wireless connection through the iperf tool, using UDP packets.