From ed8cf696f9cb1679bb210b43470154086bfa03d8 Mon Sep 17 00:00:00 2001 From: Robert Raposa Date: Mon, 25 Apr 2016 14:15:23 -0400 Subject: [PATCH] Add script for checking git commit is safe --- scripts/safe-commit-linter.sh | 51 ++++++++++++++++++++++ scripts/safe_template_linter.py | 38 ++++++++-------- scripts/tests/templates/test.py | 2 + scripts/tests/test_safe_template_linter.py | 30 ++++++++++--- 4 files changed, 95 insertions(+), 26 deletions(-) create mode 100755 scripts/safe-commit-linter.sh create mode 100644 scripts/tests/templates/test.py diff --git a/scripts/safe-commit-linter.sh b/scripts/safe-commit-linter.sh new file mode 100755 index 0000000000..9e24f0f1eb --- /dev/null +++ b/scripts/safe-commit-linter.sh @@ -0,0 +1,51 @@ +#!/usr/bin/env bash +set -e + +############################################################################### +# +# safe-commit-linter.sh +# +# Executes safe_template_linter.py on the set of files in a particular git +# commit. +# +############################################################################### + +show_help() { + echo "Usage: safe-commit-linter.sh [OPTION]" + echo "Runs the Safe Template Linter against all files in a git commit." + echo "" + echo "Mandatory arguments to long options are mandatory for short options too." + echo " -m, --main-branch=COMMIT Run against files changed between the" + echo " current branch and this commit." + echo " Defaults to origin/master." +} + +for i in "$@"; do + case $i in + -m=*|--main-branch=*) + MAIN_COMMIT="${i#*=}" + shift # past argument=value + ;; + -h|--help|*) + # help or unknown option + show_help + exit 0 + ;; + esac +done + +current_branch_hash=`git rev-parse HEAD` + +if [ -z "${MAIN_COMMIT+x}" ]; then + # if commit is not set, get hash of current branch + MAIN_COMMIT="origin/master" +fi + +merge_base=`git merge-base "$current_branch_hash" "$MAIN_COMMIT"` +diff_files=`git diff --name-only "$current_branch_hash" "$merge_base"` + +for f in $diff_files; do + echo "" + echo "Linting $f:" + ./scripts/safe_template_linter.py $f +done diff --git a/scripts/safe_template_linter.py b/scripts/safe_template_linter.py index a19738db62..245eefb95d 100755 --- a/scripts/safe_template_linter.py +++ b/scripts/safe_template_linter.py @@ -613,7 +613,7 @@ class FileResults(object): Arguments: options: A list of the following options: - is_quiet: True to print only file names, and False to print + list_files: True to print only file names, and False to print all violations. out: output file @@ -623,7 +623,7 @@ class FileResults(object): """ num_violations = 0 - if options['is_quiet']: + if options['list_files']: if self.violations is not None and 0 < len(self.violations): num_violations += 1 print(self.full_path, file=out) @@ -2326,33 +2326,33 @@ def main(): parser = argparse.ArgumentParser( formatter_class=argparse.RawDescriptionHelpFormatter, description='Checks that templates are safe.', - epilog=epilog + epilog=epilog, ) parser.add_argument( - '--quiet', dest='quiet', action='store_true', help='only display the filenames that contain violations' - ) - parser.add_argument('--file', dest='file', nargs=1, default=None, help='a single file to lint') - parser.add_argument( - '--dir', dest='directory', nargs=1, default=['.'], help='the directory to lint (including sub-directories)' + '--list-files', dest='list_files', action='store_true', + help='Only display the filenames that contain violations.' ) + parser.add_argument('path', nargs="?", default=None, help='A file to lint or directory to recursively lint.') args = parser.parse_args() options = { - 'is_quiet': args.quiet, + 'list_files': args.list_files, } - template_linters = [MakoTemplateLinter(), UnderscoreTemplateLinter(), JavaScriptLinter(), PythonLinter()] - if args.file is not None: - if os.path.isfile(args.file[0]) is False: - raise ValueError("File [{}] is not a valid file.".format(args.file[0])) - num_violations = _process_file(args.file[0], template_linters, options, out=sys.stdout) - else: - if os.path.exists(args.directory[0]) is False or os.path.isfile(args.directory[0]) is True: - raise ValueError("Directory [{}] is not a valid directory.".format(args.directory[0])) - num_violations = _process_os_walk(args.directory[0], template_linters, options, out=sys.stdout) - if options['is_quiet'] is False: + if args.path is not None and os.path.isfile(args.path): + num_violations = _process_file(args.path, template_linters, options, out=sys.stdout) + else: + directory = "." + if args.path is not None: + if os.path.exists(args.path): + directory = args.path + else: + raise ValueError("Path [{}] is not a valid file or directory.".format(args.path)) + num_violations = _process_os_walk(directory, template_linters, options, out=sys.stdout) + + if options['list_files'] is False: # matches output of jshint for simplicity print("") print("{} violations found".format(num_violations)) diff --git a/scripts/tests/templates/test.py b/scripts/tests/templates/test.py new file mode 100644 index 0000000000..1e24332396 --- /dev/null +++ b/scripts/tests/templates/test.py @@ -0,0 +1,2 @@ +message = "" +x = "{}".format(message) diff --git a/scripts/tests/test_safe_template_linter.py b/scripts/tests/test_safe_template_linter.py index 01fbc6dd02..aa2fe6a3f9 100644 --- a/scripts/tests/test_safe_template_linter.py +++ b/scripts/tests/test_safe_template_linter.py @@ -82,6 +82,19 @@ class TestSafeTemplateLinter(TestCase): Test some top-level linter functions """ + def patch_is_valid_directory(self, linter_class): + """ + Creates a mock patch for _is_valid_directory on a Linter to always + return true. This avoids nested patch calls. + + Arguments: + linter_class: The linter class to be patched + """ + patcher = mock.patch.object(linter_class, '_is_valid_directory', return_value=True) + patch_start = patcher.start() + self.addCleanup(patcher.stop) + return patch_start + def test_process_os_walk(self): """ Tests the top-level processing of template files, including Mako @@ -90,24 +103,27 @@ class TestSafeTemplateLinter(TestCase): out = StringIO() options = { - 'is_quiet': False, + 'list_files': False, } - template_linters = [MakoTemplateLinter(), JavaScriptLinter(), UnderscoreTemplateLinter()] + template_linters = [MakoTemplateLinter(), JavaScriptLinter(), UnderscoreTemplateLinter(), PythonLinter()] - with mock.patch.object(MakoTemplateLinter, '_is_valid_directory', return_value=True): - with mock.patch.object(JavaScriptLinter, '_is_valid_directory', return_value=True): - with mock.patch.object(UnderscoreTemplateLinter, '_is_valid_directory', return_value=True): - num_violations = _process_os_walk('scripts/tests/templates', template_linters, options, out) + self.patch_is_valid_directory(MakoTemplateLinter) + self.patch_is_valid_directory(JavaScriptLinter) + self.patch_is_valid_directory(UnderscoreTemplateLinter) + self.patch_is_valid_directory(PythonLinter) + + num_violations = _process_os_walk('scripts/tests/templates', template_linters, options, out) output = out.getvalue() - self.assertEqual(num_violations, 6) + self.assertEqual(num_violations, 7) self.assertIsNotNone(re.search('test\.html.*mako-missing-default', output)) self.assertIsNotNone(re.search('test\.coffee.*javascript-concat-html', output)) self.assertIsNotNone(re.search('test\.coffee.*underscore-not-escaped', output)) self.assertIsNotNone(re.search('test\.js.*javascript-concat-html', output)) self.assertIsNotNone(re.search('test\.js.*underscore-not-escaped', output)) self.assertIsNotNone(re.search('test\.underscore.*underscore-not-escaped', output)) + self.assertIsNotNone(re.search('test\.py.*python-interpolate-html', output)) @ddt