Merge lp:~bladernr/checkbox/1111878-cpu_topology-KeyError into lp:checkbox

[Jeff Lane ]

fixed cpuinfo part to define the keys for each cpu dict at creation time rather than during parsing. This should resolve a corner case where /proc/cpuinfo may not have a 'physical id' or 'core id' item, causing the script to throw a KeyError later on.

[Daniel Manrique (roadmr)]

Looks OK, reasonable solution. Let's hope the reporter sends his cpuinfo dump to test this!

review: Approve

[Daniel Manrique (roadmr)]

The attempt to merge lp:~bladernr/checkbox/1111878-cpu_topology-KeyError into lp:checkbox failed. Below is the output from the failed tests.

[precise] Bringing VM 'up'
[precise] Unable to 'up' VM!
[precise] stdout: http://paste.ubuntu.com/5623597/
[precise] stderr: http://paste.ubuntu.com/5623598/
[precise] NOTE: unable to execute tests, marked as failed
[quantal] Bringing VM 'up'
Timing for [quantal] Bringing VM 'up'
10.56user 4.82system 3:52.97elapsed 6%CPU (0avgtext+0avgdata 21516maxresident)k
7472inputs+176outputs (69major+209713minor)pagefaults 0swaps
[quantal] Starting tests...
[quantal] CheckBox test suite: pass
Timing for Checkbox test suite
1.62user 0.62system 0:15.99elapsed 14%CPU (0avgtext+0avgdata 20052maxresident)k
17536inputs+16outputs (110major+53917minor)pagefaults 0swaps
Timing for refreshing plainbox installation
0.82user 0.32system 0:06.19elapsed 18%CPU (0avgtext+0avgdata 19820maxresident)k
0inputs+16outputs (0major+55266minor)pagefaults 0swaps
[quantal] PlainBox test suite: pass
Timing for plainbox test suite
0.96user 0.35system 0:14.59elapsed 9%CPU (0avgtext+0avgdata 19832maxresident)k
0inputs+176outputs (0major+55566minor)pagefaults 0swaps
[quantal] PlainBox documentation build: pass
Timing for plainbox documentation build
0.86user 0.34system 0:16.82elapsed 7%CPU (0avgtext+0avgdata 20004maxresident)k
0inputs+16outputs (0major+53619minor)pagefaults 0swaps
[quantal] Integration tests: pass
Timing for integration tests
0.84user 0.35system 0:10.26elapsed 11%CPU (0avgtext+0avgdata 20516maxresident)k
0inputs+8outputs (0major+54328minor)pagefaults 0swaps
[quantal] Destroying VM
[raring] Bringing VM 'up'
Timing for [raring] Bringing VM 'up'
8.77user 4.14system 6:07.05elapsed 3%CPU (0avgtext+0avgdata 21144maxresident)k
480inputs+240outputs (1major+234127minor)pagefaults 0swaps
[raring] Starting tests...
[raring] CheckBox test suite: pass
Timing for Checkbox test suite
1.26user 0.52system 0:12.99elapsed 13%CPU (0avgtext+0avgdata 20712maxresident)k
6888inputs+16outputs (76major+54588minor)pagefaults 0swaps
Timing for refreshing plainbox installation
0.86user 0.31system 0:06.44elapsed 18%CPU (0avgtext+0avgdata 20456maxresident)k
0inputs+16outputs (0major+51871minor)pagefaults 0swaps
[raring] PlainBox test suite: pass
Timing for plainbox test suite
0.95user 0.40system 0:13.46elapsed 10%CPU (0avgtext+0avgdata 19852maxresident)k
0inputs+176outputs (0major+55004minor)pagefaults 0swaps
[raring] PlainBox documentation build: pass
Timing for plainbox documentation build
0.81user 0.35system 0:10.68elapsed 10%CPU (0avgtext+0avgdata 19792maxresident)k
0inputs+16outputs (0major+54977minor)pagefaults 0swaps
[raring] Integration tests: pass
Timing for integration tests
0.80user 0.34system 0:08.30elapsed 13%CPU (0avgtext+0avgdata 19888maxresident)k
0inputs+8outputs (0major+55194minor)pagefaults 0swaps
[raring] Destroying VM

[Daniel Manrique (roadmr)]

Reapprove, tarmac blew up. If it fails again it will require... human intervention.

review: Approve

[Zygmunt Krynicki (zyga)]

To quote:

"""fixed cpuinfo part to define the keys for each cpu dict at creation time rather than during parsing. This should resolve a corner case where /proc/cpuinfo may not have a 'physical id' or 'core id' item, causing the script to throw a KeyError later on."""

If the corner case you talk about is arm then I would see this as a stop-gap until we get a proper parser for arm cpuinfo.

[Jeff Lane  (bladernr)]

Oh I agree, we do need a proper parser for ARM, but this was to address a weird bug on an Intel system. We DEFINITELY will need to address this and the DMI type of info for ARM though.

=== modified file 'debian/changelog'
--- debian/changelog	2013-05-01 17:30:38 +0000
+++ debian/changelog	2013-05-01 18:26:31 +0000
@@ -6,6 +6,8 @@
     fail cases, we don't need that much for success cases. (LP: #1078897) 
   * jobs/mediacard.txt.in: Modified test instructions to be less confusing
     (LP: #970857)
+  * scripts/cpu_topology: define the cpuinfo nested dicts on creation rather
+    than define elements during parsing of /proc/cpuinfo (LP: #1111878)
 
   [ Sylvain Pineau ]
   * jobs/suspend.txt.in, scripts/gpu_test: Remove the need of running the script

=== modified file 'scripts/cpu_topology'
--- scripts/cpu_topology	2012-07-11 16:48:15 +0000
+++ scripts/cpu_topology	2013-05-01 18:26:31 +0000
@@ -23,7 +23,7 @@
         for i in temp:
             if i.startswith('processor'):
                 key = 'cpu' + (i.split(':')[1].strip())
-                self.cpuinfo[key] = {}
+                self.cpuinfo[key] = {'core_id':'', 'physical_package_id':''}
             elif i.startswith('core id'):
                 self.cpuinfo[key].update({'core_id': i.split(':')[1].strip()})
             elif i.startswith('physical id'):