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

[Marc Tardif]

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.

[Brendan Donegan (brendan-donegan)]

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

[Brendan Donegan (brendan-donegan)]

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]

1142. By Marc Tardif on 2011-12-07

Renamed sru tests and improved description.

[Marc Tardif (cr3)]

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]

1143. By Marc Tardif on 2011-12-07

Updated whitelist for sru tests.

[Marc Tardif (cr3)]

Forgot to update the whitelist, good catch brd!

review: Needs Resubmitting

[Daniel Manrique (roadmr)]

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?

[Marc Tardif (cr3)]

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

[Brendan Donegan (brendan-donegan)]

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]

1144. By Marc Tardif on 2011-12-10

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

[Marc Tardif (cr3)]

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

[Brendan Donegan (brendan-donegan)]

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

[Marc Tardif (cr3)]

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

[Brendan Donegan (brendan-donegan)]

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

review: Approve

=== modified file '.bzrignore'
--- .bzrignore	2011-06-29 13:34:45 +0000
+++ .bzrignore	2011-12-10 14:32:35 +0000
@@ -4,6 +4,7 @@
 debian/checkbox
 debian/checkbox-cli
 debian/checkbox-gtk
+debian/checkbox-sru
 debian/checkbox-urwid
 debian/*-cli.postrm
 debian/*.debhelper
@@ -16,6 +17,7 @@
 debian/stamp-patched
 debian/*.substvars
 debian/tmp
+debian/*-sru.postrm
 debian/*-urwid.postrm
 gtk/checkbox-gtk.glade.bak
 gtk/checkbox-gtk.gladep
 
=== modified file 'bin/checkbox-cli'
--- bin/checkbox-cli	2011-06-29 09:46:26 +0000
+++ bin/checkbox-cli	2011-12-10 14:32:35 +0000
@@ -3,7 +3,7 @@
 export XDG_CACHE_HOME=${XDG_CACHE_HOME:-$HOME/.cache}
 export CHECKBOX_DATA=${CHECKBOX_DATA:-.}
 export CHECKBOX_SHARE=${CHECKBOX_SHARE:-.}
-export CHECKBOX_OPTIONS=${CHECKBOX_OPTIONS:-}
+export CHECKBOX_OPTIONS=${CHECKBOX_OPTIONS:---whitelist-file=$CHECKBOX_SHARE/data/whitelists/default.whitelist}
 export PYTHONPATH=$PYTHONPATH:$CHECKBOX_SHARE
 
 if [ $CHECKBOX_DATA != '.' ]
 
=== modified file 'bin/checkbox-gtk'
--- bin/checkbox-gtk	2011-06-29 09:46:26 +0000
+++ bin/checkbox-gtk	2011-12-10 14:32:35 +0000
@@ -3,7 +3,7 @@
 export XDG_CACHE_HOME=${XDG_CACHE_HOME:-$HOME/.cache}
 export CHECKBOX_DATA=${CHECKBOX_DATA:-.}
 export CHECKBOX_SHARE=${CHECKBOX_SHARE:-.}
-export CHECKBOX_OPTIONS=${CHECKBOX_OPTIONS:-}
+export CHECKBOX_OPTIONS=${CHECKBOX_OPTIONS:---whitelist-file=$CHECKBOX_SHARE/data/whitelists/default.whitelist}
 export PYTHONPATH=$PYTHONPATH:$CHECKBOX_SHARE
 
 if [ $CHECKBOX_DATA != '.' ]
 
=== added file 'bin/checkbox-sru'
--- bin/checkbox-sru	1970-01-01 00:00:00 +0000
+++ bin/checkbox-sru	2011-12-10 14:32:35 +0000
@@ -0,0 +1,18 @@
+#!/bin/bash
+
+export XDG_CACHE_HOME=${XDG_CACHE_HOME:-$HOME/.cache}
+export CHECKBOX_DATA=${CHECKBOX_DATA:-.}
+export CHECKBOX_SHARE=${CHECKBOX_SHARE:-.}
+export CHECKBOX_OPTIONS=${CHECKBOX_OPTIONS:---whitelist-file=$CHECKBOX_SHARE/data/whitelists/sru.whitelist}
+export PYTHONPATH=$PYTHONPATH:$CHECKBOX_SHARE
+
+if [ $CHECKBOX_DATA != '.' ]
+then
+    old_data=$HOME/.checkbox
+    if [ -d $old_data ] && [ ! -d $CHECKBOX_DATA ]
+    then
+        mv -f $old_data $CHECKBOX_DATA
+    fi
+fi
+
+python $CHECKBOX_SHARE/run "$@" $CHECKBOX_SHARE/configs/$(basename $0).ini
 
=== modified file 'bin/checkbox-urwid'
--- bin/checkbox-urwid	2011-06-29 09:46:26 +0000
+++ bin/checkbox-urwid	2011-12-10 14:32:35 +0000
@@ -3,7 +3,7 @@
 export XDG_CACHE_HOME=${XDG_CACHE_HOME:-$HOME/.cache}
 export CHECKBOX_DATA=${CHECKBOX_DATA:-.}
 export CHECKBOX_SHARE=${CHECKBOX_SHARE:-.}
-export CHECKBOX_OPTIONS=${CHECKBOX_OPTIONS:-}
+export CHECKBOX_OPTIONS=${CHECKBOX_OPTIONS:---whitelist-file=$CHECKBOX_SHARE/data/whitelists/default.whitelist}
 export PYTHONPATH=$PYTHONPATH:$CHECKBOX_SHARE
 
 if [ $CHECKBOX_DATA != '.' ]
 
=== added file 'data/whitelists/sru.whitelist'
--- data/whitelists/sru.whitelist	1970-01-01 00:00:00 +0000
+++ data/whitelists/sru.whitelist	2011-12-10 14:32:35 +0000
@@ -0,0 +1,28 @@
+cdimage
+cpuinfo
+device
+dmi
+dpkg
+gconf
+lsb
+meminfo
+module
+package
+uname
+__info__
+codecs_attachment
+cpuinfo_attachment
+dmesg_attachment
+dmi_attachment
+dmidecode_attachment
+lsmod_attachment
+lspci_attachment
+gcov_attachment
+modprobe_attachment
+modules_attachment
+sysfs_attachment
+sysctl_attachment
+udev_attachment
+__sru__
+sru/suspend_success
+sru/hibernate_success
 
=== added file 'debian/checkbox-sru.install'
--- debian/checkbox-sru.install	1970-01-01 00:00:00 +0000
+++ debian/checkbox-sru.install	2011-12-10 14:32:35 +0000
@@ -0,0 +1,2 @@
+usr/bin/checkbox-sru
+usr/share/checkbox/examples/checkbox-sru.ini
 
=== added file 'debian/checkbox-sru.links'
--- debian/checkbox-sru.links	1970-01-01 00:00:00 +0000
+++ debian/checkbox-sru.links	2011-12-10 14:32:35 +0000
@@ -0,0 +1,1 @@
+usr/share/man/man1/checkbox.1.gz usr/share/man/man1/checkbox-sru.1.gz
 
=== added file 'debian/checkbox-sru.postinst'
--- debian/checkbox-sru.postinst	1970-01-01 00:00:00 +0000
+++ debian/checkbox-sru.postinst	2011-12-10 14:32:35 +0000
@@ -0,0 +1,7 @@
+#! /bin/sh -e
+
+base_package="checkbox"
+. /usr/share/debconf/confmodule
+. /usr/share/checkbox/install/postinst
+
+#DEBHELPER#
 
=== modified file 'debian/control'
--- debian/control	2011-12-06 21:32:26 +0000
+++ debian/control	2011-12-10 14:32:35 +0000
@@ -56,6 +56,15 @@
  .
  This package provides a GTK interface for answering tests.
 
+Package: checkbox-sru
+Architecture: all
+Depends: checkbox (>= ${source:Version}), ${misc:Depends}
+Description: SRU interface for checkbox 
+ This project provides an extensible interface for system testing. The
+ results can then be sent to Launchpad.
+ .
+ This package provides a non-interactive interface for SRU testing.
+
 Package: hwtest
 Section: python
 Architecture: all
 
=== modified file 'debian/rules'
--- debian/rules	2011-06-23 21:45:46 +0000
+++ debian/rules	2011-12-10 14:32:35 +0000
@@ -7,6 +7,7 @@
 	cp debian/checkbox.postrm debian/checkbox-cli.postrm
 	cp debian/checkbox.postrm debian/checkbox-urwid.postrm
 	cp debian/checkbox.postrm debian/checkbox-gtk.postrm
+	cp debian/checkbox.postrm debian/checkbox-sru.postrm
 	cp debian/hwtest.postrm debian/hwtest-cli.postrm
 	cp debian/hwtest.postrm debian/hwtest-gtk.postrm
 	dh_installdeb
@@ -16,6 +17,7 @@
 	dh_installdocs -pcheckbox-cli ./README 
 	dh_installdocs -pcheckbox-urwid ./README 
 	dh_installdocs -pcheckbox-gtk ./README 
+	dh_installdocs -pcheckbox-sru ./README 
 	dh_installdocs -phwtest ./README 
 	dh_installdocs -phwtest-cli ./README 
 	dh_installdocs -phwtest-gtk ./README 
@@ -23,7 +25,7 @@
 
 override_dh_clean:
 	-find . -name \*.mo -exec rm {} \;
-	-rm -f debian/checkbox-cli.postrm debian/checkbox-urwid.postrm debian/checkbox-gtk.postrm
+	-rm -f debian/checkbox-cli.postrm debian/checkbox-urwid.postrm debian/checkbox-gtk.postrm debian/checkbox-sru.postrm
 	-rm -f debian/hwtest-cli.postrm debian/hwtest-gtk.postrm
 	debconf-updatepo
 	dh_clean
 
=== added file 'examples/checkbox-sru.ini'
--- examples/checkbox-sru.ini	1970-01-01 00:00:00 +0000
+++ examples/checkbox-sru.ini	2011-12-10 14:32:35 +0000
@@ -0,0 +1,5 @@
+[DEFAULT]
+
+# Space separated list of files to include as a dependency for the
+# SRU interface.
+includes = %(checkbox_share)s/configs/checkbox.ini
 
=== added file 'jobs/sru.txt.in'
--- jobs/sru.txt.in	1970-01-01 00:00:00 +0000
+++ jobs/sru.txt.in	2011-12-10 14:32:35 +0000
@@ -0,0 +1,4 @@
+name: __sru__
+plugin: remote
+_description: SRU tests.
+command: sru_suspend
 
=== added file 'scripts/sru_suspend'
--- scripts/sru_suspend	1970-01-01 00:00:00 +0000
+++ scripts/sru_suspend	2011-12-10 14:32:35 +0000
@@ -0,0 +1,33 @@
+#!/bin/bash
+#
+# Simplistic test to determine whether the system successfully resumed
+# from suspend and thawed from hibernate.
+
+if zgrep -q '/usr/lib/pm-utils/sleep.d/000kernel-change resume suspend: success.' /var/log/pm-suspend.log*; then
+    suspend_status="pass"
+else
+    suspend_status="untested"
+fi
+
+if zgrep -q '/usr/lib/pm-utils/sleep.d/000kernel-change thaw hibernate: success.' /var/log/pm-suspend.log*; then
+    hibernate_status="pass"
+else
+    hibernate_status="untested"
+fi
+
+cat <<EOF
+plugin: shell
+name: sru/suspend_success
+status: $suspend_status
+description:
+ Check that /var/log/pm-suspend.log contains the string:
+ resume suspend: success.
+
+plugin: shell
+name: sru/hibernate_success
+status: $hibernate_status
+description:
+ Check that /var/log/pm-suspend.log contains the string:
+ thaw hibernate: success.
+
+EOF
 
=== modified file 'setup.py'
--- setup.py	2011-10-11 20:15:01 +0000
+++ setup.py	2011-12-10 14:32:35 +0000
@@ -188,7 +188,6 @@
         for outfile in self.outfiles:
             infile = posixpath.join("bin", posixpath.basename(outfile))
             substitute_variables(infile, outfile, {
-                "CHECKBOX_OPTIONS:-": "CHECKBOX_OPTIONS:---whitelist-file=$CHECKBOX_SHARE/data/whitelists/default.whitelist",
                 "CHECKBOX_SHARE:-.": "CHECKBOX_SHARE:-/usr/share/checkbox",
                 "CHECKBOX_DATA:-.": "CHECKBOX_DATA:-$XDG_CACHE_HOME/checkbox"})
 
@@ -230,7 +229,8 @@
         ("share/checkbox/gtk/", ["gtk/checkbox-gtk.ui", "gtk/*.png"]),
         ("share/apport/package-hooks/", ["apport/source_checkbox.py"]),
         ("share/apport/general-hooks/", ["apport/checkbox.py"])],
-    scripts = ["bin/checkbox-cli", "bin/checkbox-gtk", "bin/checkbox-urwid"],
+    scripts = ["bin/checkbox-cli", "bin/checkbox-gtk", "bin/checkbox-sru",
+        "bin/checkbox-urwid"],
     packages = ["checkbox", "checkbox.contrib", "checkbox.lib", "checkbox.parsers",
         "checkbox.reports", "checkbox_cli", "checkbox_gtk", "checkbox_urwid"],
     package_data = {