Merge lp:~jdstrand/click-reviewers-tools/lp1346481 into lp:click-reviewers-tools
- lp1346481
- Merge into trunk
Status: | Merged | ||||
---|---|---|---|---|---|
Merged at revision: | 213 | ||||
Proposed branch: | lp:~jdstrand/click-reviewers-tools/lp1346481 | ||||
Merge into: | lp:click-reviewers-tools | ||||
Diff against target: |
790 lines (+539/-17) 13 files modified
bin/click-check-push-helper (+31/-0) bin/click-run-checks (+4/-0) bin/click-show-files (+13/-0) clickreviews/cr_common.py (+2/-1) clickreviews/cr_desktop.py (+5/-1) clickreviews/cr_lint.py (+15/-5) clickreviews/cr_push_helper.py (+132/-0) clickreviews/cr_security.py (+44/-4) clickreviews/cr_tests.py (+43/-1) clickreviews/tests/test_cr_lint.py (+26/-4) clickreviews/tests/test_cr_push_helper.py (+138/-0) clickreviews/tests/test_cr_security.py (+84/-0) debian/changelog (+2/-1) |
||||
To merge this branch: | bzr merge lp:~jdstrand/click-reviewers-tools/lp1346481 | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
John Lenton (community) | Approve | ||
Martin Albisetti (community) | Approve | ||
Review via email: mp+228187@code.launchpad.net |
Commit message
Description of the change
add push-helper checks:
- add bin/click-
- add clickreviews/
- add clickreviews/
- update bin/click-
- update bin/click-
- update clickreviews/
clickreviews/
- skip missing desktop hook with push-helper
- comment out noisy msg()
clickreviews/
- add push-helper as known hook
- allow multiple apps in click, but not multiple desktop apps
- clarify check_hooks() error
clickreviews/
clickreviews/
- add template tests for push-helper
- add policy_groups tests for push-helper
- clarify check_policy_
clickreviews/
clickreviews/
- 226. By Jamie Strandboge
-
merge with trunk:
[ Daniel Holbach ]
refer to documentation about click in case we encounter .deb packages. - 227. By Jamie Strandboge
-
clickreviews/
cr_common. py: fix pep8 error
John Lenton (chipaca) wrote : | # |
Martin Albisetti (beuno) wrote : | # |
312 + if 'push-helper' not in self.manifest[
313 + # self._add_result(t, n, s)
314 + continue
Why the commenting out?
(the same a few lines lower)
LGTM otherwise.
- 228. By Jamie Strandboge
-
clickreviews/
cr_security. py: remove confusing comment from
check_template_push_helpers( ) - 229. By Jamie Strandboge
-
clickreviews/
cr_security. py: remove confusing comment from
check_policy_ groups_ push_helpers( )
Jamie Strandboge (jdstrand) wrote : | # |
> 312 + if 'push-helper' not in self.manifest[
> 313 + # self._add_result(t, n, s)
> 314 + continue
>
> Why the commenting out?
> (the same a few lines lower)
>
Ah, these should have been omitted from the commit. Fixed. (There is no reason to run this test if the app doesn't specify a push-helper hook).
>
> LGTM otherwise.
Thanks!
John Lenton (chipaca) : | # |
Jamie Strandboge (jdstrand) wrote : | # |
Thanks for the reviews! :)
Preview Diff
1 | === added file 'bin/click-check-push-helper' |
2 | --- bin/click-check-push-helper 1970-01-01 00:00:00 +0000 |
3 | +++ bin/click-check-push-helper 2014-07-24 21:41:48 +0000 |
4 | @@ -0,0 +1,31 @@ |
5 | +#!/usr/bin/python3 |
6 | +'''click-check-push-helper: perform click push-helper checks''' |
7 | +# |
8 | +# Copyright (C) 2014 Canonical Ltd. |
9 | +# |
10 | +# This program is free software: you can redistribute it and/or modify |
11 | +# it under the terms of the GNU General Public License as published by |
12 | +# the Free Software Foundation; version 3 of the License. |
13 | +# |
14 | +# This program is distributed in the hope that it will be useful, |
15 | +# but WITHOUT ANY WARRANTY; without even the implied warranty of |
16 | +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
17 | +# GNU General Public License for more details. |
18 | +# |
19 | +# You should have received a copy of the GNU General Public License |
20 | +# along with this program. If not, see <http://www.gnu.org/licenses/>. |
21 | + |
22 | +from __future__ import print_function |
23 | +import sys |
24 | + |
25 | +import clickreviews.cr_common as cr_common |
26 | +import clickreviews.cr_push_helper as cr_push_helper |
27 | + |
28 | +if __name__ == "__main__": |
29 | + if len(sys.argv) < 2: |
30 | + cr_common.error("Must give path to click package") |
31 | + |
32 | + review = cr_push_helper.ClickReviewPushHelper(sys.argv[1]) |
33 | + review.do_checks() |
34 | + rc = review.do_report() |
35 | + sys.exit(rc) |
36 | |
37 | === modified file 'bin/click-run-checks' |
38 | --- bin/click-run-checks 2014-07-16 17:56:05 +0000 |
39 | +++ bin/click-run-checks 2014-07-24 21:41:48 +0000 |
40 | @@ -56,6 +56,10 @@ |
41 | prefer_local click-check-online-accounts "$c" |
42 | |
43 | echo "" |
44 | +echo "= click-check-push-helper =" |
45 | +prefer_local click-check-push-helper "$c" |
46 | + |
47 | +echo "" |
48 | echo "= click-check-scope =" |
49 | prefer_local click-check-scope "$c" |
50 | |
51 | |
52 | === modified file 'bin/click-show-files' |
53 | --- bin/click-show-files 2014-07-18 14:18:06 +0000 |
54 | +++ bin/click-show-files 2014-07-24 21:41:48 +0000 |
55 | @@ -27,6 +27,7 @@ |
56 | from clickreviews import cr_scope |
57 | from clickreviews import cr_content_hub |
58 | from clickreviews import cr_online_accounts |
59 | +from clickreviews import cr_push_helper |
60 | |
61 | # This script just dumps important files to stdout |
62 | |
63 | @@ -86,6 +87,18 @@ |
64 | print("") |
65 | cr_common.cleanup_unpack() |
66 | |
67 | + review_push_helper = cr_push_helper.ClickReviewPushHelper(sys.argv[1]) |
68 | + for app in sorted(review_push_helper.push_helper_files): |
69 | + f = review_push_helper.push_helper_files[app] |
70 | + fh = cr_common.open_file_read(os.path.join( |
71 | + review_push_helper.unpack_dir, f)) |
72 | + print("== push_helper: %s ==" % os.path.basename(f)) |
73 | + for line in fh.readlines(): |
74 | + print(line, end="") |
75 | + fh.close() |
76 | + print("") |
77 | + cr_common.cleanup_unpack() |
78 | + |
79 | review_scope = cr_scope.ClickReviewScope(sys.argv[1]) |
80 | for app in sorted(review_scope.scopes): |
81 | f = review_scope.scopes[app]["ini_file"] |
82 | |
83 | === modified file 'clickreviews/cr_common.py' |
84 | --- clickreviews/cr_common.py 2014-06-24 13:51:19 +0000 |
85 | +++ clickreviews/cr_common.py 2014-07-24 21:41:48 +0000 |
86 | @@ -62,7 +62,8 @@ |
87 | if not self.click_package.endswith(".click"): |
88 | if self.click_package.endswith(".deb"): |
89 | error("filename does not end with '.click', but '.deb' " |
90 | - "instead. See http://askubuntu.com/a/485544/94326 for how click packages are different.") |
91 | + "instead. See http://askubuntu.com/a/485544/94326 for " |
92 | + "how click packages are different.") |
93 | error("filename does not end with '.click'") |
94 | |
95 | self.review_type = review_type |
96 | |
97 | === modified file 'clickreviews/cr_desktop.py' |
98 | --- clickreviews/cr_desktop.py 2014-05-30 08:33:09 +0000 |
99 | +++ clickreviews/cr_desktop.py 2014-07-24 21:41:48 +0000 |
100 | @@ -37,7 +37,11 @@ |
101 | for app in self.manifest['hooks']: |
102 | if 'desktop' not in self.manifest['hooks'][app]: |
103 | if 'scope' in self.manifest['hooks'][app]: |
104 | - msg("Skipped missing desktop hook for scope '%s'" % app) |
105 | + # msg("Skipped missing desktop hook for scope '%s'" % app) |
106 | + continue |
107 | + if 'push-helper' in self.manifest['hooks'][app]: |
108 | + # msg("Skipped missing desktop hook for push-helper '%s'" % |
109 | + # app) |
110 | continue |
111 | else: |
112 | error("could not find desktop hook for '%s'" % app) |
113 | |
114 | === modified file 'clickreviews/cr_lint.py' |
115 | --- clickreviews/cr_lint.py 2014-07-16 19:07:21 +0000 |
116 | +++ clickreviews/cr_lint.py 2014-07-24 21:41:48 +0000 |
117 | @@ -69,6 +69,7 @@ |
118 | 'content-hub', |
119 | 'desktop', |
120 | 'pay-ui', |
121 | + 'push-helper', |
122 | 'scope', |
123 | 'urls'] |
124 | |
125 | @@ -281,11 +282,20 @@ |
126 | # Some checks are already handled in |
127 | # cr_common.py:_verify_manifest_structure() |
128 | |
129 | - # We don't support multiple apps in 13.10 |
130 | - if len(self.manifest['hooks'].keys()) != 1: |
131 | - self._add_result('error', 'hooks', |
132 | - "more than one app key specified in hooks") |
133 | - return |
134 | + # While we support multiple apps in the hooks db, we don't support |
135 | + # multiple apps specifying desktop hooks. Eg, it is ok to specify a |
136 | + # scope, an app and a push-helper, but it isn't ok to specify two apps |
137 | + t = 'info' |
138 | + n = 'hooks_multiple_apps' |
139 | + s = 'OK' |
140 | + count = 0 |
141 | + for app in self.manifest['hooks']: |
142 | + if "desktop" in self.manifest['hooks'][app]: |
143 | + count += 1 |
144 | + if count > 1: |
145 | + t = 'error' |
146 | + s = 'more than one desktop app specified in hooks' |
147 | + self._add_result(t, n, s) |
148 | |
149 | # Verify keys are well-formatted |
150 | for app in self.manifest['hooks']: |
151 | |
152 | === added file 'clickreviews/cr_push_helper.py' |
153 | --- clickreviews/cr_push_helper.py 1970-01-01 00:00:00 +0000 |
154 | +++ clickreviews/cr_push_helper.py 2014-07-24 21:41:48 +0000 |
155 | @@ -0,0 +1,132 @@ |
156 | +'''cr_push_helper.py: click push-helper checks''' |
157 | +# |
158 | +# Copyright (C) 2014 Canonical Ltd. |
159 | +# |
160 | +# This program is free software: you can redistribute it and/or modify |
161 | +# it under the terms of the GNU General Public License as published by |
162 | +# the Free Software Foundation; version 3 of the License. |
163 | +# |
164 | +# This program is distributed in the hope that it will be useful, |
165 | +# but WITHOUT ANY WARRANTY; without even the implied warranty of |
166 | +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
167 | +# GNU General Public License for more details. |
168 | +# |
169 | +# You should have received a copy of the GNU General Public License |
170 | +# along with this program. If not, see <http://www.gnu.org/licenses/>. |
171 | + |
172 | +from __future__ import print_function |
173 | + |
174 | +from clickreviews.cr_common import ClickReview, error, open_file_read, msg |
175 | +import json |
176 | +import os |
177 | + |
178 | + |
179 | +class ClickReviewPushHelper(ClickReview): |
180 | + '''This class represents click lint reviews''' |
181 | + def __init__(self, fn): |
182 | + ClickReview.__init__(self, fn, "push_helper") |
183 | + |
184 | + self.required_keys = ['exec'] |
185 | + self.optional_keys = ['app_id'] |
186 | + |
187 | + self.push_helper_files = dict() # click-show-files and tests |
188 | + self.push_helper = dict() |
189 | + for app in self.manifest['hooks']: |
190 | + if 'push-helper' not in self.manifest['hooks'][app]: |
191 | + # msg("Skipped missing push-helper hook for '%s'" % app) |
192 | + continue |
193 | + if not isinstance(self.manifest['hooks'][app]['push-helper'], str): |
194 | + error("manifest malformed: hooks/%s/urls is not str" % app) |
195 | + (full_fn, jd) = self._extract_push_helper(app) |
196 | + self.push_helper_files[app] = full_fn |
197 | + self.push_helper[app] = jd |
198 | + |
199 | + def _extract_push_helper(self, app): |
200 | + '''Get push-helper hook content''' |
201 | + c = self.manifest['hooks'][app]['push-helper'] |
202 | + fn = os.path.join(self.unpack_dir, c) |
203 | + |
204 | + bn = os.path.basename(fn) |
205 | + if not os.path.exists(fn): |
206 | + error("Could not find '%s'" % bn) |
207 | + |
208 | + fh = open_file_read(fn) |
209 | + contents = "" |
210 | + for line in fh.readlines(): |
211 | + contents += line |
212 | + fh.close() |
213 | + |
214 | + try: |
215 | + jd = json.loads(contents) |
216 | + except Exception as e: |
217 | + error("push-helper json unparseable: %s (%s):\n%s" % (bn, |
218 | + str(e), contents)) |
219 | + |
220 | + if not isinstance(jd, dict): |
221 | + error("push-helper json is malformed: %s:\n%s" % (bn, contents)) |
222 | + |
223 | + return (fn, jd) |
224 | + |
225 | + def check_valid(self): |
226 | + '''Check validity of push-helper entries''' |
227 | + for app in sorted(self.push_helper): |
228 | + for k in self.push_helper[app].keys(): |
229 | + t = "info" |
230 | + n = "valid_%s_%s" % (app, k) |
231 | + s = "OK" |
232 | + |
233 | + if not isinstance(self.push_helper[app][k], str): |
234 | + t = "error" |
235 | + s = "'%s' is not a string" % k |
236 | + elif self.push_helper[app][k] == "": |
237 | + t = "error" |
238 | + s = "'%s' is empty" % k |
239 | + self._add_result(t, n, s) |
240 | + |
241 | + for k in self.required_keys: |
242 | + t = "info" |
243 | + n = "valid_%s_required_%s" % (app, k) |
244 | + s = "OK" |
245 | + if k not in self.push_helper[app]: |
246 | + t = "error" |
247 | + s = "'%s' is missing" % k |
248 | + self._add_result(t, n, s) |
249 | + |
250 | + def check_unknown_keys(self): |
251 | + '''Check unknown''' |
252 | + for app in sorted(self.push_helper): |
253 | + unknown = [] |
254 | + t = "info" |
255 | + n = "unknown_%s" % app |
256 | + s = "OK" |
257 | + for key in self.push_helper[app].keys(): |
258 | + if key not in self.required_keys and \ |
259 | + key not in self.optional_keys: |
260 | + unknown.append(key) |
261 | + if len(unknown) == 1: |
262 | + t = "warn" |
263 | + s = "Unknown field '%s'" % unknown[0] |
264 | + elif len(unknown) > 1: |
265 | + t = "warn" |
266 | + s = "Unknown fields '%s'" % ", ".join(unknown) |
267 | + self._add_result(t, n, s) |
268 | + |
269 | + def check_hooks(self): |
270 | + '''Verify combinations of click hooks with the push-helper hook''' |
271 | + for app in sorted(self.manifest['hooks']): |
272 | + if app not in self.push_helper: |
273 | + continue |
274 | + |
275 | + t = "info" |
276 | + n = "other_hooks_%s" % app |
277 | + s = "OK" |
278 | + |
279 | + bad = [] |
280 | + for hook in self.manifest['hooks'][app]: |
281 | + # Only the apparmor hook can be used with the push-helper |
282 | + if hook not in ['push-helper', 'apparmor']: |
283 | + bad.append(hook) |
284 | + if len(bad) > 0: |
285 | + t = 'error' |
286 | + s = "Found unusual hooks with push-helper: %s" % ", ".join(bad) |
287 | + self._add_result(t, n, s) |
288 | |
289 | === modified file 'clickreviews/cr_security.py' |
290 | --- clickreviews/cr_security.py 2014-07-18 19:47:48 +0000 |
291 | +++ clickreviews/cr_security.py 2014-07-24 21:41:48 +0000 |
292 | @@ -66,6 +66,8 @@ |
293 | 'video', |
294 | 'webview'] |
295 | |
296 | + self.allowed_push_helper_policy_groups = ['push-notification-client'] |
297 | + |
298 | self.redflag_templates = ['unconfined'] |
299 | self.extraneous_templates = ['ubuntu-sdk', |
300 | 'default'] |
301 | @@ -357,6 +359,44 @@ |
302 | |
303 | self._add_result(t, n, s) |
304 | |
305 | + def check_template_push_helpers(self): |
306 | + '''Check template for push-helpers''' |
307 | + for app in sorted(self.manifest['hooks']): |
308 | + (f, m) = self._get_security_manifest(app) |
309 | + t = 'info' |
310 | + n = 'template_push_helper(%s)' % f |
311 | + s = "OK" |
312 | + if 'push-helper' not in self.manifest['hooks'][app]: |
313 | + continue |
314 | + if 'template' in m and m['template'] != "ubuntu-sdk": |
315 | + t = 'error' |
316 | + s = "template is not 'ubuntu-sdk'" |
317 | + self._add_result(t, n, s) |
318 | + |
319 | + def check_policy_groups_push_helpers(self): |
320 | + '''Check policy_groups for push-helpers''' |
321 | + for app in sorted(self.manifest['hooks']): |
322 | + (f, m) = self._get_security_manifest(app) |
323 | + t = 'info' |
324 | + n = 'policy_groups_push_helper(%s)' % f |
325 | + s = "OK" |
326 | + if 'push-helper' not in self.manifest['hooks'][app]: |
327 | + continue |
328 | + if 'policy_groups' not in m or \ |
329 | + 'push-notification-client' not in m['policy_groups']: |
330 | + self._add_result('error', n, |
331 | + "required group 'push-notification-client' " |
332 | + "not found") |
333 | + continue |
334 | + bad = [] |
335 | + for p in m['policy_groups']: |
336 | + if p not in self.allowed_push_helper_policy_groups: |
337 | + bad.append(p) |
338 | + if len(bad) > 0: |
339 | + t = 'error' |
340 | + s = "found unusual policy groups: %s" % ", ".join(bad) |
341 | + self._add_result(t, n, s) |
342 | + |
343 | def check_policy_groups_scopes(self): |
344 | '''Check policy_groups for scopes''' |
345 | for app in sorted(self.manifest['hooks']): |
346 | @@ -397,7 +437,7 @@ |
347 | (f, m) = self._get_security_manifest(app) |
348 | |
349 | t = 'info' |
350 | - n = 'policy_groups_exists (%s)' % f |
351 | + n = 'policy_groups_exists_%s (%s)' % (app, f) |
352 | if 'policy_groups' not in m: |
353 | # If template not specified, we just use the default |
354 | self._add_result('warn', n, 'no policy groups specified') |
355 | @@ -423,7 +463,7 @@ |
356 | |
357 | # Check for duplicates |
358 | t = 'info' |
359 | - n = 'policy_groups_duplicates (%s)' % f |
360 | + n = 'policy_groups_duplicates_%s (%s)' % (app, f) |
361 | s = 'OK' |
362 | tmp = [] |
363 | for p in m['policy_groups']: |
364 | @@ -438,7 +478,7 @@ |
365 | # If we got here, we can see if valid policy groups were specified |
366 | for i in m['policy_groups']: |
367 | t = 'info' |
368 | - n = 'policy_groups_valid (%s)' % i |
369 | + n = 'policy_groups_valid_%s (%s)' % (app, i) |
370 | s = 'OK' |
371 | |
372 | # SDK will leave and empty policy group, report but don't |
373 | @@ -461,7 +501,7 @@ |
374 | |
375 | if found: |
376 | t = 'info' |
377 | - n = 'policy_groups_safe (%s)' % i |
378 | + n = 'policy_groups_safe_%s (%s)' % (app, i) |
379 | s = 'OK' |
380 | |
381 | aa_type = self._get_policy_group_type(vendor, version, i) |
382 | |
383 | === modified file 'clickreviews/cr_tests.py' |
384 | --- clickreviews/cr_tests.py 2014-07-16 18:13:22 +0000 |
385 | +++ clickreviews/cr_tests.py 2014-07-24 21:41:48 +0000 |
386 | @@ -39,6 +39,7 @@ |
387 | TEST_ACCOUNTS_PROVIDER = dict() |
388 | TEST_ACCOUNTS_QML_PLUGIN = dict() |
389 | TEST_ACCOUNTS_SERVICE = dict() |
390 | +TEST_PUSH_HELPER = dict() |
391 | |
392 | |
393 | # |
394 | @@ -148,6 +149,11 @@ |
395 | return (f, val) |
396 | |
397 | |
398 | +def _extract_push_helper(self, app): |
399 | + '''Pretend we read the push-helper file''' |
400 | + return ("%s.push-helper.json" % app, TEST_PUSH_HELPER[app]) |
401 | + |
402 | + |
403 | # http://docs.python.org/3.4/library/unittest.mock-examples.html |
404 | # Mock patching. Don't use decorators but instead patch in setUp() of the |
405 | # child. Set up a list of patches, but don't start them. Create the helper |
406 | @@ -228,6 +234,11 @@ |
407 | 'clickreviews.cr_online_accounts.ClickReviewAccounts._extract_account', |
408 | _extract_account)) |
409 | |
410 | +# push-helper overrides |
411 | +patches.append(patch( |
412 | + 'clickreviews.cr_push_helper.ClickReviewPushHelper._extract_push_helper', |
413 | + _extract_push_helper)) |
414 | + |
415 | |
416 | def mock_patch(): |
417 | '''Call in setup of child''' |
418 | @@ -295,6 +306,7 @@ |
419 | self.test_accounts_provider = dict() |
420 | self.test_accounts_qml_plugin = dict() |
421 | self.test_accounts_service = dict() |
422 | + self.test_push_helper = dict() |
423 | for app in self.test_manifest["hooks"].keys(): |
424 | # setup security manifest for each app |
425 | self.set_test_security_manifest(app, 'policy_groups', |
426 | @@ -330,6 +342,9 @@ |
427 | self.set_test_account(app, "account-qml-plugin", None) |
428 | self.set_test_account(app, "account-service", None) |
429 | |
430 | + # Reset to no content-hub entries in manifest |
431 | + self.set_test_push_helper(app, None, None) |
432 | + |
433 | self._update_test_security_manifests() |
434 | self._update_test_desktop_files() |
435 | self._update_test_url_dispatcher() |
436 | @@ -339,6 +354,7 @@ |
437 | self._update_test_accounts_provider() |
438 | self._update_test_accounts_qml_plugin() |
439 | self._update_test_accounts_service() |
440 | + self._update_test_push_helper() |
441 | |
442 | # webapps manifests (leave empty for now) |
443 | self.test_webapp_manifests = dict() |
444 | @@ -444,6 +460,15 @@ |
445 | "%s.service" % app |
446 | self._update_test_manifest() |
447 | |
448 | + def _update_test_push_helper(self): |
449 | + global TEST_PUSH_HELPER |
450 | + TEST_PUSH_HELPER = dict() |
451 | + for app in self.test_push_helper.keys(): |
452 | + TEST_PUSH_HELPER[app] = self.test_push_helper[app] |
453 | + self.test_manifest["hooks"][app]["push-helper"] = \ |
454 | + "%s.push-helper.json" % app |
455 | + self._update_test_manifest() |
456 | + |
457 | def _update_test_name(self): |
458 | self.test_name = "%s_%s_%s.click" % (self.test_control['Package'], |
459 | self.test_control['Version'], |
460 | @@ -618,6 +643,21 @@ |
461 | elif account_type == "account-service": |
462 | self._update_test_accounts_service() |
463 | |
464 | + def set_test_push_helper(self, app, key, value): |
465 | + '''Set push-helper entries. If value is None, remove key, if key is |
466 | + None, remove content-hub from manifest''' |
467 | + if key is None: |
468 | + if app in self.test_push_helper: |
469 | + self.test_push_helper.pop(app) |
470 | + elif value is None: |
471 | + if key in self.test_push_helper[app]: |
472 | + self.test_push_helper[app].pop(key) |
473 | + else: |
474 | + if app not in self.test_push_helper: |
475 | + self.test_push_helper[app] = dict() |
476 | + self.test_push_helper[app][key] = value |
477 | + self._update_test_push_helper() |
478 | + |
479 | def setUp(self): |
480 | '''Make sure our patches are applied everywhere''' |
481 | global patches |
482 | @@ -647,7 +687,9 @@ |
483 | global TEST_ACCOUNTS_QML_PLUGIN |
484 | TEST_ACCOUNTS_QML_PLUGIN = dict() |
485 | global TEST_ACCOUNTS_SERVICE |
486 | - TEST_ACCOUNTS_APPLICTION = dict() |
487 | + TEST_ACCOUNTS_APPLICATION = dict() |
488 | + global TEST_PUSH_HELPER |
489 | + TEST_PUSH_HELPER = dict() |
490 | |
491 | self._reset_test_data() |
492 | cr_common.recursive_rm(self.desktop_tmpdir) |
493 | |
494 | === modified file 'clickreviews/tests/test_cr_lint.py' |
495 | --- clickreviews/tests/test_cr_lint.py 2014-07-16 14:48:39 +0000 |
496 | +++ clickreviews/tests/test_cr_lint.py 2014-07-24 21:41:48 +0000 |
497 | @@ -644,15 +644,37 @@ |
498 | expected_counts = {'info': None, 'warn': 0, 'error': 0} |
499 | self.check_results(r, expected_counts) |
500 | |
501 | + def test_check_hooks_multiple_desktop_apps(self): |
502 | + '''Test check_hooks() - multiple desktop apps''' |
503 | + self.set_test_manifest("framework", "ubuntu-sdk-13.10") |
504 | + c = ClickReviewLint(self.test_name) |
505 | + tmp = c.manifest['hooks'][self.default_appname] |
506 | + c.manifest['hooks']["another-app"] = tmp |
507 | + c.check_hooks() |
508 | + r = c.click_report |
509 | + expected_counts = {'info': None, 'warn': 0, 'error': 1} |
510 | + self.check_results(r, expected_counts) |
511 | + |
512 | def test_check_hooks_multiple_apps(self): |
513 | - '''Test check_hooks() - multiple apps''' |
514 | + '''Test check_hooks() - multiple non-desktop apps''' |
515 | self.set_test_manifest("framework", "ubuntu-sdk-13.10") |
516 | c = ClickReviewLint(self.test_name) |
517 | - tmp = c.manifest['hooks'][self.default_appname] |
518 | - c.manifest['hooks']["another-app"] = tmp |
519 | + tmp = dict() |
520 | + for k in c.manifest['hooks'][self.default_appname].keys(): |
521 | + tmp[k] = c.manifest['hooks'][self.default_appname][k] |
522 | + tmp.pop('desktop') |
523 | + tmp['scope'] = "some-scope-exec" |
524 | + c.manifest['hooks']["some-scope"] = tmp |
525 | + tmp = dict() |
526 | + for k in c.manifest['hooks'][self.default_appname].keys(): |
527 | + tmp[k] = c.manifest['hooks'][self.default_appname][k] |
528 | + tmp.pop('desktop') |
529 | + tmp['push-helper'] = "push.json" |
530 | + c.manifest['hooks']["some-push-helper"] = tmp |
531 | + |
532 | c.check_hooks() |
533 | r = c.click_report |
534 | - expected_counts = {'info': None, 'warn': 0, 'error': 1} |
535 | + expected_counts = {'info': 7, 'warn': 0, 'error': 0} |
536 | self.check_results(r, expected_counts) |
537 | |
538 | def test_check_hooks_bad_appname(self): |
539 | |
540 | === added file 'clickreviews/tests/test_cr_push_helper.py' |
541 | --- clickreviews/tests/test_cr_push_helper.py 1970-01-01 00:00:00 +0000 |
542 | +++ clickreviews/tests/test_cr_push_helper.py 2014-07-24 21:41:48 +0000 |
543 | @@ -0,0 +1,138 @@ |
544 | +'''test_cr_push_helper.py: tests for the cr_push_helper module''' |
545 | +# |
546 | +# Copyright (C) 2013 Canonical Ltd. |
547 | +# |
548 | +# This program is free software: you can redistribute it and/or modify |
549 | +# it under the terms of the GNU General Public License as published by |
550 | +# the Free Software Foundation; version 3 of the License. |
551 | +# |
552 | +# This program is distributed in the hope that it will be useful, |
553 | +# but WITHOUT ANY WARRANTY; without even the implied warranty of |
554 | +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
555 | +# GNU General Public License for more details. |
556 | +# |
557 | +# You should have received a copy of the GNU General Public License |
558 | +# along with this program. If not, see <http://www.gnu.org/licenses/>. |
559 | + |
560 | +from clickreviews.cr_push_helper import ClickReviewPushHelper |
561 | +import clickreviews.cr_tests as cr_tests |
562 | + |
563 | + |
564 | +class TestClickReviewPushHelper(cr_tests.TestClickReview): |
565 | + """Tests for the lint review tool.""" |
566 | + def setUp(self): |
567 | + # Monkey patch various file access classes. stop() is handled with |
568 | + # addCleanup in super() |
569 | + cr_tests.mock_patch() |
570 | + super() |
571 | + |
572 | + def test_check_unknown_keys_none(self): |
573 | + '''Test check_unknown() - no unknown''' |
574 | + self.set_test_push_helper(self.default_appname, "exec", "foo") |
575 | + c = ClickReviewPushHelper(self.test_name) |
576 | + c.check_unknown_keys() |
577 | + r = c.click_report |
578 | + expected_counts = {'info': 1, 'warn': 0, 'error': 0} |
579 | + self.check_results(r, expected_counts) |
580 | + |
581 | + def test_check_unknown_keys1(self): |
582 | + '''Test check_unknown() - one unknown''' |
583 | + self.set_test_push_helper(self.default_appname, "nonexistent", "foo") |
584 | + c = ClickReviewPushHelper(self.test_name) |
585 | + c.check_unknown_keys() |
586 | + r = c.click_report |
587 | + expected_counts = {'info': 0, 'warn': 1, 'error': 0} |
588 | + self.check_results(r, expected_counts) |
589 | + |
590 | + def test_check_unknown_keys2(self): |
591 | + '''Test check_unknown() - good with one unknown''' |
592 | + self.set_test_push_helper(self.default_appname, "exec", "foo") |
593 | + self.set_test_push_helper(self.default_appname, "nonexistent", "foo") |
594 | + c = ClickReviewPushHelper(self.test_name) |
595 | + c.check_unknown_keys() |
596 | + r = c.click_report |
597 | + expected_counts = {'info': 0, 'warn': 1, 'error': 0} |
598 | + self.check_results(r, expected_counts) |
599 | + |
600 | + def test_check_valid_exec(self): |
601 | + '''Test check_valid() - exec''' |
602 | + self.set_test_push_helper(self.default_appname, "exec", "foo") |
603 | + c = ClickReviewPushHelper(self.test_name) |
604 | + c.check_valid() |
605 | + r = c.click_report |
606 | + expected_counts = {'info': 2, 'warn': 0, 'error': 0} |
607 | + self.check_results(r, expected_counts) |
608 | + |
609 | + def test_check_valid_missing_exec(self): |
610 | + '''Test check_valid() - missing exec''' |
611 | + self.set_test_push_helper(self.default_appname, "app_id", "foo_foo") |
612 | + c = ClickReviewPushHelper(self.test_name) |
613 | + c.check_valid() |
614 | + r = c.click_report |
615 | + expected_counts = {'info': None, 'warn': 0, 'error': 1} |
616 | + self.check_results(r, expected_counts) |
617 | + |
618 | + def test_check_valid_app_id(self): |
619 | + '''Test check_valid() - app_id''' |
620 | + self.set_test_push_helper(self.default_appname, "exec", "foo") |
621 | + self.set_test_push_helper(self.default_appname, "app_id", "foo_foo") |
622 | + c = ClickReviewPushHelper(self.test_name) |
623 | + c.check_valid() |
624 | + r = c.click_report |
625 | + expected_counts = {'info': 3, 'warn': 0, 'error': 0} |
626 | + self.check_results(r, expected_counts) |
627 | + |
628 | + def test_check_valid_bad_value(self): |
629 | + '''Test check_valid() - bad value''' |
630 | + self.set_test_push_helper(self.default_appname, "exec", []) |
631 | + c = ClickReviewPushHelper(self.test_name) |
632 | + c.check_valid() |
633 | + r = c.click_report |
634 | + expected_counts = {'info': None, 'warn': 0, 'error': 1} |
635 | + self.check_results(r, expected_counts) |
636 | + |
637 | + def test_check_valid_empty_value(self): |
638 | + '''Test check_valid() - empty value''' |
639 | + self.set_test_push_helper(self.default_appname, "exec", "foo") |
640 | + self.set_test_push_helper(self.default_appname, "app_id", "") |
641 | + c = ClickReviewPushHelper(self.test_name) |
642 | + c.check_valid() |
643 | + r = c.click_report |
644 | + expected_counts = {'info': None, 'warn': 0, 'error': 1} |
645 | + self.check_results(r, expected_counts) |
646 | + |
647 | + def test_check_valid_empty_value2(self): |
648 | + '''Test check_valid() - empty value''' |
649 | + self.set_test_push_helper(self.default_appname, "exec", "") |
650 | + self.set_test_push_helper(self.default_appname, "app_id", "foo_foo") |
651 | + c = ClickReviewPushHelper(self.test_name) |
652 | + c.check_valid() |
653 | + r = c.click_report |
654 | + expected_counts = {'info': None, 'warn': 0, 'error': 1} |
655 | + self.check_results(r, expected_counts) |
656 | + |
657 | + def test_check_hooks(self): |
658 | + '''Test check_hooks()''' |
659 | + self.set_test_push_helper(self.default_appname, "exec", "foo") |
660 | + c = ClickReviewPushHelper(self.test_name) |
661 | + |
662 | + # remove hooks that are added by the framework |
663 | + c.manifest['hooks'][self.default_appname].pop('desktop') |
664 | + c.manifest['hooks'][self.default_appname].pop('urls') |
665 | + |
666 | + c.check_hooks() |
667 | + r = c.click_report |
668 | + expected_counts = {'info': 1, 'warn': 0, 'error': 0} |
669 | + self.check_results(r, expected_counts) |
670 | + |
671 | + def test_check_hooks_bad(self): |
672 | + '''Test check_hooks() - bad''' |
673 | + self.set_test_push_helper(self.default_appname, "exec", "foo") |
674 | + c = ClickReviewPushHelper(self.test_name) |
675 | + |
676 | + # The desktop and urls hooks are specified by default in the framework, |
677 | + # so just running this without other setup should generate an error |
678 | + c.check_hooks() |
679 | + r = c.click_report |
680 | + expected_counts = {'info': None, 'warn': 0, 'error': 1} |
681 | + self.check_results(r, expected_counts) |
682 | |
683 | === modified file 'clickreviews/tests/test_cr_security.py' |
684 | --- clickreviews/tests/test_cr_security.py 2014-07-14 16:40:36 +0000 |
685 | +++ clickreviews/tests/test_cr_security.py 2014-07-24 21:41:48 +0000 |
686 | @@ -600,3 +600,87 @@ |
687 | report = c.click_report |
688 | expected_counts = {'info': None, 'warn': 0, 'error': 1} |
689 | self.check_results(report, expected_counts) |
690 | + |
691 | + def test_check_policy_groups_pushhelper_no_hook(self): |
692 | + '''Test check_policy_groups_pushhelper() - no hook''' |
693 | + c = ClickReviewSecurity(self.test_name) |
694 | + c.check_policy_groups_push_helpers() |
695 | + report = c.click_report |
696 | + expected_counts = {'info': 0, 'warn': 0, 'error': 0} |
697 | + self.check_results(report, expected_counts) |
698 | + |
699 | + def test_check_policy_groups_pushhelper(self): |
700 | + '''Test check_policy_groups_pushhelper()''' |
701 | + self.set_test_push_helper(self.default_appname, "exec", "foo") |
702 | + self.set_test_security_manifest(self.default_appname, |
703 | + "policy_groups", |
704 | + ["push-notification-client"]) |
705 | + c = ClickReviewSecurity(self.test_name) |
706 | + c.check_policy_groups_push_helpers() |
707 | + report = c.click_report |
708 | + expected_counts = {'info': 1, 'warn': 0, 'error': 0} |
709 | + self.check_results(report, expected_counts) |
710 | + |
711 | + def test_check_policy_groups_pushhelper_missing(self): |
712 | + '''Test check_policy_groups_pushhelper - missing''' |
713 | + self.set_test_push_helper(self.default_appname, "exec", "foo") |
714 | + self.set_test_security_manifest(self.default_appname, |
715 | + "policy_groups", |
716 | + None) |
717 | + c = ClickReviewSecurity(self.test_name) |
718 | + c.check_policy_groups_push_helpers() |
719 | + report = c.click_report |
720 | + expected_counts = {'info': None, 'warn': 0, 'error': 1} |
721 | + self.check_results(report, expected_counts) |
722 | + |
723 | + def test_check_policy_groups_pushhelper_bad(self): |
724 | + '''Test check_policy_groups_pushhelper - bad''' |
725 | + self.set_test_push_helper(self.default_appname, "exec", "foo") |
726 | + self.set_test_security_manifest(self.default_appname, |
727 | + "policy_groups", |
728 | + ["video_files", |
729 | + "networking", |
730 | + "push-notification-client"]) |
731 | + c = ClickReviewSecurity(self.test_name) |
732 | + c.check_policy_groups_push_helpers() |
733 | + report = c.click_report |
734 | + expected_counts = {'info': None, 'warn': 0, 'error': 1} |
735 | + self.check_results(report, expected_counts) |
736 | + |
737 | + def test_check_template_pushhelper(self): |
738 | + '''Test check_template_pushhelper''' |
739 | + self.set_test_push_helper(self.default_appname, "exec", "foo") |
740 | + self.set_test_security_manifest(self.default_appname, |
741 | + "template", "ubuntu-sdk") |
742 | + self.set_test_security_manifest(self.default_appname, |
743 | + "policy_groups", |
744 | + ["push-notification-client"]) |
745 | + c = ClickReviewSecurity(self.test_name) |
746 | + c.check_template_push_helpers() |
747 | + report = c.click_report |
748 | + expected_counts = {'info': 1, 'warn': 0, 'error': 0} |
749 | + self.check_results(report, expected_counts) |
750 | + |
751 | + def test_check_template_pushhelper_no_hook(self): |
752 | + '''Test check_template_pushhelper''' |
753 | + self.set_test_security_manifest(self.default_appname, |
754 | + "template", "ubuntu-sdk") |
755 | + c = ClickReviewSecurity(self.test_name) |
756 | + c.check_template_push_helpers() |
757 | + report = c.click_report |
758 | + expected_counts = {'info': 0, 'warn': 0, 'error': 0} |
759 | + self.check_results(report, expected_counts) |
760 | + |
761 | + def test_check_template_pushhelper_wrong_template(self): |
762 | + '''Test check_template_pushhelper - wrong template()''' |
763 | + self.set_test_push_helper(self.default_appname, "exec", "foo") |
764 | + self.set_test_security_manifest(self.default_appname, |
765 | + "template", "ubuntu-webapp") |
766 | + self.set_test_security_manifest(self.default_appname, |
767 | + "policy_groups", |
768 | + ["push-notification-client"]) |
769 | + c = ClickReviewSecurity(self.test_name) |
770 | + c.check_template_push_helpers() |
771 | + report = c.click_report |
772 | + expected_counts = {'info': None, 'warn': 0, 'error': 1} |
773 | + self.check_results(report, expected_counts) |
774 | |
775 | === modified file 'debian/changelog' |
776 | --- debian/changelog 2014-07-23 07:53:35 +0000 |
777 | +++ debian/changelog 2014-07-24 21:41:48 +0000 |
778 | @@ -26,11 +26,12 @@ |
779 | group in 1.2 (LP: #1340869) |
780 | * refactor the way we handle apparmor policy into a central static list |
781 | which should be easy to update. |
782 | + * implement push-helper tests |
783 | |
784 | [ Daniel Holbach ] |
785 | * refer to documentation about click in case we encounter .deb packages. |
786 | |
787 | - -- Jamie Strandboge <jamie@ubuntu.com> Fri, 18 Jul 2014 14:51:46 -0500 |
788 | + -- Jamie Strandboge <jamie@ubuntu.com> Thu, 24 Jul 2014 14:04:23 -0500 |
789 | |
790 | click-reviewers-tools (0.7.1) utopic; urgency=medium |
791 |
I haven't been able to go through the tests' tests yet, but have some nitpicks inline below.