Merge lp:~cr3/checkbox/sru into lp:checkbox

Proposed by Marc Tardif
Status: Merged
Merged at revision: 1148
Proposed branch: lp:~cr3/checkbox/sru
Merge into: lp:checkbox
Diff against target: 267 lines (+117/-6)
15 files modified
.bzrignore (+2/-0)
bin/checkbox-cli (+1/-1)
bin/checkbox-gtk (+1/-1)
bin/checkbox-sru (+18/-0)
bin/checkbox-urwid (+1/-1)
data/whitelists/sru.whitelist (+28/-0)
debian/checkbox-sru.install (+2/-0)
debian/checkbox-sru.links (+1/-0)
debian/checkbox-sru.postinst (+7/-0)
debian/control (+9/-0)
debian/rules (+3/-1)
examples/checkbox-sru.ini (+5/-0)
jobs/sru.txt.in (+4/-0)
scripts/sru_suspend (+33/-0)
setup.py (+2/-2)
To merge this branch: bzr merge lp:~cr3/checkbox/sru
Reviewer Review Type Date Requested Status
Brendan Donegan (community) Approve
Marc Tardif (community) Needs Resubmitting
Review via email: mp+84771@code.launchpad.net

Description of the change

This is an initial proposal for introducing SRU testing by the community. Until we clearly define how checkbox will be called, the current implementation is non-interactive and assumes being called like this:

  checkbox-sru --<email address hidden>

Also note that the sru job is defined as remote because the current assumption is that there would be one script that would result in many test results. For example, the sru_suspend test results in both suspend and hibernate tests. This needs to be remote instead of local so that the test is performed during the run phase rather than the gathering phase. This might change in the future but that seems to make the most sense for now.

To post a comment you must log in.
Revision history for this message
Brendan Donegan (brendan-donegan) wrote :

I have a comment about the naming. It seems like it might be wise to generalise the names of the test to describe what they do, rather than under what circumstances they're run. Also, you're clobbering my whitelist name! Thankfully it's in checkbox-certification at the moment but we did plan to move the whitelists out of checkbox-certification and into base checkbox.

review: Needs Fixing
Revision history for this message
Brendan Donegan (brendan-donegan) wrote :

To try and be constructive, rather than just poo-pooing your names, how about, e.g:

sru_suspend -> suspend/suspend_succeeded

and put it in the suspend job file rather than a separate SRU one?

lp:~cr3/checkbox/sru updated
1142. By Marc Tardif

Renamed sru tests and improved description.

Revision history for this message
Marc Tardif (cr3) wrote :

Thanks for the suggestion, I agree that the naming could be better. I have chosen:

  sru/suspend_success
  sru/hibernate_success

I have also taken the opportunity of improving the descriptions to be more useful. I have specifically chosen the sru/ namespace because these tests might eventually validate that you're effectively running a proposed kernel. I would like this side effect to be clearly delineated from the other tests.

As for the conflict with the checkbox-certification sru whitelist, I think it would make sense to clearly identify it as such: sru-certification.whitelist. Ideally, I would like both whitelists to eventually become one and the same, but we'll burn that bridge when we're on it...

review: Needs Resubmitting
lp:~cr3/checkbox/sru updated
1143. By Marc Tardif

Updated whitelist for sru tests.

Revision history for this message
Marc Tardif (cr3) wrote :

Forgot to update the whitelist, good catch brd!

review: Needs Resubmitting
Revision history for this message
Daniel Manrique (roadmr) wrote :

Looks fine to me, but what about the sru.whitelist? does it clobber something in checkbox-cert that we need to worry about? do we need to rename something?

Revision history for this message
Marc Tardif (cr3) wrote :

The same sru.whitelist file name can be used in both checkbox and checkbox-certification where they will be referenced independently. For example, this is what we use in the checkbox-satellite preseeds which clearly reference a path specific to checkbox-certification:

  --whitelist-file=/usr/share/checkbox-certification/data/sru.whitelist

Revision history for this message
Brendan Donegan (brendan-donegan) wrote :

Sorry I didn't pick these things up yesterday, but I just got the chance to actually run the thing today and I first noticed that checkbox-sru had not been given permissions. Secondly I noticed that the checkbox-sru script doesn't define a whitelist to CHECKBOX_OPTIONS, meaning it would need to be defined on the command line. I personally think it makes more sense to encode the whitelist in the script itself, just like we do with checkbox-certification-server/client

review: Needs Fixing
lp:~cr3/checkbox/sru updated
1144. By Marc Tardif

Moved whitelist-file option from setup.py directly in scripts.

Revision history for this message
Marc Tardif (cr3) wrote :

Brendan, good catch! I added the execute bit to checkbox-sru. As for the whitelist, I decided to specify the whitelist in each script instead of having setup.py do it. So, when running checkbox-* from the source tree, it will behave pretty much the same way as when running them from an installed package.

review: Needs Resubmitting
Revision history for this message
Brendan Donegan (brendan-donegan) wrote :

I was about to merge this, as I got it to work after setting the execute bit on scripts/sru_suspend, but then I noticed that in the sru_suspend script you seem to have written a test that can't fail. Is it the case that no evidence is left behind that a system failed to suspend? If so then what value is this? At the moment not having tested suspend and the suspend actually failing give equivalent results, which is just wrong.

Some chatting with cking shows that while there is not a 100% reliable method there are ways to detect that suspend/hibernate failed.

review: Needs Fixing
Revision history for this message
Marc Tardif (cr3) wrote :

There are indeed ways to detect whether suspend/hibernate failed. If you look at the source of fwts, this is done by attempting to match one of 26 regular expressions over strings output by pm-utils in /var/log/pm-*.log. If these strings change, even by one character, then the attempt to detect a failure misses which is where reliability goes out the window. Instead, the current tests only checks for one success string which increases reliability by 26 times and also happens to reduce maintenance by as many times in the event pm-utils changes a string.

If you are concerned about detecting failure, I would propose that we check for two strings: one when suspend/hibernate started and another for success. If none is detected: untested. If one is detected: fail. If two are detected: pass. However, I think improvements to tests should be discussed in a bug rather than in this merge request.

Please note that the intent of this merge request is simply to put in place the necessary infrastructure for SRU testing, hence the name of this branch. The suspend/hibernate test was mostly intended to demonstrate that the infrastructure works.

review: Needs Resubmitting
Revision history for this message
Brendan Donegan (brendan-donegan) wrote :

Ok. I am going to raise a bug for the test and merge this now.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file '.bzrignore'
2--- .bzrignore 2011-06-29 13:34:45 +0000
3+++ .bzrignore 2011-12-10 14:32:35 +0000
4@@ -4,6 +4,7 @@
5 debian/checkbox
6 debian/checkbox-cli
7 debian/checkbox-gtk
8+debian/checkbox-sru
9 debian/checkbox-urwid
10 debian/*-cli.postrm
11 debian/*.debhelper
12@@ -16,6 +17,7 @@
13 debian/stamp-patched
14 debian/*.substvars
15 debian/tmp
16+debian/*-sru.postrm
17 debian/*-urwid.postrm
18 gtk/checkbox-gtk.glade.bak
19 gtk/checkbox-gtk.gladep
20
21=== modified file 'bin/checkbox-cli'
22--- bin/checkbox-cli 2011-06-29 09:46:26 +0000
23+++ bin/checkbox-cli 2011-12-10 14:32:35 +0000
24@@ -3,7 +3,7 @@
25 export XDG_CACHE_HOME=${XDG_CACHE_HOME:-$HOME/.cache}
26 export CHECKBOX_DATA=${CHECKBOX_DATA:-.}
27 export CHECKBOX_SHARE=${CHECKBOX_SHARE:-.}
28-export CHECKBOX_OPTIONS=${CHECKBOX_OPTIONS:-}
29+export CHECKBOX_OPTIONS=${CHECKBOX_OPTIONS:---whitelist-file=$CHECKBOX_SHARE/data/whitelists/default.whitelist}
30 export PYTHONPATH=$PYTHONPATH:$CHECKBOX_SHARE
31
32 if [ $CHECKBOX_DATA != '.' ]
33
34=== modified file 'bin/checkbox-gtk'
35--- bin/checkbox-gtk 2011-06-29 09:46:26 +0000
36+++ bin/checkbox-gtk 2011-12-10 14:32:35 +0000
37@@ -3,7 +3,7 @@
38 export XDG_CACHE_HOME=${XDG_CACHE_HOME:-$HOME/.cache}
39 export CHECKBOX_DATA=${CHECKBOX_DATA:-.}
40 export CHECKBOX_SHARE=${CHECKBOX_SHARE:-.}
41-export CHECKBOX_OPTIONS=${CHECKBOX_OPTIONS:-}
42+export CHECKBOX_OPTIONS=${CHECKBOX_OPTIONS:---whitelist-file=$CHECKBOX_SHARE/data/whitelists/default.whitelist}
43 export PYTHONPATH=$PYTHONPATH:$CHECKBOX_SHARE
44
45 if [ $CHECKBOX_DATA != '.' ]
46
47=== added file 'bin/checkbox-sru'
48--- bin/checkbox-sru 1970-01-01 00:00:00 +0000
49+++ bin/checkbox-sru 2011-12-10 14:32:35 +0000
50@@ -0,0 +1,18 @@
51+#!/bin/bash
52+
53+export XDG_CACHE_HOME=${XDG_CACHE_HOME:-$HOME/.cache}
54+export CHECKBOX_DATA=${CHECKBOX_DATA:-.}
55+export CHECKBOX_SHARE=${CHECKBOX_SHARE:-.}
56+export CHECKBOX_OPTIONS=${CHECKBOX_OPTIONS:---whitelist-file=$CHECKBOX_SHARE/data/whitelists/sru.whitelist}
57+export PYTHONPATH=$PYTHONPATH:$CHECKBOX_SHARE
58+
59+if [ $CHECKBOX_DATA != '.' ]
60+then
61+ old_data=$HOME/.checkbox
62+ if [ -d $old_data ] && [ ! -d $CHECKBOX_DATA ]
63+ then
64+ mv -f $old_data $CHECKBOX_DATA
65+ fi
66+fi
67+
68+python $CHECKBOX_SHARE/run "$@" $CHECKBOX_SHARE/configs/$(basename $0).ini
69
70=== modified file 'bin/checkbox-urwid'
71--- bin/checkbox-urwid 2011-06-29 09:46:26 +0000
72+++ bin/checkbox-urwid 2011-12-10 14:32:35 +0000
73@@ -3,7 +3,7 @@
74 export XDG_CACHE_HOME=${XDG_CACHE_HOME:-$HOME/.cache}
75 export CHECKBOX_DATA=${CHECKBOX_DATA:-.}
76 export CHECKBOX_SHARE=${CHECKBOX_SHARE:-.}
77-export CHECKBOX_OPTIONS=${CHECKBOX_OPTIONS:-}
78+export CHECKBOX_OPTIONS=${CHECKBOX_OPTIONS:---whitelist-file=$CHECKBOX_SHARE/data/whitelists/default.whitelist}
79 export PYTHONPATH=$PYTHONPATH:$CHECKBOX_SHARE
80
81 if [ $CHECKBOX_DATA != '.' ]
82
83=== added file 'data/whitelists/sru.whitelist'
84--- data/whitelists/sru.whitelist 1970-01-01 00:00:00 +0000
85+++ data/whitelists/sru.whitelist 2011-12-10 14:32:35 +0000
86@@ -0,0 +1,28 @@
87+cdimage
88+cpuinfo
89+device
90+dmi
91+dpkg
92+gconf
93+lsb
94+meminfo
95+module
96+package
97+uname
98+__info__
99+codecs_attachment
100+cpuinfo_attachment
101+dmesg_attachment
102+dmi_attachment
103+dmidecode_attachment
104+lsmod_attachment
105+lspci_attachment
106+gcov_attachment
107+modprobe_attachment
108+modules_attachment
109+sysfs_attachment
110+sysctl_attachment
111+udev_attachment
112+__sru__
113+sru/suspend_success
114+sru/hibernate_success
115
116=== added file 'debian/checkbox-sru.install'
117--- debian/checkbox-sru.install 1970-01-01 00:00:00 +0000
118+++ debian/checkbox-sru.install 2011-12-10 14:32:35 +0000
119@@ -0,0 +1,2 @@
120+usr/bin/checkbox-sru
121+usr/share/checkbox/examples/checkbox-sru.ini
122
123=== added file 'debian/checkbox-sru.links'
124--- debian/checkbox-sru.links 1970-01-01 00:00:00 +0000
125+++ debian/checkbox-sru.links 2011-12-10 14:32:35 +0000
126@@ -0,0 +1,1 @@
127+usr/share/man/man1/checkbox.1.gz usr/share/man/man1/checkbox-sru.1.gz
128
129=== added file 'debian/checkbox-sru.postinst'
130--- debian/checkbox-sru.postinst 1970-01-01 00:00:00 +0000
131+++ debian/checkbox-sru.postinst 2011-12-10 14:32:35 +0000
132@@ -0,0 +1,7 @@
133+#! /bin/sh -e
134+
135+base_package="checkbox"
136+. /usr/share/debconf/confmodule
137+. /usr/share/checkbox/install/postinst
138+
139+#DEBHELPER#
140
141=== modified file 'debian/control'
142--- debian/control 2011-12-06 21:32:26 +0000
143+++ debian/control 2011-12-10 14:32:35 +0000
144@@ -56,6 +56,15 @@
145 .
146 This package provides a GTK interface for answering tests.
147
148+Package: checkbox-sru
149+Architecture: all
150+Depends: checkbox (>= ${source:Version}), ${misc:Depends}
151+Description: SRU interface for checkbox
152+ This project provides an extensible interface for system testing. The
153+ results can then be sent to Launchpad.
154+ .
155+ This package provides a non-interactive interface for SRU testing.
156+
157 Package: hwtest
158 Section: python
159 Architecture: all
160
161=== modified file 'debian/rules'
162--- debian/rules 2011-06-23 21:45:46 +0000
163+++ debian/rules 2011-12-10 14:32:35 +0000
164@@ -7,6 +7,7 @@
165 cp debian/checkbox.postrm debian/checkbox-cli.postrm
166 cp debian/checkbox.postrm debian/checkbox-urwid.postrm
167 cp debian/checkbox.postrm debian/checkbox-gtk.postrm
168+ cp debian/checkbox.postrm debian/checkbox-sru.postrm
169 cp debian/hwtest.postrm debian/hwtest-cli.postrm
170 cp debian/hwtest.postrm debian/hwtest-gtk.postrm
171 dh_installdeb
172@@ -16,6 +17,7 @@
173 dh_installdocs -pcheckbox-cli ./README
174 dh_installdocs -pcheckbox-urwid ./README
175 dh_installdocs -pcheckbox-gtk ./README
176+ dh_installdocs -pcheckbox-sru ./README
177 dh_installdocs -phwtest ./README
178 dh_installdocs -phwtest-cli ./README
179 dh_installdocs -phwtest-gtk ./README
180@@ -23,7 +25,7 @@
181
182 override_dh_clean:
183 -find . -name \*.mo -exec rm {} \;
184- -rm -f debian/checkbox-cli.postrm debian/checkbox-urwid.postrm debian/checkbox-gtk.postrm
185+ -rm -f debian/checkbox-cli.postrm debian/checkbox-urwid.postrm debian/checkbox-gtk.postrm debian/checkbox-sru.postrm
186 -rm -f debian/hwtest-cli.postrm debian/hwtest-gtk.postrm
187 debconf-updatepo
188 dh_clean
189
190=== added file 'examples/checkbox-sru.ini'
191--- examples/checkbox-sru.ini 1970-01-01 00:00:00 +0000
192+++ examples/checkbox-sru.ini 2011-12-10 14:32:35 +0000
193@@ -0,0 +1,5 @@
194+[DEFAULT]
195+
196+# Space separated list of files to include as a dependency for the
197+# SRU interface.
198+includes = %(checkbox_share)s/configs/checkbox.ini
199
200=== added file 'jobs/sru.txt.in'
201--- jobs/sru.txt.in 1970-01-01 00:00:00 +0000
202+++ jobs/sru.txt.in 2011-12-10 14:32:35 +0000
203@@ -0,0 +1,4 @@
204+name: __sru__
205+plugin: remote
206+_description: SRU tests.
207+command: sru_suspend
208
209=== added file 'scripts/sru_suspend'
210--- scripts/sru_suspend 1970-01-01 00:00:00 +0000
211+++ scripts/sru_suspend 2011-12-10 14:32:35 +0000
212@@ -0,0 +1,33 @@
213+#!/bin/bash
214+#
215+# Simplistic test to determine whether the system successfully resumed
216+# from suspend and thawed from hibernate.
217+
218+if zgrep -q '/usr/lib/pm-utils/sleep.d/000kernel-change resume suspend: success.' /var/log/pm-suspend.log*; then
219+ suspend_status="pass"
220+else
221+ suspend_status="untested"
222+fi
223+
224+if zgrep -q '/usr/lib/pm-utils/sleep.d/000kernel-change thaw hibernate: success.' /var/log/pm-suspend.log*; then
225+ hibernate_status="pass"
226+else
227+ hibernate_status="untested"
228+fi
229+
230+cat <<EOF
231+plugin: shell
232+name: sru/suspend_success
233+status: $suspend_status
234+description:
235+ Check that /var/log/pm-suspend.log contains the string:
236+ resume suspend: success.
237+
238+plugin: shell
239+name: sru/hibernate_success
240+status: $hibernate_status
241+description:
242+ Check that /var/log/pm-suspend.log contains the string:
243+ thaw hibernate: success.
244+
245+EOF
246
247=== modified file 'setup.py'
248--- setup.py 2011-10-11 20:15:01 +0000
249+++ setup.py 2011-12-10 14:32:35 +0000
250@@ -188,7 +188,6 @@
251 for outfile in self.outfiles:
252 infile = posixpath.join("bin", posixpath.basename(outfile))
253 substitute_variables(infile, outfile, {
254- "CHECKBOX_OPTIONS:-": "CHECKBOX_OPTIONS:---whitelist-file=$CHECKBOX_SHARE/data/whitelists/default.whitelist",
255 "CHECKBOX_SHARE:-.": "CHECKBOX_SHARE:-/usr/share/checkbox",
256 "CHECKBOX_DATA:-.": "CHECKBOX_DATA:-$XDG_CACHE_HOME/checkbox"})
257
258@@ -230,7 +229,8 @@
259 ("share/checkbox/gtk/", ["gtk/checkbox-gtk.ui", "gtk/*.png"]),
260 ("share/apport/package-hooks/", ["apport/source_checkbox.py"]),
261 ("share/apport/general-hooks/", ["apport/checkbox.py"])],
262- scripts = ["bin/checkbox-cli", "bin/checkbox-gtk", "bin/checkbox-urwid"],
263+ scripts = ["bin/checkbox-cli", "bin/checkbox-gtk", "bin/checkbox-sru",
264+ "bin/checkbox-urwid"],
265 packages = ["checkbox", "checkbox.contrib", "checkbox.lib", "checkbox.parsers",
266 "checkbox.reports", "checkbox_cli", "checkbox_gtk", "checkbox_urwid"],
267 package_data = {

Subscribers

People subscribed via source and target branches