Merge pull request #49 from pre-commit/fix_cwd_problem

Fix cwd problem

tests/conftest.py

@@ -32,37 +32,33 @@ def dummy_git_repo(empty_git_dir):
     yield empty_git_dir
 
 
+def _make_repo(repo_path, repo_source):
+    copy_tree_to_path(get_resource_path(repo_source), repo_path)
+    add_and_commit()
+    return repo_path
+
+
 @pytest.yield_fixture
 def python_hooks_repo(dummy_git_repo):
-    copy_tree_to_path(
-        get_resource_path('python_hooks_repo'),
-        dummy_git_repo,
-    )
-    add_and_commit()
-    yield dummy_git_repo
+    yield _make_repo(dummy_git_repo, 'python_hooks_repo')
 
 
 @pytest.yield_fixture
 def node_hooks_repo(dummy_git_repo):
-    copy_tree_to_path(
-        get_resource_path('node_hooks_repo'),
-        dummy_git_repo,
-    )
-    add_and_commit()
-    yield dummy_git_repo
+    yield _make_repo(dummy_git_repo, 'node_hooks_repo')
 
 
 @pytest.yield_fixture
 def consumer_repo(dummy_git_repo):
-    copy_tree_to_path(
-        get_resource_path('consumer_repo'),
-        dummy_git_repo,
-    )
-    add_and_commit()
-    yield dummy_git_repo
+    yield _make_repo(dummy_git_repo, 'consumer_repo')
 
 
-@pytest.fixture
+@pytest.yield_fixture
+def prints_cwd_repo(dummy_git_repo):
+    yield _make_repo(dummy_git_repo, 'prints_cwd_repo')
+
+
+@pytest.yield_fixture
 def config_for_node_hooks_repo(node_hooks_repo):
     config = {
         'repo': node_hooks_repo,
@@ -74,11 +70,10 @@ def config_for_node_hooks_repo(node_hooks_repo):
     }
     jsonschema.validate([config], CONFIG_JSON_SCHEMA)
     validate_config_extra([config])
-
-    return config
+    yield config
 
 
-@pytest.fixture
+@pytest.yield_fixture
 def config_for_python_hooks_repo(python_hooks_repo):
     config = {
         'repo': python_hooks_repo,
@@ -90,5 +85,19 @@ def config_for_python_hooks_repo(python_hooks_repo):
     }
     jsonschema.validate([config], CONFIG_JSON_SCHEMA)
     validate_config_extra([config])
+    yield config
 
-    return config
+
+@pytest.yield_fixture
+def config_for_prints_cwd_repo(prints_cwd_repo):
+    config = {
+        'repo': prints_cwd_repo,
+        'sha': git.get_head_sha(prints_cwd_repo),
+        'hooks': [{
+            'id': 'prints_cwd',
+            'files': '\.py$',
+        }],
+    }
+    jsonschema.validate([config], CONFIG_JSON_SCHEMA)
+    validate_config_extra([config])
+    yield config

tests/prefixed_command_runner_test.py

@@ -0,0 +1,137 @@
+
+import os
+import mock
+import pytest
+import subprocess
+from plumbum import local
+
+from pre_commit.prefixed_command_runner import _replace_cmd
+from pre_commit.prefixed_command_runner import CalledProcessError
+from pre_commit.prefixed_command_runner import PrefixedCommandRunner
+
+
+@pytest.fixture
+def popen_mock():
+    popen = mock.Mock(spec=subprocess.Popen)
+    popen.return_value.communicate.return_value = (mock.Mock(), mock.Mock())
+    return popen
+
+
+@pytest.fixture
+def makedirs_mock():
+    return mock.Mock(spec=os.makedirs)
+
+
+@pytest.mark.parametrize(('input', 'kwargs', 'expected_output'), (
+    ([], {}, []),
+    (['foo'], {}, ['foo']),
+    ([], {'foo': 'bar'}, []),
+    (['{foo}/baz'], {'foo': 'bar'}, ['bar/baz']),
+    (['foo'], {'foo': 'bar'}, ['foo']),
+    (['foo', '{bar}'], {'bar': 'baz'}, ['foo', 'baz']),
+))
+def test_replace_cmd(input, kwargs, expected_output):
+    ret = _replace_cmd(input, **kwargs)
+    assert ret == expected_output
+
+
+@pytest.mark.parametrize(('input', 'expected_prefix'), (
+    ('.', './'),
+    ('foo', 'foo/'),
+    ('bar/', 'bar/'),
+    ('foo/bar', 'foo/bar/'),
+    ('foo/bar/', 'foo/bar/'),
+))
+def test_init_normalizes_path_endings(input, expected_prefix):
+    instance = PrefixedCommandRunner(input)
+    assert instance.prefix_dir == expected_prefix
+
+
+def test_run_substitutes_prefix(popen_mock, makedirs_mock):
+    instance = PrefixedCommandRunner('prefix', popen=popen_mock, makedirs=makedirs_mock)
+    ret = instance.run(['{prefix}bar', 'baz'], retcode=None)
+    popen_mock.assert_called_once_with(
+        ['prefix/bar', 'baz'],
+        stdin=subprocess.PIPE,
+        stdout=subprocess.PIPE,
+        stderr=subprocess.PIPE,
+    )
+    assert ret == (
+        popen_mock.return_value.returncode,
+        popen_mock.return_value.communicate.return_value[0],
+        popen_mock.return_value.communicate.return_value[1],
+    )
+
+
+PATH_TESTS = (
+    ('foo', '', 'foo'),
+    ('foo', 'bar', 'foo/bar'),
+    ('foo/bar', '../baz', 'foo/baz'),
+    ('./', 'bar', 'bar'),
+    ('./', '', '.'),
+)
+
+
+@pytest.mark.parametrize(('prefix', 'path_end', 'expected_output'), PATH_TESTS)
+def test_path(prefix, path_end, expected_output):
+    instance = PrefixedCommandRunner(prefix)
+    ret = instance.path(path_end)
+    assert ret == expected_output
+
+
+def test_path_multiple_args():
+    instance = PrefixedCommandRunner('foo')
+    ret = instance.path('bar', 'baz')
+    assert ret == 'foo/bar/baz'
+
+
+@pytest.mark.parametrize(('prefix', 'path_end', 'expected_output'),
+    tuple(
+        (prefix, path_end, expected_output + os.sep)
+        for prefix, path_end, expected_output in PATH_TESTS
+    ),
+)
+def test_from_command_runner(prefix, path_end, expected_output):
+    first = PrefixedCommandRunner(prefix)
+    second = PrefixedCommandRunner.from_command_runner(first, path_end)
+    assert second.prefix_dir == expected_output
+
+
+def test_from_command_runner_preserves_popen(popen_mock, makedirs_mock):
+    first = PrefixedCommandRunner('foo', popen=popen_mock, makedirs=makedirs_mock)
+    second = PrefixedCommandRunner.from_command_runner(first, 'bar')
+    second.run(['foo/bar/baz'], retcode=None)
+    popen_mock.assert_called_once_with(
+        ['foo/bar/baz'],
+        stdin=subprocess.PIPE,
+        stdout=subprocess.PIPE,
+        stderr=subprocess.PIPE,
+    )
+
+
+def test_create_path_if_not_exists(tmpdir):
+    with local.cwd(tmpdir.strpath):
+        instance = PrefixedCommandRunner('foo')
+        assert not os.path.exists('foo')
+        instance._create_path_if_not_exists()
+        assert os.path.exists('foo')
+
+
+def test_exists_does_not_exist(tmpdir):
+    with local.cwd(tmpdir.strpath):
+        assert not PrefixedCommandRunner('.').exists('foo')
+
+
+def test_exists_does_exist(tmpdir):
+    with local.cwd(tmpdir.strpath):
+        os.mkdir('foo')
+        assert PrefixedCommandRunner('.').exists('foo')
+
+
+def test_raises_on_error(popen_mock, makedirs_mock):
+    popen_mock.return_value.returncode = 1
+    with pytest.raises(CalledProcessError):
+        instance = PrefixedCommandRunner(
+            '.', popen=popen_mock, makedirs=makedirs_mock,
+        )
+        instance.run(['foo'])

tests/repository_test.py

@@ -5,6 +5,8 @@ import pytest
 import pre_commit.constants as C
 from pre_commit import git
 from pre_commit.clientlib.validate_config import CONFIG_JSON_SCHEMA
+from pre_commit.clientlib.validate_config import validate_config_extra
+from pre_commit.prefixed_command_runner import PrefixedCommandRunner
 from pre_commit.repository import Repository
 
 
@@ -28,13 +30,13 @@ def test_create_repo_in_env(dummy_repo_config, dummy_git_repo):
     )
 
 @pytest.mark.integration
-def test_install_python_repo_in_env(python_hooks_repo, config_for_python_hooks_repo):
+def test_install_python_repo_in_env(config_for_python_hooks_repo):
     repo = Repository(config_for_python_hooks_repo)
-    repo.install()
+    repo.install(PrefixedCommandRunner(C.HOOKS_WORKSPACE))
 
     assert os.path.exists(
         os.path.join(
-            python_hooks_repo,
+            repo.repo_url,
             C.HOOKS_WORKSPACE,
             repo.sha,
             'py_env',
@@ -45,8 +47,9 @@ def test_install_python_repo_in_env(python_hooks_repo, config_for_python_hooks_r
 @pytest.mark.integration
 def test_run_a_python_hook(config_for_python_hooks_repo):
     repo = Repository(config_for_python_hooks_repo)
-    repo.install()
-    ret = repo.run_hook('foo', ['/dev/null'])
+    ret = repo.run_hook(
+        PrefixedCommandRunner(C.HOOKS_WORKSPACE), 'foo', ['/dev/null'],
+    )
 
     assert ret[0] == 0
     assert ret[1] == "['/dev/null']\nHello World\n"
@@ -55,12 +58,24 @@ def test_run_a_python_hook(config_for_python_hooks_repo):
 @pytest.mark.integration
 def test_run_a_hook_lots_of_files(config_for_python_hooks_repo):
     repo = Repository(config_for_python_hooks_repo)
-    repo.install()
-    ret = repo.run_hook('foo', ['/dev/null'] * 15000)
+    ret = repo.run_hook(
+        PrefixedCommandRunner(C.HOOKS_WORKSPACE), 'foo', ['/dev/null'] * 15000,
+    )
 
     assert ret[0] == 0
 
 
+@pytest.mark.integration
+def test_cwd_of_hook(config_for_prints_cwd_repo):
+    repo = Repository(config_for_prints_cwd_repo)
+    ret = repo.run_hook(
+        PrefixedCommandRunner(C.HOOKS_WORKSPACE), 'prints_cwd', [],
+    )
+
+    assert ret[0] == 0
+    assert ret[1] == '{0}\n'.format(repo.repo_url)
+
+
 @pytest.mark.skipif(
     os.environ.get('slowtests', None) == 'false',
     reason="TODO: make this test not super slow",
@@ -68,12 +83,12 @@ def test_run_a_hook_lots_of_files(config_for_python_hooks_repo):
 @pytest.mark.integration
 def test_run_a_node_hook(config_for_node_hooks_repo):
     repo = Repository(config_for_node_hooks_repo)
-    repo.install()
-    ret = repo.run_hook('foo', [])
+    ret = repo.run_hook(PrefixedCommandRunner(C.HOOKS_WORKSPACE), 'foo', [])
 
     assert ret[0] == 0
     assert ret[1] == 'Hello World\n'
 
+
 @pytest.fixture
 def mock_repo_config():
     config = {
@@ -81,12 +96,11 @@ def mock_repo_config():
         'sha': '5e713f8878b7d100c0e059f8cc34be4fc2e8f897',
         'hooks': [{
             'id': 'pyflakes',
-            'files': '*.py',
+            'files': '\.py$',
         }],
     }
-
     jsonschema.validate([config], CONFIG_JSON_SCHEMA)
-
+    validate_config_extra([config])
     return config
 
 

tests/runner_test.py

@@ -59,3 +59,9 @@ def test_pre_commit_path():
     runner = Runner('foo/bar')
     expected_path = os.path.join('foo/bar', '.git/hooks/pre-commit')
     assert runner.pre_commit_path == expected_path
+
+
+def test_cmd_runner():
+    runner = Runner('foo/bar')
+    ret = runner.cmd_runner
+    assert ret.prefix_dir == os.path.join('foo/bar', C.HOOKS_WORKSPACE) + '/'