Merge lp:~pwlars/lava-test/json-compat-fix into lp:lava-test/0.0
- json-compat-fix
- Merge into trunk
Status: | Merged |
---|---|
Merged at revision: | 35 |
Proposed branch: | lp:~pwlars/lava-test/json-compat-fix |
Merge into: | lp:lava-test/0.0 |
Diff against target: |
173 lines (+61/-28) 3 files modified
abrek/hwprofile.py (+16/-16) abrek/testdef.py (+8/-7) tests/test_hwprofile.py (+37/-5) |
To merge this branch: | bzr merge lp:~pwlars/lava-test/json-compat-fix |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
James Westby (community) | Approve | ||
Review via email: mp+36583@code.launchpad.net |
Commit message
Description of the change
These are needed in preparation for being able to submit results to launch-control. Launch-control currently fails silently and accepts invalid results, but if you look into it further you can see that it doesn't properly handle them unless these fixes are in place.
Thanks,
Paul Larson
Zygmunt Krynicki (zyga) wrote : | # |
These are needed in preparation for being able to submit results to launch-control. Launch-control currently fails silently and accepts invalid results,
Yes, you can upload anything as validation and processing is going to be asynchronous anyway. Note that you _can_ check wat went wrong by looking at BundleDeseriali
but if you look into it further you can see that it doesn't properly handle them unless these fixes are in place.
It can handle correct documents, they just need to adhere to schema.
Best regards.
Zygmunt Krynicki
- 36. By Paul Larson
-
match variable names to field names better in hwprofile
- 37. By Paul Larson
-
Make hw_profile tests more complete
Paul Larson (pwlars) wrote : | # |
> 28 - device['desc'] = desc
> 29 + device[
> Can we have the local variables not share the name of a key they are
> not for?
Done
> There are also more code changes than test changes, is the test
> coverage good enough in this area?
Changed this a bit. The tests look at the whole dict that should result from converting the cpuinfo, meminfo, etc. This doesn't add more tests, but makes the existing tests more complete.
James Westby (james-w) wrote : | # |
Hi,
Much better, thanks.
34 - device['desc'] = desc
35 + device[
I don't know how tricky it would be to clean that one up as well, but
if you can that would be great.
Thanks,
James
Paul Larson (pwlars) wrote : | # |
On Mon, 2010-09-27 at 15:22 +0000, James Westby wrote:
> 34 - device['desc'] = desc
> 35 + device[
Bah, missed that one. Thanks!
Preview Diff
1 | === modified file 'abrek/hwprofile.py' | |||
2 | --- abrek/hwprofile.py 2010-09-18 04:25:23 +0000 | |||
3 | +++ abrek/hwprofile.py 2010-09-27 14:22:38 +0000 | |||
4 | @@ -95,8 +95,8 @@ | |||
5 | 95 | for c in range(len(cpudevs)): | 95 | for c in range(len(cpudevs)): |
6 | 96 | device = {} | 96 | device = {} |
7 | 97 | device['device_type'] = 'device.cpu' | 97 | device['device_type'] = 'device.cpu' |
10 | 98 | device['attributes'] = 'Processor #{0}'.format(c) | 98 | device['description'] = 'Processor #{0}'.format(c) |
11 | 99 | device['desc'] = cpudevs[c] | 99 | device['attributes'] = cpudevs[c] |
12 | 100 | devices.append(device) | 100 | devices.append(device) |
13 | 101 | except IOError: | 101 | except IOError: |
14 | 102 | print >> sys.stderr, "WARNING: Could not read cpu information" | 102 | print >> sys.stderr, "WARNING: Could not read cpu information" |
15 | @@ -113,11 +113,11 @@ | |||
16 | 113 | machine = os.uname()[-1] | 113 | machine = os.uname()[-1] |
17 | 114 | if machine in ('i686', 'x86_64'): | 114 | if machine in ('i686', 'x86_64'): |
18 | 115 | try: | 115 | try: |
20 | 116 | name = read_file('/sys/class/dmi/id/board_name') or None | 116 | description = read_file('/sys/class/dmi/id/board_name') or None |
21 | 117 | vendor = read_file('/sys/class/dmi/id/board_vendor') or None | 117 | vendor = read_file('/sys/class/dmi/id/board_vendor') or None |
22 | 118 | version = read_file('/sys/class/dmi/id/board_version') or None | 118 | version = read_file('/sys/class/dmi/id/board_version') or None |
25 | 119 | if name: | 119 | if description: |
26 | 120 | device['attributes'] = name.strip() | 120 | device['description'] = description.strip() |
27 | 121 | if vendor: | 121 | if vendor: |
28 | 122 | desc['vendor'] = vendor.strip() | 122 | desc['vendor'] = vendor.strip() |
29 | 123 | if version: | 123 | if version: |
30 | @@ -140,7 +140,7 @@ | |||
31 | 140 | return devices | 140 | return devices |
32 | 141 | else: | 141 | else: |
33 | 142 | return devices | 142 | return devices |
35 | 143 | device['desc'] = desc | 143 | device['attributes'] = desc |
36 | 144 | device['device_type'] = 'device.board' | 144 | device['device_type'] = 'device.board' |
37 | 145 | devices.append(device) | 145 | devices.append(device) |
38 | 146 | return devices | 146 | return devices |
39 | @@ -166,11 +166,11 @@ | |||
40 | 166 | kind = 'RAM' | 166 | kind = 'RAM' |
41 | 167 | else: | 167 | else: |
42 | 168 | kind = 'swap' | 168 | kind = 'swap' |
44 | 169 | name = "{capacity}MiB of {kind}".format( | 169 | description = "{capacity}MiB of {kind}".format( |
45 | 170 | capacity=capacity >> 20, kind=kind) | 170 | capacity=capacity >> 20, kind=kind) |
46 | 171 | device = {} | 171 | device = {} |
49 | 172 | device['attributes'] = name | 172 | device['description'] = description |
50 | 173 | device['desc'] = {'capacity': capacity, 'kind': kind} | 173 | device['attributes'] = {'capacity': capacity, 'kind': kind} |
51 | 174 | device['device_type'] = "device.mem" | 174 | device['device_type'] = "device.mem" |
52 | 175 | devices.append(device) | 175 | devices.append(device) |
53 | 176 | except IOError: | 176 | except IOError: |
54 | @@ -183,18 +183,18 @@ | |||
55 | 183 | """ | 183 | """ |
56 | 184 | pattern = re.compile( | 184 | pattern = re.compile( |
57 | 185 | "^Bus \d{3} Device \d{3}: ID (?P<vendor_id>[0-9a-f]{4}):" | 185 | "^Bus \d{3} Device \d{3}: ID (?P<vendor_id>[0-9a-f]{4}):" |
59 | 186 | "(?P<product_id>[0-9a-f]{4}) (?P<name>.*)$") | 186 | "(?P<product_id>[0-9a-f]{4}) (?P<description>.*)$") |
60 | 187 | devices = [] | 187 | devices = [] |
61 | 188 | for line in commands.getoutput('lsusb').splitlines(): | 188 | for line in commands.getoutput('lsusb').splitlines(): |
62 | 189 | match = pattern.match(line) | 189 | match = pattern.match(line) |
63 | 190 | if match: | 190 | if match: |
66 | 191 | vendor_id, product_id, name = match.groups() | 191 | vendor_id, product_id, description = match.groups() |
67 | 192 | desc = {} | 192 | attributes = {} |
68 | 193 | device = {} | 193 | device = {} |
73 | 194 | desc['vendor_id'] = int(vendor_id, 16) | 194 | attributes['vendor_id'] = int(vendor_id, 16) |
74 | 195 | desc['product_id'] = int(product_id, 16) | 195 | attributes['product_id'] = int(product_id, 16) |
75 | 196 | device['desc'] = desc | 196 | device['attributes'] = attributes |
76 | 197 | device['attributes'] = name | 197 | device['description'] = description |
77 | 198 | device['device_type'] = 'device.usb' | 198 | device['device_type'] = 'device.usb' |
78 | 199 | devices.append(device) | 199 | devices.append(device) |
79 | 200 | return devices | 200 | return devices |
80 | 201 | 201 | ||
81 | === modified file 'abrek/testdef.py' | |||
82 | --- abrek/testdef.py 2010-09-21 22:35:55 +0000 | |||
83 | +++ abrek/testdef.py 2010-09-27 14:22:38 +0000 | |||
84 | @@ -92,17 +92,18 @@ | |||
85 | 92 | 92 | ||
86 | 93 | def _savetestdata(self): | 93 | def _savetestdata(self): |
87 | 94 | TIMEFORMAT = '%Y-%m-%dT%H:%M:%SZ' | 94 | TIMEFORMAT = '%Y-%m-%dT%H:%M:%SZ' |
89 | 95 | testdata = {} | 95 | testdata = [{}] |
90 | 96 | testdata['format'] = "Dashboard Bundle Format 1.0" | ||
91 | 96 | filename = os.path.join(self.resultsdir, 'testdata.json') | 97 | filename = os.path.join(self.resultsdir, 'testdata.json') |
96 | 97 | testdata['test_id'] = self.testname | 98 | testdata[0]['test_id'] = self.testname |
97 | 98 | testdata['analyzer_assigned_uuid'] = str(uuid1()) | 99 | testdata[0]['analyzer_assigned_uuid'] = str(uuid1()) |
98 | 99 | testdata['time_check_performed'] = False | 100 | testdata[0]['time_check_performed'] = False |
99 | 100 | testdata['analyzer_assigned_date'] = datetime.strftime( | 101 | testdata[0]['analyzer_assigned_date'] = datetime.strftime( |
100 | 101 | self.runner.starttime,TIMEFORMAT) | 102 | self.runner.starttime,TIMEFORMAT) |
101 | 102 | hw = hwprofile.get_hw_context() | 103 | hw = hwprofile.get_hw_context() |
103 | 103 | testdata['hw_context'] = hw | 104 | testdata[0]['hw_context'] = hw |
104 | 104 | sw = swprofile.get_sw_context() | 105 | sw = swprofile.get_sw_context() |
106 | 105 | testdata['sw_context'] = sw | 106 | testdata[0]['sw_context'] = sw |
107 | 106 | write_file(json.dumps(testdata), filename) | 107 | write_file(json.dumps(testdata), filename) |
108 | 107 | 108 | ||
109 | 108 | def run(self, quiet=False): | 109 | def run(self, quiet=False): |
110 | 109 | 110 | ||
111 | === modified file 'tests/test_hwprofile.py' | |||
112 | --- tests/test_hwprofile.py 2010-09-18 04:26:46 +0000 | |||
113 | +++ tests/test_hwprofile.py 2010-09-27 14:22:38 +0000 | |||
114 | @@ -79,22 +79,54 @@ | |||
115 | 79 | def test_get_cpu_devs(self): | 79 | def test_get_cpu_devs(self): |
116 | 80 | fake_file('/proc/cpuinfo', ARM_CPUINFO_FILE) | 80 | fake_file('/proc/cpuinfo', ARM_CPUINFO_FILE) |
117 | 81 | devs = abrek.hwprofile.get_cpu_devs() | 81 | devs = abrek.hwprofile.get_cpu_devs() |
120 | 82 | processor = "ARMv7 Processor rev 3 (v7l)" | 82 | cpuinfo = { |
121 | 83 | self.assertEqual(processor, devs[0]['desc']['Processor']) | 83 | 'attributes': { |
122 | 84 | 'CPU implementer': '0x41', | ||
123 | 85 | 'Features': 'swp half thumb fastmult vfp edsp neon vfpv3*', | ||
124 | 86 | 'CPU architecture': '7', | ||
125 | 87 | 'BogoMIPS': '483.16', | ||
126 | 88 | 'Hardware': 'OMAP3 Beagle Board', | ||
127 | 89 | 'CPU revision': '3', | ||
128 | 90 | 'CPU part': '0xc08', | ||
129 | 91 | 'Serial': '0000000000000000', | ||
130 | 92 | 'Processor': 'ARMv7 Processor rev 3 (v7l)', | ||
131 | 93 | 'CPU variant': '0x1', | ||
132 | 94 | 'Revision': '0020'}, | ||
133 | 95 | 'description': 'Processor #0', | ||
134 | 96 | 'device_type': 'device.cpu'} | ||
135 | 97 | self.assertEqual(cpuinfo, devs[0]) | ||
136 | 84 | 98 | ||
137 | 85 | def test_get_board_devs(self): | 99 | def test_get_board_devs(self): |
138 | 86 | fake_file('/sys/class/dmi/id/board_name', FAKE_BOARDNAME_FILE) | 100 | fake_file('/sys/class/dmi/id/board_name', FAKE_BOARDNAME_FILE) |
139 | 87 | devs = abrek.hwprofile.get_board_devs() | 101 | devs = abrek.hwprofile.get_board_devs() |
141 | 88 | self.assertEqual(FAKE_BOARDNAME_FILE, devs[0]['attributes']) | 102 | boardinfo = { |
142 | 103 | 'attributes': { | ||
143 | 104 | 'version': 'Not Available', | ||
144 | 105 | 'vendor': 'LENOVO'}, | ||
145 | 106 | 'description': 'XXXXXXX', | ||
146 | 107 | 'device_type': 'device.board'} | ||
147 | 108 | self.assertEqual(boardinfo, devs[0]) | ||
148 | 89 | 109 | ||
149 | 90 | def test_get_mem_devs(self): | 110 | def test_get_mem_devs(self): |
150 | 91 | fake_file('/proc/meminfo', FAKE_MEMINFO_FILE) | 111 | fake_file('/proc/meminfo', FAKE_MEMINFO_FILE) |
151 | 92 | devs = abrek.hwprofile.get_mem_devs() | 112 | devs = abrek.hwprofile.get_mem_devs() |
153 | 93 | self.assertEqual(243937280, devs[0]['desc']['capacity']) | 113 | meminfo = { |
154 | 114 | 'attributes': { | ||
155 | 115 | 'kind': 'RAM', | ||
156 | 116 | 'capacity': 243937280}, | ||
157 | 117 | 'description': '232MiB of RAM', | ||
158 | 118 | 'device_type': 'device.mem'} | ||
159 | 119 | self.assertEqual(meminfo, devs[0]) | ||
160 | 94 | 120 | ||
161 | 95 | def test_get_usb_devs(self): | 121 | def test_get_usb_devs(self): |
162 | 96 | devs = abrek.hwprofile.get_usb_devs() | 122 | devs = abrek.hwprofile.get_usb_devs() |
164 | 97 | self.assertEqual('device.usb', devs[0]['device_type']) | 123 | usbinfo = { |
165 | 124 | 'attributes': { | ||
166 | 125 | 'vendor_id': 7531, | ||
167 | 126 | 'product_id': 1}, | ||
168 | 127 | 'description': 'Linux Foundation 1.1 root hub', | ||
169 | 128 | 'device_type': 'device.usb'} | ||
170 | 129 | self.assertEqual(usbinfo, devs[0]) | ||
171 | 98 | 130 | ||
172 | 99 | 131 | ||
173 | 100 | class MissingFiles(TestCaseWithFixtures): | 132 | class MissingFiles(TestCaseWithFixtures): |
Hi,
I find these sorts of things a little confusing:
28 - device['desc'] = desc 'attributes' ] = desc
29 + device[
Can we have the local variables not share the name of a key they are
not for?
There are also more code changes than test changes, is the test
coverage good enough in this area?
Thanks,
James