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

[Marcus Tomlinson]

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

[Jamie Strandboge (jdstrand)]

LGTM. Thanks!

review: Approve

=== modified file 'check-names.list'
--- check-names.list	2015-11-24 12:13:36 +0000
+++ check-names.list	2015-12-01 07:47:30 +0000
@@ -127,6 +127,7 @@
 scope:ini_scope_required_fields|
 scope:ini_scope_section|
 scope:ini_scope_unknown_fields|
+scope:ini_scope_internal_fields|
 scope:peer_hooks_disallowed|
 scope:peer_hooks_required|
 security:apparmor_profile|

=== modified file 'clickreviews/cr_scope.py'
--- clickreviews/cr_scope.py	2015-11-17 12:41:31 +0000
+++ clickreviews/cr_scope.py	2015-12-01 07:47:30 +0000
@@ -120,6 +120,7 @@
             translated = ['description',
                           'displayname',
                           'searchhint']
+            internal = ['debugmode']
 
             missing = []
             t = 'info'
@@ -146,7 +147,7 @@
             unknown = []
             for i in self.scopes[app]["scope_config"]['ScopeConfig'].keys():
                 f = i.lower()
-                if f not in required and f not in optional and \
+                if f not in required and f not in optional and f not in internal and \
                    (f.split("[")[0] not in translated or not
                        re.search('.*\[[a-z]{2,3}(_[a-z]{2,3})?\]$', f)):
                     unknown.append(f)
@@ -162,3 +163,22 @@
                     self.scopes[app]["ini_file_rel"],
                     ", ".join(unknown))
             self._add_result(t, n, s)
+
+            t = 'info'
+            n = self._get_check_name('ini_scope_internal_fields', app=app)
+            s = "OK"
+            forbidden = []
+            for r in internal:
+                if r in self.scopes[app]["scope_config"]['ScopeConfig']:
+                    forbidden.append(r)
+            if len(forbidden) == 1:
+                t = 'error'
+                s = "Forbidden field in '%s': %s" % (
+                    self.scopes[app]["ini_file_rel"],
+                    forbidden[0])
+            elif len(forbidden) > 1:
+                t = 'error'
+                s = "Forbidden fields in '%s': %s" % (
+                    self.scopes[app]["ini_file_rel"],
+                    ", ".join(forbidden))
+            self._add_result(t, n, s)

=== modified file 'clickreviews/tests/test_cr_scope.py'
--- clickreviews/tests/test_cr_scope.py	2015-11-17 12:41:31 +0000
+++ clickreviews/tests/test_cr_scope.py	2015-12-01 07:47:30 +0000
@@ -62,7 +62,7 @@
         c = ClickReviewScope(self.test_name)
         c.check_scope_ini()
         r = c.click_report
-        expected_counts = {'info': 3, 'warn': 0, 'error': 0}
+        expected_counts = {'info': 4, 'warn': 0, 'error': 0}
         self.check_results(r, expected_counts)
 
     def test_check_scope_ini_missing_required1(self):
@@ -197,6 +197,19 @@
         expected_counts = {'info': None, 'warn': 1, 'error': 0}
         self.check_results(r, expected_counts)
 
+    def test_check_scope_ini_forbidden_field(self):
+        '''Test check_scope_ini() - forbidden field'''
+        config = self._stub_config()
+        config['debugmode'] = "true"
+        scope = self._create_scope(config)
+
+        self.set_test_scope(self.default_appname, scope)
+        c = ClickReviewScope(self.test_name)
+        c.check_scope_ini()
+        r = c.click_report
+        expected_counts = {'info': None, 'warn': 0, 'error': 1}
+        self.check_results(r, expected_counts)
+
     def test_check_peer_hooks(self):
         '''Test check_peer_hooks()'''
         c = ClickReviewScope(self.test_name)