From 047a933554e3a89137c8f98b004779ac69ce0171 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Sun, 15 Jun 2014 15:22:29 -0700 Subject: [PATCH] Move empty_git_dir out of pytest fixtures. --- testing/fixtures.py | 14 ++++++ tests/commands/autoupdate_test.py | 6 +-- tests/commands/install_test.py | 7 ++- tests/commands/run_test.py | 4 +- tests/commands/uninstall_test.py | 12 ++++-- tests/conftest.py | 71 +++++++++++++++++-------------- tests/git_test.py | 31 +++++++++----- tests/runner_test.py | 38 ++++++++++------- tests/staged_files_only_test.py | 62 ++++++++++++++++----------- tests/store_test.py | 13 ++++-- 10 files changed, 159 insertions(+), 99 deletions(-) create mode 100644 testing/fixtures.py diff --git a/testing/fixtures.py b/testing/fixtures.py new file mode 100644 index 00000000..f0dfcb85 --- /dev/null +++ b/testing/fixtures.py @@ -0,0 +1,14 @@ +from __future__ import absolute_import +from __future__ import unicode_literals + +from plumbum import local + + +git = local['git'] + + +def git_dir(tmpdir_factory): + path = tmpdir_factory.get() + with local.cwd(path): + git('init') + return path diff --git a/tests/commands/autoupdate_test.py b/tests/commands/autoupdate_test.py index 39d1b335..03da32a3 100644 --- a/tests/commands/autoupdate_test.py +++ b/tests/commands/autoupdate_test.py @@ -75,7 +75,7 @@ def out_of_date_repo(python_hooks_repo): config_wrapped = apply_defaults([config], CONFIG_JSON_SCHEMA) validate_config_extra(config_wrapped) config = config_wrapped[0] - local['git']('commit', '--allow-empty', '-m', 'foo') + local['git']['commit', '--allow-empty', '-m', 'foo']() head_sha = get_head_sha(python_hooks_repo) with open(os.path.join(python_hooks_repo, C.CONFIG_FILE), 'w') as file_obj: @@ -125,8 +125,8 @@ def hook_disappearing_repo(python_hooks_repo): get_resource_path('manifest_without_foo.yaml'), C.MANIFEST_FILE, ) - local['git']('add', '.') - local['git']('commit', '-m', 'Remove foo') + local['git']['add', '.']() + local['git']['commit', '-m', 'Remove foo']() with open(os.path.join(python_hooks_repo, C.CONFIG_FILE), 'w') as file_obj: file_obj.write( diff --git a/tests/commands/install_test.py b/tests/commands/install_test.py index dab7740e..53965e6e 100644 --- a/tests/commands/install_test.py +++ b/tests/commands/install_test.py @@ -1,3 +1,4 @@ +from __future__ import absolute_import from __future__ import unicode_literals import io @@ -8,10 +9,12 @@ import stat from pre_commit.commands.install import install from pre_commit.runner import Runner +from testing.fixtures import git_dir -def test_install_pre_commit(empty_git_dir): - runner = Runner(empty_git_dir) +def test_install_pre_commit(tmpdir_factory): + path = git_dir(tmpdir_factory) + runner = Runner(path) ret = install(runner) assert ret == 0 assert os.path.exists(runner.pre_commit_path) diff --git a/tests/commands/run_test.py b/tests/commands/run_test.py index 261c2a11..021f88b9 100644 --- a/tests/commands/run_test.py +++ b/tests/commands/run_test.py @@ -75,11 +75,9 @@ def test_run_all_hooks_failing( ({'verbose': True}, ('foo.py\nHello World',), 0, True), ({'hook': 'bash_hook'}, ('Bash hook', 'Passed'), 0, True), ({'hook': 'nope'}, ('No hook with id `nope`',), 1, True), - # All the files in the repo. - # This seems kind of weird but it is beacuse py.test reuses fixtures ( {'all_files': True, 'verbose': True}, - ('hooks.yaml', 'bin/hook.sh', 'foo.py', 'dummy'), + ('foo.py'), 0, True, ), diff --git a/tests/commands/uninstall_test.py b/tests/commands/uninstall_test.py index fcecf9d0..9d5a38ed 100644 --- a/tests/commands/uninstall_test.py +++ b/tests/commands/uninstall_test.py @@ -1,3 +1,4 @@ +from __future__ import absolute_import from __future__ import unicode_literals import os.path @@ -5,16 +6,19 @@ import os.path from pre_commit.runner import Runner from pre_commit.commands.install import install from pre_commit.commands.uninstall import uninstall +from testing.fixtures import git_dir -def test_uninstall_pre_commit_does_not_blow_up_when_not_there(empty_git_dir): - runner = Runner(empty_git_dir) +def test_uninstall_does_not_blow_up_when_not_there(tmpdir_factory): + path = git_dir(tmpdir_factory) + runner = Runner(path) ret = uninstall(runner) assert ret == 0 -def test_uninstall(empty_git_dir): - runner = Runner(empty_git_dir) +def test_uninstall(tmpdir_factory): + path = git_dir(tmpdir_factory) + runner = Runner(path) assert not os.path.exists(runner.pre_commit_path) install(runner) assert os.path.exists(runner.pre_commit_path) diff --git a/tests/conftest.py b/tests/conftest.py index 665d34ec..ce1214d1 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,4 +1,5 @@ from __future__ import absolute_import +from __future__ import unicode_literals import mock import os @@ -16,11 +17,15 @@ from pre_commit.jsonschema_extensions import apply_defaults from pre_commit.prefixed_command_runner import PrefixedCommandRunner from pre_commit.runner import Runner from pre_commit.store import Store +from testing.fixtures import git_dir from testing.util import copy_tree_to_path from testing.util import get_head_sha from testing.util import get_resource_path +git = local['git'] + + @pytest.yield_fixture def tmpdir_factory(tmpdir): class TmpdirFactory(object): @@ -43,23 +48,19 @@ def in_tmpdir(tmpdir_factory): yield path -@pytest.yield_fixture -def empty_git_dir(in_tmpdir): - local['git']('init') - yield in_tmpdir - - def add_and_commit(): - local['git']('add', '.') - local['git']('commit', '-m', 'random commit {0}'.format(time.time())) + git('add', '.') + git('commit', '-m', 'random commit {0}'.format(time.time())) @pytest.yield_fixture -def dummy_git_repo(empty_git_dir): - # This is needed otherwise there is no `HEAD` - local['touch']('dummy') - add_and_commit() - yield empty_git_dir +def dummy_git_repo(tmpdir_factory): + path = git_dir(tmpdir_factory) + with local.cwd(path): + # This is needed otherwise there is no `HEAD` + local['touch']('dummy') + add_and_commit() + yield path def _make_repo(repo_path, repo_source): @@ -192,40 +193,48 @@ def _make_repo_from_configs(*configs): @pytest.yield_fixture -def repo_with_passing_hook(config_for_script_hooks_repo, empty_git_dir): - _make_repo_from_configs(config_for_script_hooks_repo) - yield empty_git_dir +def repo_with_passing_hook(config_for_script_hooks_repo, tmpdir_factory): + path = git_dir(tmpdir_factory) + with local.cwd(path): + _make_repo_from_configs(config_for_script_hooks_repo) + yield path @pytest.yield_fixture -def repo_with_failing_hook(failing_hook_repo, empty_git_dir): - _make_repo_from_configs(_make_config(failing_hook_repo, 'failing_hook')) - yield empty_git_dir +def repo_with_failing_hook(failing_hook_repo, tmpdir_factory): + path = git_dir(tmpdir_factory) + with local.cwd(path): + _make_repo_from_configs( + _make_config(failing_hook_repo, 'failing_hook') + ) + yield path @pytest.yield_fixture def in_merge_conflict(repo_with_passing_hook): - local['git']('add', C.CONFIG_FILE) - local['git']('commit', '-m' 'add hooks file') - local['git']('clone', '.', 'foo') + local['touch']('dummy') + git('add', 'dummy') + git('add', C.CONFIG_FILE) + git('commit', '-m' 'add hooks file') + git('clone', '.', 'foo') with local.cwd('foo'): - local['git']('checkout', 'origin/master', '-b', 'foo') + git('checkout', 'origin/master', '-b', 'foo') with open('conflict_file', 'w') as conflict_file: conflict_file.write('herp\nderp\n') - local['git']('add', 'conflict_file') + git('add', 'conflict_file') with open('foo_only_file', 'w') as foo_only_file: foo_only_file.write('foo') - local['git']('add', 'foo_only_file') - local['git']('commit', '-m', 'conflict_file') - local['git']('checkout', 'origin/master', '-b', 'bar') + git('add', 'foo_only_file') + git('commit', '-m', 'conflict_file') + git('checkout', 'origin/master', '-b', 'bar') with open('conflict_file', 'w') as conflict_file: conflict_file.write('harp\nddrp\n') - local['git']('add', 'conflict_file') + git('add', 'conflict_file') with open('bar_only_file', 'w') as bar_only_file: bar_only_file.write('bar') - local['git']('add', 'bar_only_file') - local['git']('commit', '-m', 'conflict_file') - local['git']('merge', 'foo', retcode=None) + git('add', 'bar_only_file') + git('commit', '-m', 'conflict_file') + git('merge', 'foo', retcode=None) yield os.path.join(repo_with_passing_hook, 'foo') diff --git a/tests/git_test.py b/tests/git_test.py index a86383dc..64b0465e 100644 --- a/tests/git_test.py +++ b/tests/git_test.py @@ -1,21 +1,32 @@ +from __future__ import absolute_import + +import os.path import pytest from plumbum import local from pre_commit import git +from testing.fixtures import git_dir -def test_get_root(empty_git_dir): - assert git.get_root() == empty_git_dir - - foo = local.path('foo') - foo.mkdir() - - with local.cwd(foo): - assert git.get_root() == empty_git_dir +def test_get_root_at_root(tmpdir_factory): + path = git_dir(tmpdir_factory) + with local.cwd(path): + assert git.get_root() == path -def test_is_not_in_merge_conflict(empty_git_dir): - assert git.is_in_merge_conflict() is False +def test_get_root_deeper(tmpdir_factory): + path = git_dir(tmpdir_factory) + + foo_path = os.path.join(path, 'foo') + os.mkdir(foo_path) + with local.cwd(foo_path): + assert git.get_root() == path + + +def test_is_not_in_merge_conflict(tmpdir_factory): + path = git_dir(tmpdir_factory) + with local.cwd(path): + assert git.is_in_merge_conflict() is False def test_is_in_merge_conflict(in_merge_conflict): diff --git a/tests/runner_test.py b/tests/runner_test.py index ed333152..7cc7f6b4 100644 --- a/tests/runner_test.py +++ b/tests/runner_test.py @@ -1,9 +1,13 @@ +from __future__ import absolute_import +from __future__ import unicode_literals + import os import os.path -import pytest +from plumbum import local import pre_commit.constants as C from pre_commit.runner import Runner +from testing.fixtures import git_dir def test_init_has_no_side_effects(tmpdir): @@ -13,24 +17,26 @@ def test_init_has_no_side_effects(tmpdir): assert os.getcwd() == current_wd -def test_create_sets_correct_directory(empty_git_dir): - runner = Runner.create() - assert runner.git_root == empty_git_dir - assert os.getcwd() == empty_git_dir +def test_create_sets_correct_directory(tmpdir_factory): + path = git_dir(tmpdir_factory) + with local.cwd(path): + runner = Runner.create() + assert runner.git_root == path + assert os.getcwd() == path -@pytest.yield_fixture -def git_dir_with_directory(empty_git_dir): - os.mkdir('foo') - yield empty_git_dir +def test_create_changes_to_git_root(tmpdir_factory): + path = git_dir(tmpdir_factory) + with local.cwd(path): + # Change into some directory, create should set to root + foo_path = os.path.join(path, 'foo') + os.mkdir(foo_path) + os.chdir(foo_path) + assert os.getcwd() != path - -def test_changes_to_root_of_git_dir(git_dir_with_directory): - os.chdir('foo') - assert os.getcwd() != git_dir_with_directory - runner = Runner.create() - assert runner.git_root == git_dir_with_directory - assert os.getcwd() == git_dir_with_directory + runner = Runner.create() + assert runner.git_root == path + assert os.getcwd() == path def test_config_file_path(): diff --git a/tests/staged_files_only_test.py b/tests/staged_files_only_test.py index 79830ae4..e549bd42 100644 --- a/tests/staged_files_only_test.py +++ b/tests/staged_files_only_test.py @@ -1,3 +1,4 @@ +from __future__ import absolute_import from __future__ import unicode_literals import io @@ -10,6 +11,7 @@ from plumbum import local from pre_commit.staged_files_only import staged_files_only from testing.auto_namedtuple import auto_namedtuple +from testing.fixtures import git_dir from testing.util import get_resource_path @@ -22,12 +24,14 @@ def get_short_git_status(): @pytest.yield_fixture -def foo_staged(empty_git_dir): - with io.open('foo', 'w') as foo_file: - foo_file.write(FOO_CONTENTS) - local['git']('add', 'foo') - foo_filename = os.path.join(empty_git_dir, 'foo') - yield auto_namedtuple(path=empty_git_dir, foo_filename=foo_filename) +def foo_staged(tmpdir_factory): + path = git_dir(tmpdir_factory) + with local.cwd(path): + with io.open('foo', 'w') as foo_file: + foo_file.write(FOO_CONTENTS) + local['git']('add', 'foo') + foo_filename = os.path.join(path, 'foo') + yield auto_namedtuple(path=path, foo_filename=foo_filename) def _test_foo_state(path, foo_contents=FOO_CONTENTS, status='A'): @@ -96,11 +100,13 @@ def test_foo_both_modify_conflicting(foo_staged, cmd_runner): @pytest.yield_fixture -def img_staged(empty_git_dir): - img_filename = os.path.join(empty_git_dir, 'img.jpg') - shutil.copy(get_resource_path('img1.jpg'), img_filename) - local['git']('add', 'img.jpg') - yield auto_namedtuple(path=empty_git_dir, img_filename=img_filename) +def img_staged(tmpdir_factory): + path = git_dir(tmpdir_factory) + with local.cwd(path): + img_filename = os.path.join(path, 'img.jpg') + shutil.copy(get_resource_path('img1.jpg'), img_filename) + local['git']('add', 'img.jpg') + yield auto_namedtuple(path=path, img_filename=img_filename) def _test_img_state(path, expected_file='img1.jpg', status='A'): @@ -149,12 +155,14 @@ def test_img_conflict(img_staged, cmd_runner): @pytest.yield_fixture -def submodule_with_commits(empty_git_dir): - local['git']('commit', '--allow-empty', '-m', 'foo') - sha1 = local['git']('rev-parse', 'HEAD').strip() - local['git']('commit', '--allow-empty', '-m', 'bar') - sha2 = local['git']('rev-parse', 'HEAD').strip() - yield auto_namedtuple(path=empty_git_dir, sha1=sha1, sha2=sha2) +def submodule_with_commits(tmpdir_factory): + path = git_dir(tmpdir_factory) + with local.cwd(path): + local['git']('commit', '--allow-empty', '-m', 'foo') + sha1 = local['git']('rev-parse', 'HEAD').strip() + local['git']('commit', '--allow-empty', '-m', 'bar') + sha2 = local['git']('rev-parse', 'HEAD').strip() + yield auto_namedtuple(path=path, sha1=sha1, sha2=sha2) def checkout_submodule(sha): @@ -163,15 +171,17 @@ def checkout_submodule(sha): @pytest.yield_fixture -def sub_staged(submodule_with_commits, empty_git_dir): - local['git']('submodule', 'add', submodule_with_commits.path, 'sub') - checkout_submodule(submodule_with_commits.sha1) - local['git']('add', 'sub') - yield auto_namedtuple( - path=empty_git_dir, - sub_path=os.path.join(empty_git_dir, 'sub'), - submodule=submodule_with_commits, - ) +def sub_staged(submodule_with_commits, tmpdir_factory): + path = git_dir(tmpdir_factory) + with local.cwd(path): + local['git']('submodule', 'add', submodule_with_commits.path, 'sub') + checkout_submodule(submodule_with_commits.sha1) + local['git']('add', 'sub') + yield auto_namedtuple( + path=path, + sub_path=os.path.join(path, 'sub'), + submodule=submodule_with_commits, + ) def _test_sub_state(path, sha='sha1', status='A'): diff --git a/tests/store_test.py b/tests/store_test.py index 2bdea9de..5ebfe05d 100644 --- a/tests/store_test.py +++ b/tests/store_test.py @@ -1,3 +1,6 @@ +from __future__ import absolute_import +from __future__ import unicode_literals + import io import mock import os @@ -10,6 +13,7 @@ from pre_commit import five from pre_commit.store import _get_default_directory from pre_commit.store import logger from pre_commit.store import Store +from testing.fixtures import git_dir from testing.util import get_head_sha @@ -71,13 +75,14 @@ def log_info_mock(): yield info_mock -def test_clone(store, empty_git_dir, log_info_mock): - with local.cwd(empty_git_dir): +def test_clone(store, tmpdir_factory, log_info_mock): + path = git_dir(tmpdir_factory) + with local.cwd(path): local['git']('commit', '--allow-empty', '-m', 'foo') - sha = get_head_sha(empty_git_dir) + sha = get_head_sha(path) local['git']('commit', '--allow-empty', '-m', 'bar') - ret = store.clone(empty_git_dir, sha) + ret = store.clone(path, sha) # Should have printed some stuff log_info_mock.assert_called_with('This may take a few minutes...')