Merge lp:~bladernr/checkbox/fix-wireless-suspend-tests into lp:checkbox

Proposed by Jeff Lane 
Status: Merged
Merged at revision: 1063
Proposed branch: lp:~bladernr/checkbox/fix-wireless-suspend-tests
Merge into: lp:checkbox
Diff against target: 37 lines (+5/-3)
2 files modified
debian/changelog (+3/-1)
jobs/suspend.txt.in (+2/-2)
To merge this branch: bzr merge lp:~bladernr/checkbox/fix-wireless-suspend-tests
Reviewer Review Type Date Requested Status
Daniel Manrique (community) Approve
Review via email: mp+76612@code.launchpad.net

Description of the change

instead of just pushing this to trunk, looking for a review to sanity check removing the bits from wireless_after_suspend that ALSO write to $CHECKBOX_DATA/iface.

That seemed unnecessary if the goal of the iface file is just to ensure we restart the interface that was in use when wireless_before_suspend ran.

I tested this on a thinkpad x201 and had no issues with it changed like this.

To post a comment you must log in.
Revision history for this message
Daniel Manrique (roadmr) wrote :

I tested the commands individually and they seem sane, I also agree that the *after* test doesn't need to write to the iface file, just restore as per what was written there.

And of course, clobbering the file with > instead of appending >> makes a lot of sense, though that would only surface if repeated runs are made without clearing the .checkbox directory; of course, that's an arcane trick so anything that spares end users from pain is welcome.

This looks good so I'll be merging it. Thanks!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'debian/changelog'
--- debian/changelog 2011-09-21 14:30:39 +0000
+++ debian/changelog 2011-09-22 16:35:48 +0000
@@ -19,8 +19,10 @@
19 [Jeff Lane]19 [Jeff Lane]
20 * Fix names of optical drive tests and remove a non-existing test from the20 * Fix names of optical drive tests and remove a non-existing test from the
21 whitelist (LP: #854808) 21 whitelist (LP: #854808)
22 * Fix wireless_*_suspend jobs so they recreate iface file instead of append
23 each time (LP: #855845)
2224
23 -- Daniel Manrique <daniel.manrique@canonical.com> Tue, 20 Sep 2011 15:59:48 -040025 -- Jeff Lane <jeff@ubuntu.com> Thu, 22 Sep 2011 12:24:03 -0400
2426
25checkbox (0.12.7) oneiric; urgency=low27checkbox (0.12.7) oneiric; urgency=low
2628
2729
=== modified file 'jobs/suspend.txt.in'
--- jobs/suspend.txt.in 2011-09-20 20:02:33 +0000
+++ jobs/suspend.txt.in 2011-09-22 16:35:48 +0000
@@ -30,7 +30,7 @@
30plugin: shell30plugin: shell
31name: suspend/wireless_before_suspend31name: suspend/wireless_before_suspend
32depends: networking/wireless_connection32depends: networking/wireless_connection
33command: nmcli -t -f DEVICES con status >> $CHECKBOX_DATA/iface && connect_wireless && internet_test --interface=`nmcli dev list | grep -B 1 wireless | grep GENERAL.DEVICE | awk '{print $2}'` && reconnect `cat $CHECKBOX_DATA/iface`33command: nmcli -t -f DEVICES con status > $CHECKBOX_DATA/iface && connect_wireless && internet_test --interface=`nmcli dev list | grep -B 1 wireless | grep GENERAL.DEVICE | awk '{print $2}'` && reconnect `cat $CHECKBOX_DATA/iface`
34_description:34_description:
35 This test disconnects all connections and then connects to the wireless35 This test disconnects all connections and then connects to the wireless
36 interface. It then checks the connection to confirm it's working as expected.36 interface. It then checks the connection to confirm it's working as expected.
@@ -111,7 +111,7 @@
111plugin: shell111plugin: shell
112name: suspend/wireless_after_suspend112name: suspend/wireless_after_suspend
113depends: suspend/suspend_advanced suspend/wireless_before_suspend113depends: suspend/suspend_advanced suspend/wireless_before_suspend
114command: nmcli -t -f DEVICES con status >> $CHECKBOX_DATA/iface && connect_wireless && internet_test --interface=`nmcli dev list | grep -B 1 wireless | grep GENERAL.DEVICE | awk '{print $2}'` && reconnect `cat $CHECKBOX_DATA/iface`114command: connect_wireless && internet_test --interface=`nmcli dev list | grep -B 1 wireless | grep GENERAL.DEVICE | awk '{print $2}'` && reconnect `cat $CHECKBOX_DATA/iface`
115_description:115_description:
116 This test checks that the wireless interface is working after suspending the system. It116 This test checks that the wireless interface is working after suspending the system. It
117 disconnects all interfaces and then connects to the wireless interface and checks that the117 disconnects all interfaces and then connects to the wireless interface and checks that the

Subscribers

People subscribed via source and target branches