From afed6600770c03cca37a4c1db6e52ee741c38063 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Sat, 22 Mar 2014 14:57:22 -0700 Subject: [PATCH 1/4] Add failing test for too many files --- tests/repository_test.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/repository_test.py b/tests/repository_test.py index c0dff617..f8d73cb8 100644 --- a/tests/repository_test.py +++ b/tests/repository_test.py @@ -52,6 +52,16 @@ def test_run_a_python_hook(config_for_python_pre_commit_git_repo): assert ret[1] == 'Hello World\n' +@pytest.mark.integration +def test_run_a_hook_lots_of_files(config_for_python_pre_commit_git_repo): + repo = Repository(config_for_python_pre_commit_git_repo) + repo.install() + ret = repo.run_hook('foo', ['/dev/null'] * 15000) + + assert ret[0] == 0 + assert ret[1] == 'Hello World\n' + + @pytest.mark.skipif(True, reason="TODO: make this test not super slow") def test_run_a_node_hook(config_for_node_pre_commit_git_repo): repo = Repository(config_for_node_pre_commit_git_repo) From a1ba715b50c67951011afee38bf5a7ed719a36b5 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Sat, 22 Mar 2014 15:22:06 -0700 Subject: [PATCH 2/4] Modify a test to make sure arguments are getting passed correctly --- tests/conftest.py | 4 +++- tests/repository_test.py | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 794ad221..11063ec1 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -67,6 +67,8 @@ setup( local.path('__init__.py').write('') local.path('main.py').write(""" def func(): + import sys + print repr(sys.argv[1:]) print 'Hello World' return 0 """) @@ -142,4 +144,4 @@ def config_for_python_pre_commit_git_repo(python_pre_commit_git_repo): jsonschema.validate([config], CONFIG_JSON_SCHEMA) - return config \ No newline at end of file + return config diff --git a/tests/repository_test.py b/tests/repository_test.py index f8d73cb8..f34b4ca2 100644 --- a/tests/repository_test.py +++ b/tests/repository_test.py @@ -46,10 +46,10 @@ def test_install_python_repo_in_env(python_pre_commit_git_repo, config_for_pytho def test_run_a_python_hook(config_for_python_pre_commit_git_repo): repo = Repository(config_for_python_pre_commit_git_repo) repo.install() - ret = repo.run_hook('foo', []) + ret = repo.run_hook('foo', ['/dev/null']) assert ret[0] == 0 - assert ret[1] == 'Hello World\n' + assert ret[1] == "['/dev/null']\nHello World\n" @pytest.mark.integration From 6cda5bfe270e53b2fb5f66dd046c93ff5c3ac75e Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Sat, 22 Mar 2014 16:33:24 -0700 Subject: [PATCH 3/4] Clean up how the environments work. --- pre_commit/languages/helpers.py | 33 ++++++++++++++++++++++++++++++- pre_commit/languages/node.py | 35 +++++++++++++++++++-------------- pre_commit/languages/python.py | 10 ++++------ tests/repository_test.py | 6 +++++- 4 files changed, 61 insertions(+), 23 deletions(-) diff --git a/pre_commit/languages/helpers.py b/pre_commit/languages/helpers.py index 8397e9a8..ba83ec74 100644 --- a/pre_commit/languages/helpers.py +++ b/pre_commit/languages/helpers.py @@ -1,6 +1,37 @@ +from plumbum import local + + def run_hook(env, hook, file_args): return env.run( ' '.join([hook['entry']] + hook.get('args', []) + list(file_args)), retcode=None, - ) \ No newline at end of file + ) + return env.run( + ' '.join(['xargs |', hook['entry']] + hook.get('args', [])), + retcode=None, + stdin='\n'.join(file_args) + '\n', + ) + + +class Environment(object): + @property + def env_prefix(self): + """env_prefix is a value that is prefixed to the command that is run. + + Usually this is to source a virtualenv, etc. + + Commands basically end up looking like: + + bash -c '{env_prefix} {cmd}' + + so you'll often want to end your prefix with && + """ + raise NotImplementedError + + def run(self, cmd, **kwargs): + """Returns (returncode, stdout, stderr).""" + return local['bash'][ + '-c', + ' '.join([self.env_prefix, cmd]) + ].run(**kwargs) diff --git a/pre_commit/languages/node.py b/pre_commit/languages/node.py index f3fbd322..8b56b4b9 100644 --- a/pre_commit/languages/node.py +++ b/pre_commit/languages/node.py @@ -8,37 +8,42 @@ from pre_commit.languages import python NODE_ENV = 'node_env' -class NodeEnv(object): - def __init__(self, py_env): - self.py_env = py_env - self.env_prefix = '. {0}/bin/activate &&'.format(NODE_ENV) - - def run(self, cmd, **kwargs): - return self.py_env.run(' '.join([self.env_prefix, cmd]), **kwargs) +class NodeEnv(python.PythonEnv): + @property + def env_prefix(self): + base = super(NodeEnv, self).env_prefix + return ' '.join([base, '. {0}/bin/activate &&'.format(NODE_ENV)]) @contextlib.contextmanager -def in_env(py_env): - yield NodeEnv(py_env) +def in_env(): + yield NodeEnv() def install_environment(): assert local.path('package.json').exists() - if local.path('node_env').exists(): + if local.path(NODE_ENV).exists(): return local['virtualenv'][python.PY_ENV]() with python.in_env() as python_env: python_env.run('pip install nodeenv') - python_env.run('nodeenv --jobs 4 {0}'.format(NODE_ENV)) - with in_env(python_env) as node_env: + try: + # Try and use the system level node executable first + python_env.run('nodeenv -n system {0}'.format(NODE_ENV)) + except Exception: + # TODO: log exception here + # cleanup + local.path(NODE_ENV).remove() + python_env.run('nodeenv --jobs 4 {0}'.format(NODE_ENV)) + + with in_env() as node_env: node_env.run('npm install -g') def run_hook(hook, file_args): - with python.in_env() as py_env: - with in_env(py_env) as node_env: - return helpers.run_hook(node_env, hook, file_args) \ No newline at end of file + with in_env() as node_env: + return helpers.run_hook(node_env, hook, file_args) diff --git a/pre_commit/languages/python.py b/pre_commit/languages/python.py index 850ad660..06a4ab51 100644 --- a/pre_commit/languages/python.py +++ b/pre_commit/languages/python.py @@ -6,12 +6,10 @@ from pre_commit.languages import helpers PY_ENV = 'py_env' -class PythonEnv(object): - def __init__(self): - self.env_prefix = '. {0}/bin/activate &&'.format(PY_ENV) - - def run(self, cmd, **kwargs): - return local['bash']['-c', ' '.join([self.env_prefix, cmd])].run(**kwargs) +class PythonEnv(helpers.Environment): + @property + def env_prefix(self): + return '. {0}/bin/activate &&'.format(PY_ENV) @contextlib.contextmanager diff --git a/tests/repository_test.py b/tests/repository_test.py index f34b4ca2..0d15ea72 100644 --- a/tests/repository_test.py +++ b/tests/repository_test.py @@ -52,6 +52,7 @@ def test_run_a_python_hook(config_for_python_pre_commit_git_repo): assert ret[1] == "['/dev/null']\nHello World\n" +@pytest.mark.xfail @pytest.mark.integration def test_run_a_hook_lots_of_files(config_for_python_pre_commit_git_repo): repo = Repository(config_for_python_pre_commit_git_repo) @@ -62,7 +63,10 @@ def test_run_a_hook_lots_of_files(config_for_python_pre_commit_git_repo): assert ret[1] == 'Hello World\n' -@pytest.mark.skipif(True, reason="TODO: make this test not super slow") +@pytest.mark.skipif( + os.environ.get('slowtests', None) == 'false', + reason="TODO: make this test not super slow", +) def test_run_a_node_hook(config_for_node_pre_commit_git_repo): repo = Repository(config_for_node_pre_commit_git_repo) repo.install() From f66b3f61565a90da622b3bd99e81405015dea821 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Sat, 22 Mar 2014 16:54:50 -0700 Subject: [PATCH 4/4] Fixes lots of files problem. Closes #34 --- pre_commit/languages/helpers.py | 26 +++++++++++++------------- tests/repository_test.py | 2 -- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/pre_commit/languages/helpers.py b/pre_commit/languages/helpers.py index ba83ec74..f0a01658 100644 --- a/pre_commit/languages/helpers.py +++ b/pre_commit/languages/helpers.py @@ -1,16 +1,11 @@ -from plumbum import local +import subprocess def run_hook(env, hook, file_args): return env.run( - ' '.join([hook['entry']] + hook.get('args', []) + list(file_args)), - retcode=None, - ) - return env.run( - ' '.join(['xargs |', hook['entry']] + hook.get('args', [])), - retcode=None, - stdin='\n'.join(file_args) + '\n', + ' '.join(['xargs', hook['entry']] + hook.get('args', [])), + stdin='\n'.join(list(file_args) + ['']), ) @@ -29,9 +24,14 @@ class Environment(object): """ raise NotImplementedError - def run(self, cmd, **kwargs): + def run(self, cmd, stdin=None, **kwargs): """Returns (returncode, stdout, stderr).""" - return local['bash'][ - '-c', - ' '.join([self.env_prefix, cmd]) - ].run(**kwargs) + proc = subprocess.Popen( + ['bash', '-c', ' '.join([self.env_prefix, cmd])], + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + ) + stdout, stderr = proc.communicate(stdin) + + return proc.returncode, stdout, stderr diff --git a/tests/repository_test.py b/tests/repository_test.py index 0d15ea72..8ba7f799 100644 --- a/tests/repository_test.py +++ b/tests/repository_test.py @@ -52,7 +52,6 @@ def test_run_a_python_hook(config_for_python_pre_commit_git_repo): assert ret[1] == "['/dev/null']\nHello World\n" -@pytest.mark.xfail @pytest.mark.integration def test_run_a_hook_lots_of_files(config_for_python_pre_commit_git_repo): repo = Repository(config_for_python_pre_commit_git_repo) @@ -60,7 +59,6 @@ def test_run_a_hook_lots_of_files(config_for_python_pre_commit_git_repo): ret = repo.run_hook('foo', ['/dev/null'] * 15000) assert ret[0] == 0 - assert ret[1] == 'Hello World\n' @pytest.mark.skipif(