Merge lp:~pete-woods/click-reviewers-tools/wrong-scope-ini-path into lp:click-reviewers-tools

[Pete Woods]

Change scope reviewing rules to match current ini file path and format

Change scope reviewing rules to match current ini file path and format

See specification at http://bazaar.launchpad.net/~unity-team/unity-scopes-api/trunk/view/head:/CONFIGFILES

[lp:~pete-woods/click-reviewers-tools/wrong-scope-ini-path]

219. By Pete Woods on 2014-07-30

Match scope review with actual ini file specifications

[Jamie Strandboge (jdstrand)]

ACK, thanks! It might be nice to add some checks for the various fields along with tests, but that is a future enhancement.

review: Approve

[Jamie Strandboge (jdstrand)]

Oh, I forgot to mention, there were whitespace pep8 issues as reported by ./run-pep8. I fixed those.

[Pete Woods (pete-woods)]

Yeah. I was tempted to do that. But ideally want to get this released soon so that scopes can start flowing into the store.

We should also add validation for the Appearance section properties.

[Pete Woods (pete-woods)]

Thanks for the whitespace fixes :)

=== modified file 'clickreviews/cr_scope.py'
--- clickreviews/cr_scope.py	2014-07-16 17:47:07 +0000
+++ clickreviews/cr_scope.py	2014-07-30 16:44:52 +0000
@@ -21,6 +21,8 @@
 import os
 
 
+KNOWN_SECTIONS = set(["ScopeConfig", "Appearance"])
+
 class ClickReviewScope(ClickReview):
     '''This class represents click lint reviews'''
     def __init__(self, fn):
@@ -48,7 +50,7 @@
         elif not os.path.isdir(fn):
             error("'%s' is not a directory" % bn)
 
-        ini_fn = os.path.join(fn, "%s.ini" % self.manifest['name'])
+        ini_fn = os.path.join(fn, "%s_%s.ini" % (self.manifest['name'], app))
         ini_fn_bn = os.path.relpath(ini_fn, self.unpack_dir)
         if not os.path.exists(ini_fn):
             error("Could not find scope INI file '%s'" % ini_fn_bn)
@@ -72,13 +74,15 @@
             n = 'ini_%s_scope_section' % app
             s = "OK"
 
-            if len(self.scopes[app]["scope_config"].sections()) > 1:
+            sections = set(self.scopes[app]["scope_config"].sections())
+            unknown_sections = sections.difference(KNOWN_SECTIONS)
+
+            if unknown_sections:
                 t = 'error'
-                s = "'%s' has too many sections: %s" % (
+                s = "'%s' has unknown sections: %s" % (
                     self.scopes[app]["ini_file_rel"],
-                    ", ".join(self.scopes[app]["scope_config"].sections()))
-            elif "ScopeConfig" not in \
-                    self.scopes[app]["scope_config"].sections():
+                    ", ".join(unknown_sections))
+            elif "ScopeConfig" not in sections:
                 t = 'error'
                 s = "Could not find 'ScopeConfig' in '%s'" % (
                     self.scopes[app]["ini_file_rel"])
@@ -87,13 +91,18 @@
             self._add_result(t, n, s)
 
             # Make these all lower case for easier comparisons
-            required = ['scoperunner',
-                        'displayname',
+            required = ['author',
+                        'description',
+                        'displayname']
+            optional = ['art',
+                        'hotkey',
                         'icon',
+                        'idletimeout',
+                        'invisible',
+                        'locationdataneeded',
+                        'resultsttltype',
+                        'scoperunner',
                         'searchhint']
-            optional = ['description',
-                        'author',
-                        'art']
 
             missing = []
             t = 'info'

=== modified file 'clickreviews/tests/test_cr_scope.py'
--- clickreviews/tests/test_cr_scope.py	2014-07-15 18:35:09 +0000
+++ clickreviews/tests/test_cr_scope.py	2014-07-30 16:44:52 +0000
@@ -31,8 +31,9 @@
         '''Create a scope to pass to tests'''
         scope = dict()
         scope["dir_rel"] = "scope-directory"
-        scope["ini_file_rel"] = "%s/%s.ini" % (scope["dir_rel"],
-                                               self.default_appname)
+        scope["ini_file_rel"] = "%s/%s_%s.ini" % (scope["dir_rel"],
+                                               self.default_appname,
+                                               'foo')
         scope["scope_config"] = configparser.ConfigParser()
         scope["scope_config"]['ScopeConfig'] = config_dict
 
@@ -48,6 +49,11 @@
             'Art': '',
             'Icon': 'foo.svg',
             'SearchHint': 'Search Foo',
+            'HotKey': 'h',
+            'IdleTimeout': '1234',
+            'Invisible': 'false',
+            'LocationDataNeeded': 'false',
+            'ResultsTtlType': 'small',
         }
 
         return config_dict
@@ -64,9 +70,9 @@
         self.check_results(r, expected_counts)
 
     def test_check_scope_ini_missing_required1(self):
-        '''Test check_scope_ini() - missing ScopeRunner'''
+        '''Test check_scope_ini() - missing Description'''
         config = self._stub_config()
-        del config['ScopeRunner']
+        del config['Description']
         scope = self._create_scope(config)
 
         self.set_test_scope(self.default_appname, scope)
@@ -90,9 +96,9 @@
         self.check_results(r, expected_counts)
 
     def test_check_scope_ini_missing_required3(self):
-        '''Test check_scope_ini() - missing Icon'''
+        '''Test check_scope_ini() - missing Author'''
         config = self._stub_config()
-        del config['Icon']
+        del config['Author']
         scope = self._create_scope(config)
 
         self.set_test_scope(self.default_appname, scope)
@@ -103,25 +109,11 @@
         self.check_results(r, expected_counts)
 
     def test_check_scope_ini_missing_required4(self):
-        '''Test check_scope_ini() - missing SearchHint'''
-        config = self._stub_config()
-        del config['SearchHint']
-        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_scope_ini_missing_required5(self):
         '''Test check_scope_ini() - missing multiple'''
         config = self._stub_config()
-        del config['ScopeRunner']
+        del config['Description']
         del config['DisplayName']
-        del config['Icon']
-        del config['SearchHint']
+        del config['Author']
         scope = self._create_scope(config)
 
         self.set_test_scope(self.default_appname, scope)