From be3fbdf94ee7925dcb362de75fe105d82a7e637e Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Sat, 15 Jul 2017 12:31:29 -0700 Subject: [PATCH] Upgrade add-trailing-comma to 0.4.0 --- .pre-commit-config.yaml | 2 +- CONTRIBUTING.md | 23 ---- pre_commit/languages/ruby.py | 12 +- testing/fixtures.py | 16 +-- tests/clientlib_test.py | 156 +++++++++++++------------- tests/color_test.py | 10 +- tests/commands/run_test.py | 104 ++++++++++------- tests/prefixed_command_runner_test.py | 16 +-- tests/runner_test.py | 76 +++++++------ 9 files changed, 216 insertions(+), 199 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 46a47341..4bca47fe 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -22,6 +22,6 @@ - id: reorder-python-imports language_version: python2.7 - repo: https://github.com/asottile/add-trailing-comma - sha: v0.3.0 + sha: v0.4.0 hooks: - id: add-trailing-comma diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1e0b2460..ae4511f7 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -46,29 +46,6 @@ variable `slowtests=false`. With the environment activated simply run `pre-commit install`. -## Style - -This repository follows pep8 (and enforces it with flake8). There are a few -nitpicky things I also like that I'll outline below. - -### Multi-line method invocation - -Multiple line method invocation should look as follows - -```python -function_call( - argument, - argument, - argument, -) -``` - -Some notable features: -- The initial parenthesis is at the end of the line -- Parameters are indented one indentation level further than the function name -- The last parameter contains a trailing comma (This helps make `git blame` - more accurate and reduces merge conflicts when adding / removing parameters). - ## Documentation Documentation is hosted at http://pre-commit.com diff --git a/pre_commit/languages/ruby.py b/pre_commit/languages/ruby.py index 6a0bde27..41c13a87 100644 --- a/pre_commit/languages/ruby.py +++ b/pre_commit/languages/ruby.py @@ -24,11 +24,13 @@ def get_env_patch(venv, language_version): # pragma: windows no cover ('GEM_HOME', os.path.join(venv, 'gems')), ('RBENV_ROOT', venv), ('BUNDLE_IGNORE_CONFIG', '1'), - ('PATH', ( - os.path.join(venv, 'gems', 'bin'), os.pathsep, - os.path.join(venv, 'shims'), os.pathsep, - os.path.join(venv, 'bin'), os.pathsep, Var('PATH'), - )), + ( + 'PATH', ( + os.path.join(venv, 'gems', 'bin'), os.pathsep, + os.path.join(venv, 'shims'), os.pathsep, + os.path.join(venv, 'bin'), os.pathsep, Var('PATH'), + ), + ), ) if language_version != 'default': patches += (('RBENV_VERSION', language_version),) diff --git a/testing/fixtures.py b/testing/fixtures.py index 4a3f1446..be434216 100644 --- a/testing/fixtures.py +++ b/testing/fixtures.py @@ -69,13 +69,15 @@ def modify_config(path='.', commit=True): def config_with_local_hooks(): return OrderedDict(( ('repo', 'local'), - ('hooks', [OrderedDict(( - ('id', 'do_not_commit'), - ('name', 'Block if "DO NOT COMMIT" is found'), - ('entry', 'DO NOT COMMIT'), - ('language', 'pcre'), - ('files', '^(.*)$'), - ))]), + ( + 'hooks', [OrderedDict(( + ('id', 'do_not_commit'), + ('name', 'Block if "DO NOT COMMIT" is found'), + ('entry', 'DO NOT COMMIT'), + ('language', 'pcre'), + ('files', '^(.*)$'), + ))], + ), )) diff --git a/tests/clientlib_test.py b/tests/clientlib_test.py index 9e66025d..1fe1d80b 100644 --- a/tests/clientlib_test.py +++ b/tests/clientlib_test.py @@ -56,94 +56,100 @@ def test_validate_config_main(args, expected_output): assert validate_config_main(args) == expected_output -@pytest.mark.parametrize(('config_obj', 'expected'), ( - ([], False), - ( - [{ - 'repo': 'git@github.com:pre-commit/pre-commit-hooks', - 'sha': 'cd74dc150c142c3be70b24eaf0b02cae9d235f37', - 'hooks': [{'id': 'pyflakes', 'files': '\\.py$'}], - }], - True, +@pytest.mark.parametrize( + ('config_obj', 'expected'), ( + ([], False), + ( + [{ + 'repo': 'git@github.com:pre-commit/pre-commit-hooks', + 'sha': 'cd74dc150c142c3be70b24eaf0b02cae9d235f37', + 'hooks': [{'id': 'pyflakes', 'files': '\\.py$'}], + }], + True, + ), + ( + [{ + 'repo': 'git@github.com:pre-commit/pre-commit-hooks', + 'sha': 'cd74dc150c142c3be70b24eaf0b02cae9d235f37', + 'hooks': [ + { + 'id': 'pyflakes', + 'files': '\\.py$', + 'args': ['foo', 'bar', 'baz'], + }, + ], + }], + True, + ), + ( + [{ + 'repo': 'git@github.com:pre-commit/pre-commit-hooks', + 'sha': 'cd74dc150c142c3be70b24eaf0b02cae9d235f37', + 'hooks': [ + { + 'id': 'pyflakes', + 'files': '\\.py$', + # Exclude pattern must be a string + 'exclude': 0, + 'args': ['foo', 'bar', 'baz'], + }, + ], + }], + False, + ), ), - ( - [{ - 'repo': 'git@github.com:pre-commit/pre-commit-hooks', - 'sha': 'cd74dc150c142c3be70b24eaf0b02cae9d235f37', - 'hooks': [ - { - 'id': 'pyflakes', - 'files': '\\.py$', - 'args': ['foo', 'bar', 'baz'], - }, - ], - }], - True, - ), - ( - [{ - 'repo': 'git@github.com:pre-commit/pre-commit-hooks', - 'sha': 'cd74dc150c142c3be70b24eaf0b02cae9d235f37', - 'hooks': [ - { - 'id': 'pyflakes', - 'files': '\\.py$', - # Exclude pattern must be a string - 'exclude': 0, - 'args': ['foo', 'bar', 'baz'], - }, - ], - }], - False, - ), -)) +) def test_config_valid(config_obj, expected): ret = is_valid_according_to_schema(config_obj, CONFIG_SCHEMA) assert ret is expected -@pytest.mark.parametrize('config_obj', ( - [{ - 'repo': 'local', - 'sha': 'foo', - 'hooks': [{ - 'id': 'do_not_commit', - 'name': 'Block if "DO NOT COMMIT" is found', - 'entry': 'DO NOT COMMIT', - 'language': 'pcre', - 'files': '^(.*)$', +@pytest.mark.parametrize( + 'config_obj', ( + [{ + 'repo': 'local', + 'sha': 'foo', + 'hooks': [{ + 'id': 'do_not_commit', + 'name': 'Block if "DO NOT COMMIT" is found', + 'entry': 'DO NOT COMMIT', + 'language': 'pcre', + 'files': '^(.*)$', + }], }], - }], -)) + ), +) def test_config_with_local_hooks_definition_fails(config_obj): with pytest.raises(schema.ValidationError): schema.validate(config_obj, CONFIG_SCHEMA) -@pytest.mark.parametrize('config_obj', ( - [{ - 'repo': 'local', - 'hooks': [{ - 'id': 'arg-per-line', - 'name': 'Args per line hook', - 'entry': 'bin/hook.sh', - 'language': 'script', - 'files': '', - 'args': ['hello', 'world'], +@pytest.mark.parametrize( + 'config_obj', ( + [{ + 'repo': 'local', + 'hooks': [{ + 'id': 'arg-per-line', + 'name': 'Args per line hook', + 'entry': 'bin/hook.sh', + 'language': 'script', + 'files': '', + 'args': ['hello', 'world'], + }], }], - }], - [{ - 'repo': 'local', - 'hooks': [{ - 'id': 'arg-per-line', - 'name': 'Args per line hook', - 'entry': 'bin/hook.sh', - 'language': 'script', - 'files': '', - 'args': ['hello', 'world'], - }] - }], -)) + [{ + 'repo': 'local', + 'hooks': [{ + 'id': 'arg-per-line', + 'name': 'Args per line hook', + 'entry': 'bin/hook.sh', + 'language': 'script', + 'files': '', + 'args': ['hello', 'world'], + }] + }], + ), +) def test_config_with_local_hooks_definition_passes(config_obj): schema.validate(config_obj, CONFIG_SCHEMA) diff --git a/tests/color_test.py b/tests/color_test.py index 4fb7676a..0b8a4d69 100644 --- a/tests/color_test.py +++ b/tests/color_test.py @@ -11,10 +11,12 @@ from pre_commit.color import InvalidColorSetting from pre_commit.color import use_color -@pytest.mark.parametrize(('in_text', 'in_color', 'in_use_color', 'expected'), ( - ('foo', GREEN, True, '{}foo\033[0m'.format(GREEN)), - ('foo', GREEN, False, 'foo'), -)) +@pytest.mark.parametrize( + ('in_text', 'in_color', 'in_use_color', 'expected'), ( + ('foo', GREEN, True, '{}foo\033[0m'.format(GREEN)), + ('foo', GREEN, False, 'foo'), + ), +) def test_format_color(in_text, in_color, in_use_color, expected): ret = format_color(in_text, in_color, in_use_color) assert ret == expected diff --git a/tests/commands/run_test.py b/tests/commands/run_test.py index 01164a63..59f2ec9a 100644 --- a/tests/commands/run_test.py +++ b/tests/commands/run_test.py @@ -86,8 +86,10 @@ def _do_run(cap_out, repo, args, environ={}, config_file=C.CONFIG_FILE): return ret, printed -def _test_run(cap_out, repo, opts, expected_outputs, expected_ret, stage, - config_file=C.CONFIG_FILE): +def _test_run( + cap_out, repo, opts, expected_outputs, expected_ret, stage, + config_file=C.CONFIG_FILE, +): if stage: stage_a_file() args = _get_opts(**opts) @@ -571,18 +573,24 @@ def test_lots_of_files(mock_out_store_directory, tempdir_factory): @pytest.mark.parametrize( - ('hook_stage', 'stage_for_first_hook', 'stage_for_second_hook', - 'expected_output'), + ( + 'hook_stage', 'stage_for_first_hook', 'stage_for_second_hook', + 'expected_output', + ), ( ('push', ['commit'], ['commit'], [b'', b'']), - ('push', ['commit', 'push'], ['commit', 'push'], - [b'hook 1', b'hook 2']), + ( + 'push', ['commit', 'push'], ['commit', 'push'], + [b'hook 1', b'hook 2'], + ), ('push', [], [], [b'hook 1', b'hook 2']), ('push', [], ['commit'], [b'hook 1', b'']), ('push', ['push'], ['commit'], [b'hook 1', b'']), ('push', ['commit'], ['push'], [b'', b'hook 2']), - ('commit', ['commit', 'push'], ['commit', 'push'], - [b'hook 1', b'hook 2']), + ( + 'commit', ['commit', 'push'], ['commit', 'push'], + [b'hook 1', b'hook 2'], + ), ('commit', ['commit'], ['commit'], [b'hook 1', b'hook 2']), ('commit', [], [], [b'hook 1', b'hook 2']), ('commit', [], ['commit'], [b'hook 1', b'hook 2']), @@ -600,21 +608,25 @@ def test_local_hook_for_stages( ): config = OrderedDict(( ('repo', 'local'), - ('hooks', (OrderedDict(( - ('id', 'flake8'), - ('name', 'hook 1'), - ('entry', 'python -m flake8.__main__'), - ('language', 'system'), - ('files', r'\.py$'), - ('stages', stage_for_first_hook), - )), OrderedDict(( - ('id', 'do_not_commit'), - ('name', 'hook 2'), - ('entry', 'DO NOT COMMIT'), - ('language', 'pcre'), - ('files', '^(.*)$'), - ('stages', stage_for_second_hook), - )))), + ( + 'hooks', ( + OrderedDict(( + ('id', 'flake8'), + ('name', 'hook 1'), + ('entry', 'python -m flake8.__main__'), + ('language', 'system'), + ('files', r'\.py$'), + ('stages', stage_for_first_hook), + )), OrderedDict(( + ('id', 'do_not_commit'), + ('name', 'hook 2'), + ('entry', 'DO NOT COMMIT'), + ('language', 'pcre'), + ('files', '^(.*)$'), + ('stages', stage_for_second_hook), + )), + ), + ), )) add_config_to_repo(repo_with_passing_hook, config) @@ -637,19 +649,23 @@ def test_local_hook_passes( ): config = OrderedDict(( ('repo', 'local'), - ('hooks', (OrderedDict(( - ('id', 'flake8'), - ('name', 'flake8'), - ('entry', 'python -m flake8.__main__'), - ('language', 'system'), - ('files', r'\.py$'), - )), OrderedDict(( - ('id', 'do_not_commit'), - ('name', 'Block if "DO NOT COMMIT" is found'), - ('entry', 'DO NOT COMMIT'), - ('language', 'pcre'), - ('files', '^(.*)$'), - )))), + ( + 'hooks', ( + OrderedDict(( + ('id', 'flake8'), + ('name', 'flake8'), + ('entry', 'python -m flake8.__main__'), + ('language', 'system'), + ('files', r'\.py$'), + )), OrderedDict(( + ('id', 'do_not_commit'), + ('name', 'Block if "DO NOT COMMIT" is found'), + ('entry', 'DO NOT COMMIT'), + ('language', 'pcre'), + ('files', '^(.*)$'), + )), + ), + ), )) add_config_to_repo(repo_with_passing_hook, config) @@ -672,13 +688,15 @@ def test_local_hook_fails( ): config = OrderedDict(( ('repo', 'local'), - ('hooks', [OrderedDict(( - ('id', 'no-todo'), - ('name', 'No TODO'), - ('entry', 'sh -c "! grep -iI todo $@" --'), - ('language', 'system'), - ('files', ''), - ))]), + ( + 'hooks', [OrderedDict(( + ('id', 'no-todo'), + ('name', 'No TODO'), + ('entry', 'sh -c "! grep -iI todo $@" --'), + ('language', 'system'), + ('files', ''), + ))], + ), )) add_config_to_repo(repo_with_passing_hook, config) diff --git a/tests/prefixed_command_runner_test.py b/tests/prefixed_command_runner_test.py index 41b436c1..c928dc8a 100644 --- a/tests/prefixed_command_runner_test.py +++ b/tests/prefixed_command_runner_test.py @@ -54,13 +54,15 @@ def makedirs_mock(): return mock.Mock(spec=os.makedirs) -@pytest.mark.parametrize(('input', 'expected_prefix'), ( - norm_slash(('.', './')), - norm_slash(('foo', 'foo/')), - norm_slash(('bar/', 'bar/')), - norm_slash(('foo/bar', 'foo/bar/')), - norm_slash(('foo/bar/', 'foo/bar/')), -)) +@pytest.mark.parametrize( + ('input', 'expected_prefix'), ( + norm_slash(('.', './')), + norm_slash(('foo', 'foo/')), + norm_slash(('bar/', 'bar/')), + norm_slash(('foo/bar', 'foo/bar/')), + norm_slash(('foo/bar/', 'foo/bar/')), + ), +) def test_init_normalizes_path_endings(input, expected_prefix): input = input.replace('/', os.sep) expected_prefix = expected_prefix.replace('/', os.sep) diff --git a/tests/runner_test.py b/tests/runner_test.py index eb1f48ef..0201156c 100644 --- a/tests/runner_test.py +++ b/tests/runner_test.py @@ -57,20 +57,24 @@ def test_repositories(tempdir_factory, mock_out_store_directory): def test_local_hooks(tempdir_factory, mock_out_store_directory): config = OrderedDict(( ('repo', 'local'), - ('hooks', (OrderedDict(( - ('id', 'arg-per-line'), - ('name', 'Args per line hook'), - ('entry', 'bin/hook.sh'), - ('language', 'script'), - ('files', ''), - ('args', ['hello', 'world']), - )), OrderedDict(( - ('id', 'do_not_commit'), - ('name', 'Block if "DO NOT COMMIT" is found'), - ('entry', 'DO NOT COMMIT'), - ('language', 'pcre'), - ('files', '^(.*)$'), - )))), + ( + 'hooks', ( + OrderedDict(( + ('id', 'arg-per-line'), + ('name', 'Args per line hook'), + ('entry', 'bin/hook.sh'), + ('language', 'script'), + ('files', ''), + ('args', ['hello', 'world']), + )), OrderedDict(( + ('id', 'do_not_commit'), + ('name', 'Block if "DO NOT COMMIT" is found'), + ('entry', 'DO NOT COMMIT'), + ('language', 'pcre'), + ('files', '^(.*)$'), + )), + ), + ), )) git_path = git_dir(tempdir_factory) add_config_to_repo(git_path, config) @@ -82,26 +86,30 @@ def test_local_hooks(tempdir_factory, mock_out_store_directory): def test_local_hooks_alt_config(tempdir_factory, mock_out_store_directory): config = OrderedDict(( ('repo', 'local'), - ('hooks', (OrderedDict(( - ('id', 'arg-per-line'), - ('name', 'Args per line hook'), - ('entry', 'bin/hook.sh'), - ('language', 'script'), - ('files', ''), - ('args', ['hello', 'world']), - )), OrderedDict(( - ('id', 'ugly-format-json'), - ('name', 'Ugly format json'), - ('entry', 'ugly-format-json'), - ('language', 'python'), - ('files', ''), - )), OrderedDict(( - ('id', 'do_not_commit'), - ('name', 'Block if "DO NOT COMMIT" is found'), - ('entry', 'DO NOT COMMIT'), - ('language', 'pcre'), - ('files', '^(.*)$'), - )))), + ( + 'hooks', ( + OrderedDict(( + ('id', 'arg-per-line'), + ('name', 'Args per line hook'), + ('entry', 'bin/hook.sh'), + ('language', 'script'), + ('files', ''), + ('args', ['hello', 'world']), + )), OrderedDict(( + ('id', 'ugly-format-json'), + ('name', 'Ugly format json'), + ('entry', 'ugly-format-json'), + ('language', 'python'), + ('files', ''), + )), OrderedDict(( + ('id', 'do_not_commit'), + ('name', 'Block if "DO NOT COMMIT" is found'), + ('entry', 'DO NOT COMMIT'), + ('language', 'pcre'), + ('files', '^(.*)$'), + )), + ), + ), )) git_path = git_dir(tempdir_factory) alt_config_file = 'alternate_config.yaml'