From 29033f10caadb816f77a6a8826ae9d780b2684ce Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Sat, 24 Feb 2018 16:44:59 -0800 Subject: [PATCH] Move cwd() to tests-only --- pre_commit/commands/autoupdate.py | 23 ++++++------- pre_commit/make_archives.py | 4 +-- pre_commit/store.py | 19 +++++----- pre_commit/util.py | 10 ------ testing/fixtures.py | 16 ++++----- testing/util.py | 11 ++++++ tests/commands/autoupdate_test.py | 40 ++++++++++------------ tests/commands/install_uninstall_test.py | 7 ++-- tests/commands/run_test.py | 2 +- tests/commands/try_repo_test.py | 2 +- tests/conftest.py | 15 ++++---- tests/git_test.py | 2 +- tests/main_test.py | 2 +- tests/make_archives_test.py | 20 +++++------ tests/meta_hooks/check_hooks_apply_test.py | 2 +- tests/meta_hooks/useless_excludes_test.py | 2 +- tests/repository_test.py | 2 +- tests/runner_test.py | 2 +- tests/staged_files_only_test.py | 2 +- tests/store_test.py | 2 +- tests/util_test.py | 2 +- 21 files changed, 84 insertions(+), 103 deletions(-) diff --git a/pre_commit/commands/autoupdate.py b/pre_commit/commands/autoupdate.py index 666cd117..ce6fc34a 100644 --- a/pre_commit/commands/autoupdate.py +++ b/pre_commit/commands/autoupdate.py @@ -18,7 +18,6 @@ from pre_commit.commands.migrate_config import migrate_config from pre_commit.repository import Repository from pre_commit.util import CalledProcessError from pre_commit.util import cmd_output -from pre_commit.util import cwd class RepositoryCannotBeUpdatedError(RuntimeError): @@ -35,17 +34,17 @@ def _update_repo(repo_config, runner, tags_only): """ repo_path = runner.store.clone(repo_config['repo'], repo_config['sha']) - with cwd(repo_path): - cmd_output('git', 'fetch') - tag_cmd = ('git', 'describe', 'origin/master', '--tags') - if tags_only: - tag_cmd += ('--abbrev=0',) - else: - tag_cmd += ('--exact',) - try: - rev = cmd_output(*tag_cmd)[1].strip() - except CalledProcessError: - rev = cmd_output('git', 'rev-parse', 'origin/master')[1].strip() + cmd_output('git', '-C', repo_path, 'fetch') + tag_cmd = ('git', '-C', repo_path, 'describe', 'origin/master', '--tags') + if tags_only: + tag_cmd += ('--abbrev=0',) + else: + tag_cmd += ('--exact',) + try: + rev = cmd_output(*tag_cmd)[1].strip() + except CalledProcessError: + tag_cmd = ('git', '-C', repo_path, 'rev-parse', 'origin/master') + rev = cmd_output(*tag_cmd)[1].strip() # Don't bother trying to update if our sha is the same if rev == repo_config['sha']: diff --git a/pre_commit/make_archives.py b/pre_commit/make_archives.py index 90809c10..2e7658da 100644 --- a/pre_commit/make_archives.py +++ b/pre_commit/make_archives.py @@ -8,7 +8,6 @@ import tarfile from pre_commit import output from pre_commit.util import cmd_output -from pre_commit.util import cwd from pre_commit.util import resource_filename from pre_commit.util import rmtree from pre_commit.util import tmpdir @@ -42,8 +41,7 @@ def make_archive(name, repo, ref, destdir): with tmpdir() as tempdir: # Clone the repository to the temporary directory cmd_output('git', 'clone', repo, tempdir) - with cwd(tempdir): - cmd_output('git', 'checkout', ref) + cmd_output('git', '-C', tempdir, 'checkout', ref) # We don't want the '.git' directory # It adds a bunch of size to the archive and we don't use it at diff --git a/pre_commit/store.py b/pre_commit/store.py index 7e49c8fd..735d67cf 100644 --- a/pre_commit/store.py +++ b/pre_commit/store.py @@ -14,7 +14,6 @@ from pre_commit import file_lock from pre_commit.util import clean_path_on_failure from pre_commit.util import cmd_output from pre_commit.util import copy_tree_to_path -from pre_commit.util import cwd from pre_commit.util import no_git_env from pre_commit.util import resource_filename @@ -142,16 +141,14 @@ class Store(object): def clone(self, repo, ref, deps=()): """Clone the given url and checkout the specific ref.""" def clone_strategy(directory): - cmd_output( - 'git', 'clone', '--no-checkout', repo, directory, - env=no_git_env(), - ) - with cwd(directory): - cmd_output('git', 'reset', ref, '--hard', env=no_git_env()) - cmd_output( - 'git', 'submodule', 'update', '--init', '--recursive', - env=no_git_env(), - ) + env = no_git_env() + + def _git_cmd(*args): + return cmd_output('git', '-C', directory, *args, env=env) + + _git_cmd('clone', '--no-checkout', repo, '.') + _git_cmd('reset', ref, '--hard') + _git_cmd('submodule', 'update', '--init', '--recursive') return self._new_repo(repo, ref, deps, clone_strategy) diff --git a/pre_commit/util.py b/pre_commit/util.py index 081adf27..882ebb00 100644 --- a/pre_commit/util.py +++ b/pre_commit/util.py @@ -16,16 +16,6 @@ from pre_commit import five from pre_commit import parse_shebang -@contextlib.contextmanager -def cwd(path): - original_cwd = os.getcwd() - os.chdir(path) - try: - yield - finally: - os.chdir(original_cwd) - - def mkdirp(path): try: os.makedirs(path) diff --git a/testing/fixtures.py b/testing/fixtures.py index bff32805..3537ca71 100644 --- a/testing/fixtures.py +++ b/testing/fixtures.py @@ -17,7 +17,6 @@ from pre_commit.clientlib import CONFIG_SCHEMA from pre_commit.clientlib import load_manifest from pre_commit.util import cmd_output from pre_commit.util import copy_tree_to_path -from pre_commit.util import cwd from testing.util import get_resource_path @@ -30,9 +29,8 @@ def git_dir(tempdir_factory): def make_repo(tempdir_factory, repo_source): path = git_dir(tempdir_factory) copy_tree_to_path(get_resource_path(repo_source), path) - with cwd(path): - cmd_output('git', 'add', '.') - cmd_output('git', 'commit', '-m', 'Add hooks') + cmd_output('git', '-C', path, 'add', '.') + cmd_output('git', '-C', path, 'commit', '-m', 'Add hooks') return path @@ -116,17 +114,15 @@ def write_config(directory, config, config_file=C.CONFIG_FILE): def add_config_to_repo(git_path, config, config_file=C.CONFIG_FILE): write_config(git_path, config, config_file=config_file) - with cwd(git_path): - cmd_output('git', 'add', config_file) - cmd_output('git', 'commit', '-m', 'Add hooks config') + cmd_output('git', '-C', git_path, 'add', config_file) + cmd_output('git', '-C', git_path, 'commit', '-m', 'Add hooks config') return git_path def remove_config_from_repo(git_path, config_file=C.CONFIG_FILE): os.unlink(os.path.join(git_path, config_file)) - with cwd(git_path): - cmd_output('git', 'add', config_file) - cmd_output('git', 'commit', '-m', 'Remove hooks config') + cmd_output('git', '-C', git_path, 'add', config_file) + cmd_output('git', '-C', git_path, 'commit', '-m', 'Remove hooks config') return git_path diff --git a/testing/util.py b/testing/util.py index aa4b76f5..025bc0bb 100644 --- a/testing/util.py +++ b/testing/util.py @@ -1,5 +1,6 @@ from __future__ import unicode_literals +import contextlib import os.path import sys @@ -103,3 +104,13 @@ def run_opts( show_diff_on_failure=show_diff_on_failure, commit_msg_filename=commit_msg_filename, ) + + +@contextlib.contextmanager +def cwd(path): + original_cwd = os.getcwd() + os.chdir(path) + try: + yield + finally: + os.chdir(original_cwd) diff --git a/tests/commands/autoupdate_test.py b/tests/commands/autoupdate_test.py index 91e7733f..11c71705 100644 --- a/tests/commands/autoupdate_test.py +++ b/tests/commands/autoupdate_test.py @@ -1,5 +1,6 @@ from __future__ import unicode_literals +import os.path import pipes import shutil from collections import OrderedDict @@ -14,7 +15,6 @@ from pre_commit.commands.autoupdate import autoupdate from pre_commit.commands.autoupdate import RepositoryCannotBeUpdatedError from pre_commit.runner import Runner from pre_commit.util import cmd_output -from pre_commit.util import cwd from testing.auto_namedtuple import auto_namedtuple from testing.fixtures import add_config_to_repo from testing.fixtures import config_with_local_hooks @@ -62,14 +62,13 @@ def test_autoupdate_old_revision_broken( path = make_repo(tempdir_factory, 'python_hooks_repo') config = make_config_from_repo(path, check=False) - with cwd(path): - cmd_output('git', 'mv', C.MANIFEST_FILE, 'nope.yaml') - cmd_output('git', 'commit', '-m', 'simulate old repo') - # Assume this is the revision the user's old repository was at - rev = git.head_sha(path) - cmd_output('git', 'mv', 'nope.yaml', C.MANIFEST_FILE) - cmd_output('git', 'commit', '-m', 'move hooks file') - update_rev = git.head_sha(path) + cmd_output('git', '-C', path, 'mv', C.MANIFEST_FILE, 'nope.yaml') + cmd_output('git', '-C', path, 'commit', '-m', 'simulate old repo') + # Assume this is the revision the user's old repository was at + rev = git.head_sha(path) + cmd_output('git', '-C', path, 'mv', 'nope.yaml', C.MANIFEST_FILE) + cmd_output('git', '-C', path, 'commit', '-m', 'move hooks file') + update_rev = git.head_sha(path) config['sha'] = rev write_config('.', config) @@ -87,8 +86,7 @@ def out_of_date_repo(tempdir_factory): original_sha = git.head_sha(path) # Make a commit - with cwd(path): - cmd_output('git', 'commit', '--allow-empty', '-m', 'foo') + cmd_output('git', '-C', path, 'commit', '--allow-empty', '-m', 'foo') head_sha = git.head_sha(path) yield auto_namedtuple( @@ -223,8 +221,7 @@ def test_loses_formatting_when_not_detectable( @pytest.fixture def tagged_repo(out_of_date_repo): - with cwd(out_of_date_repo.path): - cmd_output('git', 'tag', 'v1.2.3') + cmd_output('git', '-C', out_of_date_repo.path, 'tag', 'v1.2.3') yield out_of_date_repo @@ -243,8 +240,8 @@ def test_autoupdate_tagged_repo( @pytest.fixture def tagged_repo_with_more_commits(tagged_repo): - with cwd(tagged_repo.path): - cmd_output('git', 'commit', '--allow-empty', '-m', 'commit!') + cmd = ('git', '-C', tagged_repo.path, 'commit', '--allow-empty', '-mfoo') + cmd_output(*cmd) yield tagged_repo @@ -267,13 +264,12 @@ def hook_disappearing_repo(tempdir_factory): path = make_repo(tempdir_factory, 'python_hooks_repo') original_sha = git.head_sha(path) - with cwd(path): - shutil.copy( - get_resource_path('manifest_without_foo.yaml'), - C.MANIFEST_FILE, - ) - cmd_output('git', 'add', '.') - cmd_output('git', 'commit', '-m', 'Remove foo') + shutil.copy( + get_resource_path('manifest_without_foo.yaml'), + os.path.join(path, C.MANIFEST_FILE), + ) + cmd_output('git', '-C', path, 'add', '.') + cmd_output('git', '-C', path, 'commit', '-m', 'Remove foo') yield auto_namedtuple(path=path, original_sha=original_sha) diff --git a/tests/commands/install_uninstall_test.py b/tests/commands/install_uninstall_test.py index 00d5eff4..a49a3e4f 100644 --- a/tests/commands/install_uninstall_test.py +++ b/tests/commands/install_uninstall_test.py @@ -20,7 +20,6 @@ from pre_commit.commands.install_uninstall import PRIOR_HASHES from pre_commit.commands.install_uninstall import uninstall from pre_commit.runner import Runner from pre_commit.util import cmd_output -from pre_commit.util import cwd from pre_commit.util import make_executable from pre_commit.util import mkdirp from pre_commit.util import resource_filename @@ -28,6 +27,7 @@ from testing.fixtures import git_dir from testing.fixtures import make_consuming_repo from testing.fixtures import remove_config_from_repo from testing.util import cmd_output_mocked_pre_commit_home +from testing.util import cwd from testing.util import xfailif_no_symlink @@ -153,9 +153,8 @@ def test_install_pre_commit_and_run_custom_path(tempdir_factory): def test_install_in_submodule_and_run(tempdir_factory): src_path = make_consuming_repo(tempdir_factory, 'script_hooks_repo') parent_path = git_dir(tempdir_factory) - with cwd(parent_path): - cmd_output('git', 'submodule', 'add', src_path, 'sub') - cmd_output('git', 'commit', '-m', 'foo') + cmd_output('git', '-C', parent_path, 'submodule', 'add', src_path, 'sub') + cmd_output('git', '-C', parent_path, 'commit', '-m', 'foo') sub_pth = os.path.join(parent_path, 'sub') with cwd(sub_pth): diff --git a/tests/commands/run_test.py b/tests/commands/run_test.py index 94dd5219..8107e79a 100644 --- a/tests/commands/run_test.py +++ b/tests/commands/run_test.py @@ -18,13 +18,13 @@ from pre_commit.commands.run import _has_unmerged_paths from pre_commit.commands.run import run from pre_commit.runner import Runner from pre_commit.util import cmd_output -from pre_commit.util import cwd from pre_commit.util import make_executable from testing.fixtures import add_config_to_repo from testing.fixtures import make_consuming_repo from testing.fixtures import modify_config from testing.fixtures import read_config from testing.util import cmd_output_mocked_pre_commit_home +from testing.util import cwd from testing.util import run_opts from testing.util import xfailif_no_symlink diff --git a/tests/commands/try_repo_test.py b/tests/commands/try_repo_test.py index e530dee8..a29181b8 100644 --- a/tests/commands/try_repo_test.py +++ b/tests/commands/try_repo_test.py @@ -5,10 +5,10 @@ import re from pre_commit.commands.try_repo import try_repo from pre_commit.util import cmd_output -from pre_commit.util import cwd from testing.auto_namedtuple import auto_namedtuple from testing.fixtures import git_dir from testing.fixtures import make_repo +from testing.util import cwd from testing.util import run_opts diff --git a/tests/conftest.py b/tests/conftest.py index 246820e9..678010f5 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -17,10 +17,10 @@ from pre_commit.logging_handler import add_logging_handler from pre_commit.runner import Runner from pre_commit.store import Store from pre_commit.util import cmd_output -from pre_commit.util import cwd from testing.fixtures import git_dir from testing.fixtures import make_consuming_repo from testing.fixtures import write_config +from testing.util import cwd @pytest.fixture @@ -68,10 +68,9 @@ def _make_conflict(): @pytest.fixture def in_merge_conflict(tempdir_factory): path = make_consuming_repo(tempdir_factory, 'script_hooks_repo') - with cwd(path): - open('dummy', 'a').close() - cmd_output('git', 'add', 'dummy') - cmd_output('git', 'commit', '-m', 'Add config.') + open(os.path.join(path, 'dummy'), 'a').close() + cmd_output('git', '-C', path, 'add', 'dummy') + cmd_output('git', '-C', path, 'commit', '-m', 'Add config.') conflict_path = tempdir_factory.get() cmd_output('git', 'clone', path, conflict_path) @@ -84,10 +83,8 @@ def in_merge_conflict(tempdir_factory): def in_conflicting_submodule(tempdir_factory): git_dir_1 = git_dir(tempdir_factory) git_dir_2 = git_dir(tempdir_factory) - with cwd(git_dir_2): - cmd_output('git', 'commit', '--allow-empty', '-m', 'init!') - with cwd(git_dir_1): - cmd_output('git', 'submodule', 'add', git_dir_2, 'sub') + cmd_output('git', '-C', git_dir_2, 'commit', '--allow-empty', '-minit!') + cmd_output('git', '-C', git_dir_1, 'submodule', 'add', git_dir_2, 'sub') with cwd(os.path.join(git_dir_1, 'sub')): _make_conflict() yield diff --git a/tests/git_test.py b/tests/git_test.py index 8f80dcad..58f14f50 100644 --- a/tests/git_test.py +++ b/tests/git_test.py @@ -9,8 +9,8 @@ import pytest from pre_commit import git from pre_commit.error_handler import FatalError from pre_commit.util import cmd_output -from pre_commit.util import cwd from testing.fixtures import git_dir +from testing.util import cwd def test_get_root_at_root(tempdir_factory): diff --git a/tests/main_test.py b/tests/main_test.py index deb3ba18..ae6a73e7 100644 --- a/tests/main_test.py +++ b/tests/main_test.py @@ -8,8 +8,8 @@ import mock import pytest from pre_commit import main -from pre_commit.util import cwd from testing.auto_namedtuple import auto_namedtuple +from testing.util import cwd FNS = ( diff --git a/tests/make_archives_test.py b/tests/make_archives_test.py index 2cb62697..414f853c 100644 --- a/tests/make_archives_test.py +++ b/tests/make_archives_test.py @@ -9,7 +9,6 @@ import pytest from pre_commit import git from pre_commit import make_archives from pre_commit.util import cmd_output -from pre_commit.util import cwd from testing.fixtures import git_dir @@ -17,16 +16,15 @@ def test_make_archive(tempdir_factory): output_dir = tempdir_factory.get() git_path = git_dir(tempdir_factory) # Add a files to the git directory - with cwd(git_path): - open('foo', 'a').close() - cmd_output('git', 'add', '.') - cmd_output('git', 'commit', '-m', 'foo') - # We'll use this sha - head_sha = git.head_sha('.') - # And check that this file doesn't exist - open('bar', 'a').close() - cmd_output('git', 'add', '.') - cmd_output('git', 'commit', '-m', 'bar') + open(os.path.join(git_path, 'foo'), 'a').close() + cmd_output('git', '-C', git_path, 'add', '.') + cmd_output('git', '-C', git_path, 'commit', '-m', 'foo') + # We'll use this sha + head_sha = git.head_sha(git_path) + # And check that this file doesn't exist + open(os.path.join(git_path, 'bar'), 'a').close() + cmd_output('git', '-C', git_path, 'add', '.') + cmd_output('git', '-C', git_path, 'commit', '-m', 'bar') # Do the thing archive_path = make_archives.make_archive( diff --git a/tests/meta_hooks/check_hooks_apply_test.py b/tests/meta_hooks/check_hooks_apply_test.py index 86bc598d..c777daa8 100644 --- a/tests/meta_hooks/check_hooks_apply_test.py +++ b/tests/meta_hooks/check_hooks_apply_test.py @@ -1,9 +1,9 @@ from collections import OrderedDict from pre_commit.meta_hooks import check_hooks_apply -from pre_commit.util import cwd from testing.fixtures import add_config_to_repo from testing.fixtures import git_dir +from testing.util import cwd def test_hook_excludes_everything( diff --git a/tests/meta_hooks/useless_excludes_test.py b/tests/meta_hooks/useless_excludes_test.py index 08b87aa8..137c357f 100644 --- a/tests/meta_hooks/useless_excludes_test.py +++ b/tests/meta_hooks/useless_excludes_test.py @@ -1,9 +1,9 @@ from collections import OrderedDict from pre_commit.meta_hooks import check_useless_excludes -from pre_commit.util import cwd from testing.fixtures import add_config_to_repo from testing.fixtures import git_dir +from testing.util import cwd def test_useless_exclude_global(capsys, tempdir_factory): diff --git a/tests/repository_test.py b/tests/repository_test.py index dea387f2..7f0593bf 100644 --- a/tests/repository_test.py +++ b/tests/repository_test.py @@ -22,12 +22,12 @@ from pre_commit.languages import python from pre_commit.languages import ruby from pre_commit.repository import Repository from pre_commit.util import cmd_output -from pre_commit.util import cwd from testing.fixtures import config_with_local_hooks from testing.fixtures import git_dir from testing.fixtures import make_config_from_repo from testing.fixtures import make_repo from testing.fixtures import modify_manifest +from testing.util import cwd from testing.util import get_resource_path from testing.util import skipif_cant_run_docker from testing.util import skipif_cant_run_swift diff --git a/tests/runner_test.py b/tests/runner_test.py index b5c0ce75..df324712 100644 --- a/tests/runner_test.py +++ b/tests/runner_test.py @@ -7,10 +7,10 @@ from collections import OrderedDict import pre_commit.constants as C from pre_commit.runner import Runner from pre_commit.util import cmd_output -from pre_commit.util import cwd from testing.fixtures import add_config_to_repo from testing.fixtures import git_dir from testing.fixtures import make_consuming_repo +from testing.util import cwd def test_init_has_no_side_effects(tmpdir): diff --git a/tests/staged_files_only_test.py b/tests/staged_files_only_test.py index d4dfadd6..481a2886 100644 --- a/tests/staged_files_only_test.py +++ b/tests/staged_files_only_test.py @@ -11,9 +11,9 @@ import pytest from pre_commit.staged_files_only import staged_files_only from pre_commit.util import cmd_output -from pre_commit.util import cwd from testing.auto_namedtuple import auto_namedtuple from testing.fixtures import git_dir +from testing.util import cwd from testing.util import get_resource_path diff --git a/tests/store_test.py b/tests/store_test.py index deb22bb8..86c3ec44 100644 --- a/tests/store_test.py +++ b/tests/store_test.py @@ -13,9 +13,9 @@ from pre_commit import git from pre_commit.store import _get_default_directory from pre_commit.store import Store from pre_commit.util import cmd_output -from pre_commit.util import cwd from pre_commit.util import rmtree from testing.fixtures import git_dir +from testing.util import cwd def test_our_session_fixture_works(): diff --git a/tests/util_test.py b/tests/util_test.py index 156148d5..967163e4 100644 --- a/tests/util_test.py +++ b/tests/util_test.py @@ -8,9 +8,9 @@ import pytest from pre_commit.util import CalledProcessError from pre_commit.util import clean_path_on_failure from pre_commit.util import cmd_output -from pre_commit.util import cwd from pre_commit.util import memoize_by_cwd from pre_commit.util import tmpdir +from testing.util import cwd def test_CalledProcessError_str():