Incoroporate PR feedback

* Make config_file a required argument to Runner
* Update main.py
* Update tests to make them all green

New test to test alternate config functionality coming in next commit

pre_commit/main.py

@@ -35,7 +35,10 @@ def _add_color_option(parser):
 
 
 def _add_config_option(parser):
-    parser.add_argument('-c', '--config', help='Path to alternate config file')
+    parser.add_argument(
+        '-c', '--config', default='.pre-commit-config.yaml',
+        help='Path to alternate config file'
+    )
 
 
 def main(argv=None):
@@ -43,6 +46,7 @@ def main(argv=None):
     argv = [five.to_text(arg) for arg in argv]
     parser = argparse.ArgumentParser()
 
+    parser.set_defaults(config='.pre-commit-config.yaml')
     # http://stackoverflow.com/a/8521644/812183
     parser.add_argument(
         '-V', '--version',
@@ -58,6 +62,7 @@ def main(argv=None):
         'install', help='Install the pre-commit script.',
     )
     _add_color_option(install_parser)
+    _add_config_option(install_parser)
     install_parser.add_argument(
         '-f', '--overwrite', action='store_true',
         help='Overwrite existing hooks / remove migration mode.',
@@ -78,6 +83,7 @@ def main(argv=None):
         'uninstall', help='Uninstall the pre-commit script.',
     )
     _add_color_option(uninstall_parser)
+    _add_config_option(uninstall_parser)
     uninstall_parser.add_argument(
         '-t', '--hook-type', choices=('pre-commit', 'pre-push'),
         default='pre-commit',
@@ -87,7 +93,7 @@ def main(argv=None):
         'clean', help='Clean out pre-commit files.',
     )
     _add_color_option(clean_parser)
-
+    _add_config_option(clean_parser)
     autoupdate_parser = subparsers.add_parser(
         'autoupdate',
         help="Auto-update pre-commit config to the latest repos' versions.",
@@ -97,6 +103,7 @@ def main(argv=None):
 
     run_parser = subparsers.add_parser('run', help='Run hooks.')
     _add_color_option(run_parser)
+    _add_config_option(run_parser)
     run_parser.add_argument('hook', nargs='?', help='A single hook-id to run')
     run_parser.add_argument(
         '--no-stash', default=False, action='store_true',
@@ -124,7 +131,6 @@ def main(argv=None):
         '--hook-stage', choices=('commit', 'push'), default='commit',
         help='The stage during which the hook is fired e.g. commit or push.',
     )
-    _add_config_option(run_parser)
     run_mutex_group = run_parser.add_mutually_exclusive_group(required=False)
     run_mutex_group.add_argument(
         '--all-files', '-a', action='store_true', default=False,
@@ -158,10 +164,7 @@ def main(argv=None):
 
     with error_handler():
         add_logging_handler(args.color)
-        runner_kwargs = {}
+        runner = Runner.create(args.config)
-        if hasattr(args, 'config_file'):
-            runner_kwargs['config_file'] = args.config_file
-        runner = Runner.create(**runner_kwargs)
         git.check_for_cygwin_mismatch()
 
         if args.command == 'install':

pre_commit/runner.py

@@ -4,7 +4,6 @@ import os.path
 
 from cached_property import cached_property
 
-import pre_commit.constants as C
 from pre_commit import git
 from pre_commit.clientlib.validate_config import load_config
 from pre_commit.repository import Repository
@@ -16,19 +15,19 @@ class Runner(object):
     repository under test.
     """
 
-    def __init__(self, git_root, config_file=None):
+    def __init__(self, git_root, config_file):
         self.git_root = git_root
-        self.config_file = config_file or C.CONFIG_FILE
+        self.config_file = config_file
 
     @classmethod
-    def create(cls, config_file=None):
+    def create(cls, config_file):
         """Creates a PreCommitRunner by doing the following:
             - Finds the root of the current git repository
             - chdir to that directory
         """
         root = git.get_root()
         os.chdir(root)
-        return cls(root, config_file=config_file)
+        return cls(root, config_file)
 
     @cached_property
     def git_dir(self):

tests/commands/autoupdate_test.py

@@ -45,7 +45,7 @@ def test_autoupdate_up_to_date_repo(
 
     before = open(C.CONFIG_FILE).read()
     assert '^$' not in before
-    runner = Runner('.')
+    runner = Runner('.', C.CONFIG_FILE)
     ret = autoupdate(runner)
     after = open(C.CONFIG_FILE).read()
     assert ret == 0
@@ -86,7 +86,7 @@ def test_autoupdate_out_of_date_repo(
     write_config('.', config)
 
     before = open(C.CONFIG_FILE).read()
-    runner = Runner('.')
+    runner = Runner('.', C.CONFIG_FILE)
     ret = autoupdate(runner)
     after = open(C.CONFIG_FILE).read()
     assert ret == 0
@@ -111,7 +111,7 @@ def test_autoupdate_tagged_repo(
     )
     write_config('.', config)
 
-    ret = autoupdate(Runner('.'))
+    ret = autoupdate(Runner('.', C.CONFIG_FILE))
     assert ret == 0
     assert 'v1.2.3' in open(C.CONFIG_FILE).read()
 
@@ -156,7 +156,7 @@ def test_autoupdate_hook_disappearing_repo(
     write_config('.', config)
 
     before = open(C.CONFIG_FILE).read()
-    runner = Runner('.')
+    runner = Runner('.', C.CONFIG_FILE)
     ret = autoupdate(runner)
     after = open(C.CONFIG_FILE).read()
     assert ret == 1
@@ -167,7 +167,7 @@ def test_autoupdate_local_hooks(tempdir_factory):
     git_path = git_dir(tempdir_factory)
     config = config_with_local_hooks()
     path = add_config_to_repo(git_path, config)
-    runner = Runner(path)
+    runner = Runner(path, C.CONFIG_FILE)
     assert autoupdate(runner) == 0
     new_config_writen = load_config(runner.config_file_path)
     assert len(new_config_writen) == 1
@@ -183,7 +183,7 @@ def test_autoupdate_local_hooks_with_out_of_date_repo(
     local_config = config_with_local_hooks()
     config = [local_config, stale_config]
     write_config('.', config)
-    runner = Runner('.')
+    runner = Runner('.', C.CONFIG_FILE)
     assert autoupdate(runner) == 0
     new_config_writen = load_config(runner.config_file_path)
     assert len(new_config_writen) == 2

tests/commands/install_uninstall_test.py

@@ -10,6 +10,7 @@ import sys
 
 import mock
 
+import pre_commit.constants as C
 from pre_commit.commands.install_uninstall import IDENTIFYING_HASH
 from pre_commit.commands.install_uninstall import install
 from pre_commit.commands.install_uninstall import is_our_pre_commit
@@ -53,7 +54,7 @@ def test_is_previous_pre_commit(in_tmpdir):
 
 def test_install_pre_commit(tempdir_factory):
     path = git_dir(tempdir_factory)
-    runner = Runner(path)
+    runner = Runner(path, C.CONFIG_FILE)
     ret = install(runner)
     assert ret == 0
     assert os.path.exists(runner.pre_commit_path)
@@ -87,7 +88,7 @@ def test_install_hooks_directory_not_present(tempdir_factory):
     hooks = os.path.join(path, '.git', 'hooks')
     if os.path.exists(hooks):  # pragma: no cover (latest git)
         shutil.rmtree(hooks)
-    runner = Runner(path)
+    runner = Runner(path, C.CONFIG_FILE)
     install(runner)
     assert os.path.exists(runner.pre_commit_path)
 
@@ -97,7 +98,7 @@ def test_install_hooks_dead_symlink(
         tempdir_factory,
 ):  # pragma: no cover (non-windows)
     path = git_dir(tempdir_factory)
-    runner = Runner(path)
+    runner = Runner(path, C.CONFIG_FILE)
     mkdirp(os.path.dirname(runner.pre_commit_path))
     os.symlink('/fake/baz', os.path.join(path, '.git', 'hooks', 'pre-commit'))
     install(runner)
@@ -106,14 +107,14 @@ def test_install_hooks_dead_symlink(
 
 def test_uninstall_does_not_blow_up_when_not_there(tempdir_factory):
     path = git_dir(tempdir_factory)
-    runner = Runner(path)
+    runner = Runner(path, C.CONFIG_FILE)
     ret = uninstall(runner)
     assert ret == 0
 
 
 def test_uninstall(tempdir_factory):
     path = git_dir(tempdir_factory)
-    runner = Runner(path)
+    runner = Runner(path, C.CONFIG_FILE)
     assert not os.path.exists(runner.pre_commit_path)
     install(runner)
     assert os.path.exists(runner.pre_commit_path)
@@ -156,7 +157,7 @@ NORMAL_PRE_COMMIT_RUN = re.compile(
 def test_install_pre_commit_and_run(tempdir_factory):
     path = make_consuming_repo(tempdir_factory, 'script_hooks_repo')
     with cwd(path):
-        assert install(Runner(path)) == 0
+        assert install(Runner(path, C.CONFIG_FILE)) == 0
 
         ret, output = _get_commit_output(tempdir_factory)
         assert ret == 0
@@ -172,7 +173,7 @@ def test_install_in_submodule_and_run(tempdir_factory):
 
     sub_pth = os.path.join(parent_path, 'sub')
     with cwd(sub_pth):
-        assert install(Runner(sub_pth)) == 0
+        assert install(Runner(sub_pth, C.CONFIG_FILE)) == 0
         ret, output = _get_commit_output(tempdir_factory)
         assert ret == 0
         assert NORMAL_PRE_COMMIT_RUN.match(output)
@@ -189,7 +190,7 @@ def test_commit_am(tempdir_factory):
         with io.open('unstaged', 'w') as foo_file:
             foo_file.write('Oh hai')
 
-        assert install(Runner(path)) == 0
+        assert install(Runner(path, C.CONFIG_FILE)) == 0
 
         ret, output = _get_commit_output(tempdir_factory)
         assert ret == 0
@@ -198,8 +199,8 @@ def test_commit_am(tempdir_factory):
 def test_install_idempotent(tempdir_factory):
     path = make_consuming_repo(tempdir_factory, 'script_hooks_repo')
     with cwd(path):
-        assert install(Runner(path)) == 0
+        assert install(Runner(path, C.CONFIG_FILE)) == 0
-        assert install(Runner(path)) == 0
+        assert install(Runner(path, C.CONFIG_FILE)) == 0
 
         ret, output = _get_commit_output(tempdir_factory)
         assert ret == 0
@@ -219,7 +220,7 @@ def test_environment_not_sourced(tempdir_factory):
     with cwd(path):
         # Patch the executable to simulate rming virtualenv
         with mock.patch.object(sys, 'executable', '/bin/false'):
-            assert install(Runner(path)) == 0
+            assert install(Runner(path, C.CONFIG_FILE)) == 0
 
         # Use a specific homedir to ignore --user installs
         homedir = tempdir_factory.get()
@@ -257,7 +258,7 @@ FAILING_PRE_COMMIT_RUN = re.compile(
 def test_failing_hooks_returns_nonzero(tempdir_factory):
     path = make_consuming_repo(tempdir_factory, 'failing_hook_repo')
     with cwd(path):
-        assert install(Runner(path)) == 0
+        assert install(Runner(path, C.CONFIG_FILE)) == 0
 
         ret, output = _get_commit_output(tempdir_factory)
         assert ret == 1
@@ -275,7 +276,7 @@ EXISTING_COMMIT_RUN = re.compile(
 def test_install_existing_hooks_no_overwrite(tempdir_factory):
     path = make_consuming_repo(tempdir_factory, 'script_hooks_repo')
     with cwd(path):
-        runner = Runner(path)
+        runner = Runner(path, C.CONFIG_FILE)
 
         # Write out an "old" hook
         mkdirp(os.path.dirname(runner.pre_commit_path))
@@ -301,7 +302,7 @@ def test_install_existing_hooks_no_overwrite(tempdir_factory):
 def test_install_existing_hook_no_overwrite_idempotent(tempdir_factory):
     path = make_consuming_repo(tempdir_factory, 'script_hooks_repo')
     with cwd(path):
-        runner = Runner(path)
+        runner = Runner(path, C.CONFIG_FILE)
 
         # Write out an "old" hook
         mkdirp(os.path.dirname(runner.pre_commit_path))
@@ -330,7 +331,7 @@ FAIL_OLD_HOOK = re.compile(
 def test_failing_existing_hook_returns_1(tempdir_factory):
     path = make_consuming_repo(tempdir_factory, 'script_hooks_repo')
     with cwd(path):
-        runner = Runner(path)
+        runner = Runner(path, C.CONFIG_FILE)
 
         # Write out a failing "old" hook
         mkdirp(os.path.dirname(runner.pre_commit_path))
@@ -349,7 +350,7 @@ def test_failing_existing_hook_returns_1(tempdir_factory):
 def test_install_overwrite_no_existing_hooks(tempdir_factory):
     path = make_consuming_repo(tempdir_factory, 'script_hooks_repo')
     with cwd(path):
-        assert install(Runner(path), overwrite=True) == 0
+        assert install(Runner(path, C.CONFIG_FILE), overwrite=True) == 0
 
         ret, output = _get_commit_output(tempdir_factory)
         assert ret == 0
@@ -359,7 +360,7 @@ def test_install_overwrite_no_existing_hooks(tempdir_factory):
 def test_install_overwrite(tempdir_factory):
     path = make_consuming_repo(tempdir_factory, 'script_hooks_repo')
     with cwd(path):
-        runner = Runner(path)
+        runner = Runner(path, C.CONFIG_FILE)
 
         # Write out the "old" hook
         mkdirp(os.path.dirname(runner.pre_commit_path))
@@ -377,7 +378,7 @@ def test_install_overwrite(tempdir_factory):
 def test_uninstall_restores_legacy_hooks(tempdir_factory):
     path = make_consuming_repo(tempdir_factory, 'script_hooks_repo')
     with cwd(path):
-        runner = Runner(path)
+        runner = Runner(path, C.CONFIG_FILE)
 
         # Write out an "old" hook
         mkdirp(os.path.dirname(runner.pre_commit_path))
@@ -398,7 +399,7 @@ def test_uninstall_restores_legacy_hooks(tempdir_factory):
 def test_replace_old_commit_script(tempdir_factory):
     path = make_consuming_repo(tempdir_factory, 'script_hooks_repo')
     with cwd(path):
-        runner = Runner(path)
+        runner = Runner(path, C.CONFIG_FILE)
 
         # Install a script that looks like our old script
         pre_commit_contents = io.open(
@@ -424,7 +425,7 @@ def test_replace_old_commit_script(tempdir_factory):
 def test_uninstall_doesnt_remove_not_our_hooks(tempdir_factory):
     path = git_dir(tempdir_factory)
     with cwd(path):
-        runner = Runner(path)
+        runner = Runner(path, C.CONFIG_FILE)
         mkdirp(os.path.dirname(runner.pre_commit_path))
         with io.open(runner.pre_commit_path, 'w') as pre_commit_file:
             pre_commit_file.write('#!/usr/bin/env bash\necho 1\n')
@@ -449,7 +450,7 @@ def test_installs_hooks_with_hooks_True(
 ):
     path = make_consuming_repo(tempdir_factory, 'script_hooks_repo')
     with cwd(path):
-        install(Runner(path), hooks=True)
+        install(Runner(path, C.CONFIG_FILE), hooks=True)
         ret, output = _get_commit_output(
             tempdir_factory, pre_commit_home=mock_out_store_directory,
         )
@@ -461,7 +462,7 @@ def test_installs_hooks_with_hooks_True(
 def test_installed_from_venv(tempdir_factory):
     path = make_consuming_repo(tempdir_factory, 'script_hooks_repo')
     with cwd(path):
-        install(Runner(path))
+        install(Runner(path, C.CONFIG_FILE))
         # No environment so pre-commit is not on the path when running!
         # Should still pick up the python from when we installed
         ret, output = _get_commit_output(
@@ -495,7 +496,7 @@ def test_pre_push_integration_failing(tempdir_factory):
     path = tempdir_factory.get()
     cmd_output('git', 'clone', upstream, path)
     with cwd(path):
-        install(Runner(path), hook_type='pre-push')
+        install(Runner(path, C.CONFIG_FILE), hook_type='pre-push')
         # commit succeeds because pre-commit is only installed for pre-push
         assert _get_commit_output(tempdir_factory)[0] == 0
 
@@ -511,7 +512,7 @@ def test_pre_push_integration_accepted(tempdir_factory):
     path = tempdir_factory.get()
     cmd_output('git', 'clone', upstream, path)
     with cwd(path):
-        install(Runner(path), hook_type='pre-push')
+        install(Runner(path, C.CONFIG_FILE), hook_type='pre-push')
         assert _get_commit_output(tempdir_factory)[0] == 0
 
         retc, output = _get_push_output(tempdir_factory)
@@ -525,7 +526,7 @@ def test_pre_push_integration_empty_push(tempdir_factory):
     path = tempdir_factory.get()
     cmd_output('git', 'clone', upstream, path)
     with cwd(path):
-        install(Runner(path), hook_type='pre-push')
+        install(Runner(path, C.CONFIG_FILE), hook_type='pre-push')
         _get_push_output(tempdir_factory)
         retc, output = _get_push_output(tempdir_factory)
         assert output == 'Everything up-to-date\n'

tests/commands/run_test.py

@@ -75,7 +75,7 @@ def _get_opts(
 
 
 def _do_run(cap_out, repo, args, environ={}):
-    runner = Runner(repo)
+    runner = Runner(repo, C.CONFIG_FILE)
     with cwd(runner.git_root):  # replicates Runner.create behaviour
         ret = run(runner, args, environ=environ)
     printed = cap_out.get_bytes()
@@ -375,7 +375,7 @@ def test_non_ascii_hook_id(
         repo_with_passing_hook, mock_out_store_directory, tempdir_factory,
 ):
     with cwd(repo_with_passing_hook):
-        install(Runner(repo_with_passing_hook))
+        install(Runner(repo_with_passing_hook, C.CONFIG_FILE))
         _, stdout, _ = cmd_output_mocked_pre_commit_home(
             sys.executable, '-m', 'pre_commit.main', 'run', '☃',
             retcode=None, tempdir_factory=tempdir_factory,
@@ -393,7 +393,7 @@ def test_stdout_write_bug_py26(
             config[0]['hooks'][0]['args'] = ['☃']
         stage_a_file()
 
-        install(Runner(repo_with_failing_hook))
+        install(Runner(repo_with_failing_hook, C.CONFIG_FILE))
 
         # Have to use subprocess because pytest monkeypatches sys.stdout
         _, stdout, _ = cmd_output_mocked_pre_commit_home(
@@ -411,7 +411,7 @@ def test_stdout_write_bug_py26(
 def test_hook_install_failure(mock_out_store_directory, tempdir_factory):
     git_path = make_consuming_repo(tempdir_factory, 'not_installable_repo')
     with cwd(git_path):
-        install(Runner(git_path))
+        install(Runner(git_path, C.CONFIG_FILE))
 
         _, stdout, _ = cmd_output_mocked_pre_commit_home(
             'git', 'commit', '-m', 'Commit!',
@@ -460,7 +460,7 @@ def test_lots_of_files(mock_out_store_directory, tempdir_factory):
             open(filename, 'w').close()
 
         cmd_output('bash', '-c', 'git add .')
-        install(Runner(git_path))
+        install(Runner(git_path, C.CONFIG_FILE))
 
         cmd_output_mocked_pre_commit_home(
             'git', 'commit', '-m', 'Commit!',
@@ -646,7 +646,7 @@ def test_files_running_subdir(
         repo_with_passing_hook, mock_out_store_directory, tempdir_factory,
 ):
     with cwd(repo_with_passing_hook):
-        install(Runner(repo_with_passing_hook))
+        install(Runner(repo_with_passing_hook, C.CONFIG_FILE))
 
         os.mkdir('subdir')
         open('subdir/foo.py', 'w').close()

tests/conftest.py

@@ -9,6 +9,7 @@ import os.path
 import mock
 import pytest
 
+import pre_commit.constants as C
 from pre_commit import five
 from pre_commit import output
 from pre_commit.logging_handler import add_logging_handler
@@ -136,7 +137,7 @@ def cmd_runner(tempdir_factory):
 
 @pytest.yield_fixture
 def runner_with_mocked_store(mock_out_store_directory):
-    yield Runner('/')
+    yield Runner('/', C.CONFIG_FILE)
 
 
 @pytest.yield_fixture

tests/runner_test.py

@@ -15,7 +15,7 @@ from testing.fixtures import make_consuming_repo
 
 def test_init_has_no_side_effects(tmpdir):
     current_wd = os.getcwd()
-    runner = Runner(tmpdir.strpath)
+    runner = Runner(tmpdir.strpath, C.CONFIG_FILE)
     assert runner.git_root == tmpdir.strpath
     assert os.getcwd() == current_wd
 
@@ -23,7 +23,7 @@ def test_init_has_no_side_effects(tmpdir):
 def test_create_sets_correct_directory(tempdir_factory):
     path = git_dir(tempdir_factory)
     with cwd(path):
-        runner = Runner.create()
+        runner = Runner.create(C.CONFIG_FILE)
         assert os.path.normcase(runner.git_root) == os.path.normcase(path)
         assert os.path.normcase(os.getcwd()) == os.path.normcase(path)
 
@@ -37,20 +37,20 @@ def test_create_changes_to_git_root(tempdir_factory):
         os.chdir(foo_path)
         assert os.getcwd() != path
 
-        runner = Runner.create()
+        runner = Runner.create(C.CONFIG_FILE)
         assert os.path.normcase(runner.git_root) == os.path.normcase(path)
         assert os.path.normcase(os.getcwd()) == os.path.normcase(path)
 
 
 def test_config_file_path():
-    runner = Runner(os.path.join('foo', 'bar'))
+    runner = Runner(os.path.join('foo', 'bar'), C.CONFIG_FILE)
     expected_path = os.path.join('foo', 'bar', C.CONFIG_FILE)
     assert runner.config_file_path == expected_path
 
 
 def test_repositories(tempdir_factory, mock_out_store_directory):
     path = make_consuming_repo(tempdir_factory, 'script_hooks_repo')
-    runner = Runner(path)
+    runner = Runner(path, C.CONFIG_FILE)
     assert len(runner.repositories) == 1
 
 
@@ -74,7 +74,7 @@ def test_local_hooks(tempdir_factory, mock_out_store_directory):
     ))
     git_path = git_dir(tempdir_factory)
     add_config_to_repo(git_path, config)
-    runner = Runner(git_path)
+    runner = Runner(git_path, C.CONFIG_FILE)
     assert len(runner.repositories) == 1
     assert len(runner.repositories[0].hooks) == 2
 
@@ -82,7 +82,7 @@ def test_local_hooks(tempdir_factory, mock_out_store_directory):
 def test_pre_commit_path(in_tmpdir):
     path = os.path.join('foo', 'bar')
     cmd_output('git', 'init', path)
-    runner = Runner(path)
+    runner = Runner(path, C.CONFIG_FILE)
     expected_path = os.path.join(path, '.git', 'hooks', 'pre-commit')
     assert runner.pre_commit_path == expected_path
 
@@ -90,12 +90,12 @@ def test_pre_commit_path(in_tmpdir):
 def test_pre_push_path(in_tmpdir):
     path = os.path.join('foo', 'bar')
     cmd_output('git', 'init', path)
-    runner = Runner(path)
+    runner = Runner(path, C.CONFIG_FILE)
     expected_path = os.path.join(path, '.git', 'hooks', 'pre-push')
     assert runner.pre_push_path == expected_path
 
 
 def test_cmd_runner(mock_out_store_directory):
-    runner = Runner(os.path.join('foo', 'bar'))
+    runner = Runner(os.path.join('foo', 'bar'), C.CONFIG_FILE)
     ret = runner.cmd_runner
     assert ret.prefix_dir == os.path.join(mock_out_store_directory) + os.sep