Merge lp:~bladernr/checkbox/1103343-rendercheck-test-fix into lp:checkbox

Proposed by Jeff Lane 
Status: Merged
Approved by: Daniel Manrique
Approved revision: 1902
Merged at revision: 1901
Proposed branch: lp:~bladernr/checkbox/1103343-rendercheck-test-fix
Merge into: lp:checkbox
Diff against target: 47 lines (+6/-2)
3 files modified
debian/changelog (+4/-1)
jobs/rendercheck.txt.in (+1/-1)
scripts/rendercheck_test (+1/-0)
To merge this branch: bzr merge lp:~bladernr/checkbox/1103343-rendercheck-test-fix
Reviewer Review Type Date Requested Status
Daniel Manrique (community) Approve
Review via email: mp+144590@code.launchpad.net

Description of the change

Jeffrey Chang noticed that when rendercheck_test is run on a system without rendercheck installed, checkbox still records a passing error. This appears to be because rendercheck_test depended on the errno module, but that module was never imported. This MR adds the missing import for errno which resolves the issue, causing rendercheck to exit with a fail code properly if rendercheck is not present on the SUT.

Additionally, I've reworded the command definition to properly return a fail code if rendercheck fails to run or exits with errors.

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

Short and sweet. I checked the shell logic with some dummy commands, and I agree on the errno inclusion. I guess the original author didn't test on a system *without* rendercheck :)

Anyway, merging.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/changelog'
2--- debian/changelog 2013-01-22 20:06:57 +0000
3+++ debian/changelog 2013-01-23 21:33:23 +0000
4@@ -8,6 +8,9 @@
5 [ Jeff Lane ]
6 * jobs/monitor.txt.in - added new job monitor/multi-head to test mulitple
7 displays on desktops.
8+ * scripts/rendercheck_test - added missing import for errno (LP: #1103343)
9+ jobs/rendercheck.txt.in - fixed command string to report the correct exit
10+ code to checkbox
11
12 [ Brendan Donegan ]
13 * jobs/bluetooth.txt.in, jobs/suspend.txt.in - unblock Bluetooth hardware
14@@ -21,7 +24,7 @@
15 in config file in the event the system under test does not have internet
16 access. Updated jobs/virtualization.txt.in
17
18- -- Jeff Marcom <jeff.marcom@ubuntu.com> Wed, 09 Jan 2013 16:40:02 -0500
19+ -- Jeff Marcom <jeff.marcom@ubuntu.com> Wed, 23 Jan 2013 16:30:22 -0500
20
21 checkbox (0.15) raring; urgency=low
22
23
24=== modified file 'jobs/rendercheck.txt.in'
25--- jobs/rendercheck.txt.in 2012-09-23 16:51:13 +0000
26+++ jobs/rendercheck.txt.in 2013-01-23 21:33:23 +0000
27@@ -2,7 +2,7 @@
28 name: rendercheck/tests
29 requires:
30 package.name == 'x11-apps'
31-command: rendercheck_test -d -o $CHECKBOX_DATA/rendercheck-results && echo "Rendercheck tests completed successfully" || echo "Rendercheck completed, but there are errors. Please see the log $CHECKBOX_DATA/rendercheck-results for details"
32+command: ( rendercheck_test -d -o $CHECKBOX_DATA/rendercheck-results && echo "Rendercheck tests completed successfully" ) || ( echo "Error running rendercheck. Please see the log $CHECKBOX_DATA/rendercheck-results for details" >&2 && false )
33 _description:
34 Runs all of the rendercheck test suites. This test can take a few minutes.
35
36
37=== modified file 'scripts/rendercheck_test'
38--- scripts/rendercheck_test 2012-09-12 12:16:52 +0000
39+++ scripts/rendercheck_test 2013-01-23 21:33:23 +0000
40@@ -29,6 +29,7 @@
41 import os
42 import re
43 import tempfile
44+import errno
45
46
47 class RenderCheck(object):

Subscribers

People subscribed via source and target branches