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

[Marc Tardif]

Merged fix to bug 1057762 by cr3.

This merge request fixes this error:

2012-09-27 16:22:15,313 ERROR Error running event handler <string> report_message({'usage': 'filter_templates [OPTIONS] [FILE...]', 'suite': 'optical/cdrom-write-automated'}) for event type 'report-message'
Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/checkbox/reactor.py", line 74, in fire
    results.append(handler(*args, **kwargs))
  File "<string>", line 150, in report_message
KeyError: 'name'

[Zygmunt Krynicki (zyga)]

While this looks all fine and I would gladly accept it I came to realize I don't really know how to review merge requests like that. I'm not familiar with most scripts or specifics of whitelists.

Could you share a piece of advice, how to reliably review such merge requests?

[Marc Tardif (cr3)]

You could either run checkbox from the root of the branch and look for the error pasted above in the checkbox.log file. Or, you could more precisely try running the commands that failed. For example, this is how you could run the command for the optical/dvd-write test:

$ export PATH=`pwd`/scripts:$PATH
$ cat <<'EOF' | run_templates -t -s 'udev_resource | filter_templates -w "category=CDROM"'
plugin: manual
name: optical/dvd-write_`ls /sys$path/block`
requires: device.path == "$path"
user: root
command: optical_write_test /dev/`ls /sys$path/block` | ansi_parser
description:
 PURPOSE:
     This test will check your system's $product writing capabilities. This test requires a blank DVD-R or DVD+R. If you do not have a blank DVD disk, skip this test.
 STEPS:
     1. Enter a blank DVD-R or DVD+R into your drive
     2. Click "Test" to begin.
     3. When the CD tray ejects the media after burning, close it (DO NOT remove the disk, it is needed for the second portion of the test). Note, you must close the drive within 5 minutes or the test will time out.
 VERIFICATION:
     This test should automatically select "Yes" if it passes, "No" if it fails.
EOF

Before the merge request, you'll get an error. After, you won't get an error anymore.

[Marc Tardif (cr3)]

Note that this merge request fixes a critical bug that is affecting other tests. Please make sure to test and merge this code as quickly as possible, otherwise I suspect someone else will spend time trying to fix the same problem. Thanks!

[Brendan Donegan (brendan-donegan)]

Looks good, +1

review: Approve

=== modified file 'debian/changelog'
--- debian/changelog	2012-09-27 18:10:45 +0000
+++ debian/changelog	2012-09-27 20:53:35 +0000
@@ -90,6 +90,8 @@
   * [FEATURE] tools/lint: Added script to consistently check syntax.
   * plugins/apport_prompt.py: Removed apport plugin that caused crashes
     when trying to send bug report (LP #1047857)
+  * jobs/optical.txt.in: Fixed missing category assignment in optical
+    dvd write tests (LP: #1057762)
 
   [Sean Feole]
   * [FEATURE] scripts/battery_test: measures battery capacity before and after

=== modified file 'jobs/optical.txt.in'
--- jobs/optical.txt.in	2012-09-26 16:43:32 +0000
+++ jobs/optical.txt.in	2012-09-27 20:53:35 +0000
@@ -74,7 +74,7 @@
  device.category == 'CDROM'
  optical_drive.cd == 'writable'
 command:
- cat <<'EOF' | run_templates -t -s 'udev_resource | filter_templates -w "category"'
+ cat <<'EOF' | run_templates -t -s 'udev_resource | filter_templates -w "category=CDROM"'
  plugin: shell
  name: optical/cdrom-write-automated_`ls /sys$path/block`
  requires: device.path == "$path"
@@ -133,7 +133,7 @@
  device.category == 'CDROM'
  optical_drive.dvd == 'writable'
 command:
- cat <<'EOF' | run_templates -t -s 'udev_resource | filter_templates -w "category"'
+ cat <<'EOF' | run_templates -t -s 'udev_resource | filter_templates -w "category=CDROM"'
  plugin: shell
  name: optical/dvd-write-automated_`ls /sys$path/block`
  requires: device.path == "$path"