Merge lp:~marcustomlinson/click-reviewers-tools/lp-1511063 into lp:click-reviewers-tools

Proposed by Marcus Tomlinson on 2015-12-01
Status: Merged
Merged at revision: 554
Proposed branch: lp:~marcustomlinson/click-reviewers-tools/lp-1511063
Merge into: lp:click-reviewers-tools
Diff against target: 88 lines (+36/-2)
3 files modified
check-names.list (+1/-0)
clickreviews/cr_scope.py (+21/-1)
clickreviews/tests/test_cr_scope.py (+14/-1)
To merge this branch: bzr merge lp:~marcustomlinson/click-reviewers-tools/lp-1511063
Reviewer Review Type Date Requested Status
Jamie Strandboge 2015-12-01 Approve on 2015-12-01
Review via email: mp+279073@code.launchpad.net

Commit message

Forbid the internal "DebugMode" scope.ini key from making its way into the store

To post a comment you must log in.
Jamie Strandboge (jdstrand) wrote :

LGTM. Thanks!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'check-names.list'
2--- check-names.list 2015-11-24 12:13:36 +0000
3+++ check-names.list 2015-12-01 07:47:30 +0000
4@@ -127,6 +127,7 @@
5 scope:ini_scope_required_fields|
6 scope:ini_scope_section|
7 scope:ini_scope_unknown_fields|
8+scope:ini_scope_internal_fields|
9 scope:peer_hooks_disallowed|
10 scope:peer_hooks_required|
11 security:apparmor_profile|
12
13=== modified file 'clickreviews/cr_scope.py'
14--- clickreviews/cr_scope.py 2015-11-17 12:41:31 +0000
15+++ clickreviews/cr_scope.py 2015-12-01 07:47:30 +0000
16@@ -120,6 +120,7 @@
17 translated = ['description',
18 'displayname',
19 'searchhint']
20+ internal = ['debugmode']
21
22 missing = []
23 t = 'info'
24@@ -146,7 +147,7 @@
25 unknown = []
26 for i in self.scopes[app]["scope_config"]['ScopeConfig'].keys():
27 f = i.lower()
28- if f not in required and f not in optional and \
29+ if f not in required and f not in optional and f not in internal and \
30 (f.split("[")[0] not in translated or not
31 re.search('.*\[[a-z]{2,3}(_[a-z]{2,3})?\]$', f)):
32 unknown.append(f)
33@@ -162,3 +163,22 @@
34 self.scopes[app]["ini_file_rel"],
35 ", ".join(unknown))
36 self._add_result(t, n, s)
37+
38+ t = 'info'
39+ n = self._get_check_name('ini_scope_internal_fields', app=app)
40+ s = "OK"
41+ forbidden = []
42+ for r in internal:
43+ if r in self.scopes[app]["scope_config"]['ScopeConfig']:
44+ forbidden.append(r)
45+ if len(forbidden) == 1:
46+ t = 'error'
47+ s = "Forbidden field in '%s': %s" % (
48+ self.scopes[app]["ini_file_rel"],
49+ forbidden[0])
50+ elif len(forbidden) > 1:
51+ t = 'error'
52+ s = "Forbidden fields in '%s': %s" % (
53+ self.scopes[app]["ini_file_rel"],
54+ ", ".join(forbidden))
55+ self._add_result(t, n, s)
56
57=== modified file 'clickreviews/tests/test_cr_scope.py'
58--- clickreviews/tests/test_cr_scope.py 2015-11-17 12:41:31 +0000
59+++ clickreviews/tests/test_cr_scope.py 2015-12-01 07:47:30 +0000
60@@ -62,7 +62,7 @@
61 c = ClickReviewScope(self.test_name)
62 c.check_scope_ini()
63 r = c.click_report
64- expected_counts = {'info': 3, 'warn': 0, 'error': 0}
65+ expected_counts = {'info': 4, 'warn': 0, 'error': 0}
66 self.check_results(r, expected_counts)
67
68 def test_check_scope_ini_missing_required1(self):
69@@ -197,6 +197,19 @@
70 expected_counts = {'info': None, 'warn': 1, 'error': 0}
71 self.check_results(r, expected_counts)
72
73+ def test_check_scope_ini_forbidden_field(self):
74+ '''Test check_scope_ini() - forbidden field'''
75+ config = self._stub_config()
76+ config['debugmode'] = "true"
77+ scope = self._create_scope(config)
78+
79+ self.set_test_scope(self.default_appname, scope)
80+ c = ClickReviewScope(self.test_name)
81+ c.check_scope_ini()
82+ r = c.click_report
83+ expected_counts = {'info': None, 'warn': 0, 'error': 1}
84+ self.check_results(r, expected_counts)
85+
86 def test_check_peer_hooks(self):
87 '''Test check_peer_hooks()'''
88 c = ClickReviewScope(self.test_name)

Subscribers

People subscribed via source and target branches