diff -Nru click-reviewers-tools-0.9/bin/click-review click-reviewers-tools-0.10/bin/click-review --- click-reviewers-tools-0.9/bin/click-review 2014-08-25 07:46:21.000000000 +0000 +++ click-reviewers-tools-0.10/bin/click-review 2014-09-05 13:02:08.000000000 +0000 @@ -1,131 +1,135 @@ #!/usr/bin/python3 +from clickreviews import modules import argparse -import clickreviews -import imp -import inspect import json -import pkgutil +import os import sys -def get_modules(): - ''' - Here we have a look at all the modules in the - clickreviews package and filter out a few which - are not relevant. - - Basically we look at all the ones which are - derived from cr_common, where we can later on - instantiate a *Review* object and run the - necessary checks. - ''' - - all_modules = [name for _, name, _ in - pkgutil.iter_modules(clickreviews.__path__)] - narrow_down_modules = \ - lambda a: a.startswith('cr_') and \ - a not in ['cr_common', 'cr_tests', 'cr_skeleton'] - return list(filter(narrow_down_modules, all_modules)) - - -def init_main_class(module_name, click_file): - ''' - This function will find the Click*Review class in - the specified module and instantiate it with the - location of the .click file we want to inspect. - ''' - - module = imp.load_source(module_name, - '%s/%s.py' % (clickreviews.__path__[0], - module_name)) - - classes = inspect.getmembers(module, inspect.isclass) - find_main_class = lambda a: a[0].startswith('Click') and \ - not a[0].endswith('Exception') and \ - a[1].__module__ == module_name - main_class = list(filter(find_main_class, classes)) - init_object = getattr(module, main_class[0][0]) - return init_object(click_file) - - -def print_findings(which, kind): +def print_findings(results, description): ''' Print a summary of the issues found. ''' - if not which: + if not description or not results: return '' - print(kind) - print(''.center(len(kind), '-')) - for key in which.keys(): + print(description) + print(''.center(len(description), '-')) + for key in results.keys(): print(' - %s' % key) - print('\t%s' % which[key]['text']) - if 'link' in which[key]: - print('\t%s' % which[key]['link']) + print('\t%s' % results[key]['text']) + if 'link' in results[key]: + print('\t%s' % results[key]['link']) -def report(results, args): +class Results(object): + results = {} errors = {} warnings = {} info = {} + rc = 0 - for module in results: - for key in results[module]['error']: - errors[key] = results[module]['error'][key] - for key in results[module]['warn']: - warnings[key] = results[module]['warn'][key] - if args.verbose: - for key in results[module]['info']: - info[key] = results[module]['info'][key] - - if not args.json: - print_findings(errors, 'Errors') - print_findings(warnings, 'Warnings') - if args.verbose: - print_findings(info, 'Info') - if warnings or errors: - print('%s: FAIL' % args.filename) + def __init__(self, args): + self.args = args + self.click_fn = self.args.filename + self.cr_modules = modules.get_modules() + + def _sumarise_results(self): + for module in self.results: + for key in self.results[module]['error']: + self.errors[key] = self.results[module]['error'][key] + for key in self.results[module]['warn']: + self.warnings[key] = self.results[module]['warn'][key] + if self.args.verbose: + for key in self.results[module]['info']: + self.info[key] = self.results[module]['info'][key] + + def _complete_report(self): + self._sumarise_results() + + if self.args.json: + print(json.dumps(self.results, sort_keys=True, indent=2, + separators=(',', ': '))) else: - print('%s: pass' % args.filename) - else: - print(json.dumps(results, sort_keys=True, indent=2, + print_findings(self.errors, 'Errors') + print_findings(self.warnings, 'Warnings') + if self.args.verbose: + print_findings(self.info, 'Info') + if self.warnings or self.errors: + print('%s: FAIL' % self.args.filename) + else: + print('%s: pass' % self.args.filename) + if self.warnings or self.errors: + self.rc = 1 + + def _report_module(self, section): + ''' + This is currently only used in the --sdk option. + It will print the output for each section when it's + available. This will prevent the SDK from having to wait + until all checks have been run. + ''' + output = self.results[section] + print('= %s =' % section) + print(json.dumps(output, sort_keys=True, indent=2, separators=(',', ': '))) - if warnings or errors: - return 1 - return 0 + if output['error'] or output['warn']: + self.rc = 1 + + def _run_module_checks(self, module): + # What we are doing here is basically what all the + # ./bin/click-check-* scripts do as well, so for + # example something like: + # + # review = cr_push_helper.ClickReviewPushHelper(sys.argv[1]) + # review.do_checks() + # rc = review.do_report() + # + section = module.replace('cr_', '') + review = modules.init_main_class(module, self.click_fn) + if review: + review.do_checks() + self.results[section] = review.click_report + return None + + def run_all_checks(self): + if self.args.sdk: + for module in self.cr_modules: + section = self._run_module_checks(module) + if section: + self._report_module(section) + else: + for module in self.cr_modules: + self._run_module_checks(module) + self._complete_report() def main(): parser = argparse.ArgumentParser() parser.add_argument('filename', type=str, help='.click file to be inspected') - parser.add_argument('-v', '--verbose', help='increase output verbosity', + parser.add_argument('-v', '--verbose', + help='increase output verbosity', action='store_true') parser.add_argument('--json', help='print json output', action='store_true') + parser.add_argument('--sdk', + help='use output format suitable for the Ubuntu SDK', + action='store_true') args = parser.parse_args() - click_file = args.filename - # What we are doing here is basically what all the - # ./bin/click-check-* scripts do as well, so for - # example something like: - # - # review = cr_push_helper.ClickReviewPushHelper(sys.argv[1]) - # review.do_checks() - # rc = review.do_report() - # - results = {} - modules = get_modules() - if not modules: + if not os.path.exists(args.filename): + print(".click file '%s' does not exist." % args.filename) + sys.exit(1) + + results = Results(args) + if not results.cr_modules: print("No 'clickreviews' modules found.") sys.exit(1) - for module in modules: - review = init_main_class(module, click_file) - review.do_checks() - results[module.replace('cr_', '')] = review.click_report - rc = report(results, args) - sys.exit(rc) + + results.run_all_checks() + sys.exit(results.rc) if __name__ == '__main__': diff -Nru click-reviewers-tools-0.9/bin/clickreviews/cr_desktop.py click-reviewers-tools-0.10/bin/clickreviews/cr_desktop.py --- click-reviewers-tools-0.9/bin/clickreviews/cr_desktop.py 2014-08-25 07:46:21.000000000 +0000 +++ click-reviewers-tools-0.10/bin/clickreviews/cr_desktop.py 2014-09-05 13:02:08.000000000 +0000 @@ -39,10 +39,13 @@ if 'scope' in self.manifest['hooks'][app]: # msg("Skipped missing desktop hook for scope '%s'" % app) continue - if 'push-helper' in self.manifest['hooks'][app]: + elif 'push-helper' in self.manifest['hooks'][app]: # msg("Skipped missing desktop hook for push-helper '%s'" % # app) continue + elif 'pay-ui' in self.manifest['hooks'][app]: + # msg("Skipped missing desktop hook for pay-ui '%s'" % app) + continue else: error("could not find desktop hook for '%s'" % app) if not isinstance(self.manifest['hooks'][app]['desktop'], str): diff -Nru click-reviewers-tools-0.9/bin/clickreviews/cr_security.py click-reviewers-tools-0.10/bin/clickreviews/cr_security.py --- click-reviewers-tools-0.9/bin/clickreviews/cr_security.py 2014-08-25 07:46:21.000000000 +0000 +++ click-reviewers-tools-0.10/bin/clickreviews/cr_security.py 2014-09-05 13:02:08.000000000 +0000 @@ -60,6 +60,7 @@ 'template_variables', 'write_path'] self.allowed_webapp_policy_groups = ['audio', + # 'camera', non-functional ATM 'content_exchange', 'location', 'networking', diff -Nru click-reviewers-tools-0.9/bin/clickreviews/modules.py click-reviewers-tools-0.10/bin/clickreviews/modules.py --- click-reviewers-tools-0.9/bin/clickreviews/modules.py 1970-01-01 00:00:00.000000000 +0000 +++ click-reviewers-tools-0.10/bin/clickreviews/modules.py 2014-09-05 13:02:08.000000000 +0000 @@ -0,0 +1,71 @@ +import clickreviews +import imp +import inspect +import os +import pkgutil + +IRRELEVANT_MODULES = ['cr_common', 'cr_tests', 'cr_skeleton'] + + +def narrow_down_modules(modules): + ''' + Get a list of file names or module names and filter out + the ones we know are irrelevant. + ''' + relevant_modules = [] + for module in modules: + module_name = os.path.basename(module).replace('.py', '') + if module_name not in IRRELEVANT_MODULES and \ + module_name.startswith('cr_'): + relevant_modules += [module] + return relevant_modules + + +def get_modules(): + ''' + Here we have a look at all the modules in the + clickreviews package and filter out a few which + are not relevant. + + Basically we look at all the ones which are + derived from cr_common, where we can later on + instantiate a *Review* object and run the + necessary checks. + ''' + + all_modules = [name for _, name, _ in + pkgutil.iter_modules(clickreviews.__path__)] + return narrow_down_modules(all_modules) + + +def find_main_class(module_name): + ''' + This function will find the Click*Review class in + the specified module. + ''' + module = imp.load_source(module_name, + '%s/%s.py' % (clickreviews.__path__[0], + module_name)) + + classes = inspect.getmembers(module, inspect.isclass) + find_cr_class = lambda a: a[0].startswith('Click') and \ + not a[0].endswith('Exception') and \ + a[1].__module__ == module_name + cr_class = list(filter(find_cr_class, classes)) + if not cr_class: + return None + init_object = getattr(module, cr_class[0][0]) + return init_object + + +def init_main_class(module_name, click_file): + ''' + This function will instantiate the main Click*Review + class of a given module and instantiate it with the + location of the .click file we want to inspect. + ''' + + init_object = find_main_class(module_name) + if not init_object: + return None + return init_object(click_file) diff -Nru click-reviewers-tools-0.9/bin/clickreviews/tests/test_modules.py click-reviewers-tools-0.10/bin/clickreviews/tests/test_modules.py --- click-reviewers-tools-0.9/bin/clickreviews/tests/test_modules.py 1970-01-01 00:00:00.000000000 +0000 +++ click-reviewers-tools-0.10/bin/clickreviews/tests/test_modules.py 2014-09-05 13:02:08.000000000 +0000 @@ -0,0 +1,31 @@ +from clickreviews import modules, cr_tests +import clickreviews +import glob + + +class TestModules(cr_tests.TestClickReview): + '''Tests for the modules module.''' + def setUp(self): + self.cr_modules = modules.get_modules() + cr_tests.mock_patch() + super() + + def test_number_of_suitable_modules(self): + path = clickreviews.__path__[0] + module_files = glob.glob(path + '/*.py') + relevant_module_files = modules.narrow_down_modules(module_files) + self.assertEqual(len(relevant_module_files), + len(self.cr_modules)) + + def test_number_of_suitable_modules_greater0(self): + self.assertTrue(len(self.cr_modules) > 0) + + def test_number_of_available_review_classes(self): + count = 0 + for module_name in self.cr_modules: + review = modules.find_main_class(module_name) + if review: + count += 1 + self.assertEqual(count, len(self.cr_modules), + 'Not all files in clickreviews/cr_*.py contain ' + 'classes named Click*Review.') diff -Nru click-reviewers-tools-0.9/clickreviews/cr_desktop.py click-reviewers-tools-0.10/clickreviews/cr_desktop.py --- click-reviewers-tools-0.9/clickreviews/cr_desktop.py 2014-08-25 07:46:21.000000000 +0000 +++ click-reviewers-tools-0.10/clickreviews/cr_desktop.py 2014-09-05 13:02:08.000000000 +0000 @@ -39,10 +39,13 @@ if 'scope' in self.manifest['hooks'][app]: # msg("Skipped missing desktop hook for scope '%s'" % app) continue - if 'push-helper' in self.manifest['hooks'][app]: + elif 'push-helper' in self.manifest['hooks'][app]: # msg("Skipped missing desktop hook for push-helper '%s'" % # app) continue + elif 'pay-ui' in self.manifest['hooks'][app]: + # msg("Skipped missing desktop hook for pay-ui '%s'" % app) + continue else: error("could not find desktop hook for '%s'" % app) if not isinstance(self.manifest['hooks'][app]['desktop'], str): diff -Nru click-reviewers-tools-0.9/clickreviews/cr_security.py click-reviewers-tools-0.10/clickreviews/cr_security.py --- click-reviewers-tools-0.9/clickreviews/cr_security.py 2014-08-25 07:46:21.000000000 +0000 +++ click-reviewers-tools-0.10/clickreviews/cr_security.py 2014-09-05 13:02:08.000000000 +0000 @@ -60,6 +60,7 @@ 'template_variables', 'write_path'] self.allowed_webapp_policy_groups = ['audio', + # 'camera', non-functional ATM 'content_exchange', 'location', 'networking', diff -Nru click-reviewers-tools-0.9/clickreviews/modules.py click-reviewers-tools-0.10/clickreviews/modules.py --- click-reviewers-tools-0.9/clickreviews/modules.py 1970-01-01 00:00:00.000000000 +0000 +++ click-reviewers-tools-0.10/clickreviews/modules.py 2014-09-05 13:02:08.000000000 +0000 @@ -0,0 +1,71 @@ +import clickreviews +import imp +import inspect +import os +import pkgutil + +IRRELEVANT_MODULES = ['cr_common', 'cr_tests', 'cr_skeleton'] + + +def narrow_down_modules(modules): + ''' + Get a list of file names or module names and filter out + the ones we know are irrelevant. + ''' + relevant_modules = [] + for module in modules: + module_name = os.path.basename(module).replace('.py', '') + if module_name not in IRRELEVANT_MODULES and \ + module_name.startswith('cr_'): + relevant_modules += [module] + return relevant_modules + + +def get_modules(): + ''' + Here we have a look at all the modules in the + clickreviews package and filter out a few which + are not relevant. + + Basically we look at all the ones which are + derived from cr_common, where we can later on + instantiate a *Review* object and run the + necessary checks. + ''' + + all_modules = [name for _, name, _ in + pkgutil.iter_modules(clickreviews.__path__)] + return narrow_down_modules(all_modules) + + +def find_main_class(module_name): + ''' + This function will find the Click*Review class in + the specified module. + ''' + module = imp.load_source(module_name, + '%s/%s.py' % (clickreviews.__path__[0], + module_name)) + + classes = inspect.getmembers(module, inspect.isclass) + find_cr_class = lambda a: a[0].startswith('Click') and \ + not a[0].endswith('Exception') and \ + a[1].__module__ == module_name + cr_class = list(filter(find_cr_class, classes)) + if not cr_class: + return None + init_object = getattr(module, cr_class[0][0]) + return init_object + + +def init_main_class(module_name, click_file): + ''' + This function will instantiate the main Click*Review + class of a given module and instantiate it with the + location of the .click file we want to inspect. + ''' + + init_object = find_main_class(module_name) + if not init_object: + return None + return init_object(click_file) diff -Nru click-reviewers-tools-0.9/clickreviews/tests/test_modules.py click-reviewers-tools-0.10/clickreviews/tests/test_modules.py --- click-reviewers-tools-0.9/clickreviews/tests/test_modules.py 1970-01-01 00:00:00.000000000 +0000 +++ click-reviewers-tools-0.10/clickreviews/tests/test_modules.py 2014-09-05 13:02:08.000000000 +0000 @@ -0,0 +1,31 @@ +from clickreviews import modules, cr_tests +import clickreviews +import glob + + +class TestModules(cr_tests.TestClickReview): + '''Tests for the modules module.''' + def setUp(self): + self.cr_modules = modules.get_modules() + cr_tests.mock_patch() + super() + + def test_number_of_suitable_modules(self): + path = clickreviews.__path__[0] + module_files = glob.glob(path + '/*.py') + relevant_module_files = modules.narrow_down_modules(module_files) + self.assertEqual(len(relevant_module_files), + len(self.cr_modules)) + + def test_number_of_suitable_modules_greater0(self): + self.assertTrue(len(self.cr_modules) > 0) + + def test_number_of_available_review_classes(self): + count = 0 + for module_name in self.cr_modules: + review = modules.find_main_class(module_name) + if review: + count += 1 + self.assertEqual(count, len(self.cr_modules), + 'Not all files in clickreviews/cr_*.py contain ' + 'classes named Click*Review.') diff -Nru click-reviewers-tools-0.9/debian/bzr-builder.manifest click-reviewers-tools-0.10/debian/bzr-builder.manifest --- click-reviewers-tools-0.9/debian/bzr-builder.manifest 2014-08-25 07:46:21.000000000 +0000 +++ click-reviewers-tools-0.10/debian/bzr-builder.manifest 2014-09-05 13:02:09.000000000 +0000 @@ -1,2 +1,2 @@ -# bzr-builder format 0.3 deb-version {debupstream}-0~229 -lp:click-reviewers-tools revid:daniel.holbach@canonical.com-20140825074305-232ghg70ya03pfci +# bzr-builder format 0.3 deb-version {debupstream}-0~238 +lp:click-reviewers-tools revid:jamie@ubuntu.com-20140904154604-gv34v50h0751a3nx diff -Nru click-reviewers-tools-0.9/debian/changelog click-reviewers-tools-0.10/debian/changelog --- click-reviewers-tools-0.9/debian/changelog 2014-08-25 07:46:21.000000000 +0000 +++ click-reviewers-tools-0.10/debian/changelog 2014-09-05 13:02:09.000000000 +0000 @@ -1,10 +1,29 @@ -click-reviewers-tools (0.9-0~229~ubuntu14.04.1) trusty; urgency=low +click-reviewers-tools (0.10-0~238~ubuntu14.04.1) trusty; urgency=low * Auto build. - -- Daniel Holbach Mon, 25 Aug 2014 07:46:21 +0000 + -- Daniel Holbach Fri, 05 Sep 2014 13:02:09 +0000 -click-reviewers-tools (0.9) UNRELEASED; urgency=medium +click-reviewers-tools (0.10) UNRELEASED; urgency=medium + + [ Daniel Holbach ] + * Split out code to find Click*Review classes in the clickreviews package + into its own module, add tests for it. + * Refactor bin/click-review to make it easier to extend. + * Add --sdk option, so the SDK can start using it. (LP: #1363857) + * Safeguard against broken clickreviews check modules, or modules that are + still in development. (LP: #1364449) + + [ Jamie Strandboge ] + * There is now a special pay-ui hook instead of the payui app reusing the + desktop hook. We added a check for manual review for when the 'pay-ui' + hook was implemented in previous commits, but now we should adjust the + cr_desktop.py hook to not error when the pay-ui hook is specified but + the desktop hook is not. + + -- Daniel Holbach Tue, 02 Sep 2014 18:31:50 +0200 + +click-reviewers-tools (0.9) utopic; urgency=medium [ Jamie Strandboge ] * data/frameworks.json: add ubuntu-sdk-14.10-qml-dev3 @@ -19,7 +38,8 @@ * Point to the correct scope ini path. [ Daniel Holbach ] - * Add 'click-review', which just prints errors/warnings. (LP: #1355215) + * Add 'click-review', a more versatile approach to what 'click-run-checks' + was doing. (LP: #1355215) * Run pep8 during the build. -- Daniel Holbach Wed, 20 Aug 2014 16:03:35 +0200