From 67d6fcb0f68f2b6737c8430995112d29f09ef4de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Schoentgen?= Date: Fri, 10 Aug 2018 10:21:20 +0200 Subject: [PATCH] Fix several ResourceWarning: unclosed file --- pre_commit/commands/autoupdate.py | 3 +- pre_commit/commands/install_uninstall.py | 3 +- pre_commit/git.py | 3 +- pre_commit/repository.py | 3 +- testing/fixtures.py | 9 +++-- tests/commands/autoupdate_test.py | 48 ++++++++++++++++-------- tests/commands/install_uninstall_test.py | 3 +- tests/conftest.py | 20 ++++++++++ tests/error_handler_test.py | 10 ++--- tests/staged_files_only_test.py | 10 ++--- 10 files changed, 78 insertions(+), 34 deletions(-) diff --git a/pre_commit/commands/autoupdate.py b/pre_commit/commands/autoupdate.py index 241126dd..8f3714c4 100644 --- a/pre_commit/commands/autoupdate.py +++ b/pre_commit/commands/autoupdate.py @@ -72,7 +72,8 @@ REV_LINE_FMT = '{}rev:{}{}{}' def _write_new_config_file(path, output): - original_contents = open(path).read() + with open(path) as f: + original_contents = f.read() output = remove_defaults(output, CONFIG_SCHEMA) new_contents = ordered_dump(output, **C.YAML_DUMP_KWARGS) diff --git a/pre_commit/commands/install_uninstall.py b/pre_commit/commands/install_uninstall.py index f5947de7..d76a6c1a 100644 --- a/pre_commit/commands/install_uninstall.py +++ b/pre_commit/commands/install_uninstall.py @@ -38,7 +38,8 @@ def _hook_paths(git_root, hook_type): def is_our_script(filename): if not os.path.exists(filename): return False - contents = io.open(filename).read() + with io.open(filename) as f: + contents = f.read() return any(h in contents for h in (CURRENT_HASH,) + PRIOR_HASHES) diff --git a/pre_commit/git.py b/pre_commit/git.py index d9e01f5f..9ec9c9fb 100644 --- a/pre_commit/git.py +++ b/pre_commit/git.py @@ -70,7 +70,8 @@ def get_conflicted_files(): logger.info('Checking merge-conflict files only.') # Need to get the conflicted files from the MERGE_MSG because they could # have resolved the conflict by choosing one side or the other - merge_msg = open(os.path.join(get_git_dir('.'), 'MERGE_MSG'), 'rb').read() + with open(os.path.join(get_git_dir('.'), 'MERGE_MSG'), 'rb') as f: + merge_msg = f.read() merge_conflict_filenames = parse_merge_msg_for_conflicts(merge_msg) # This will get the rest of the changes made after the merge. diff --git a/pre_commit/repository.py b/pre_commit/repository.py index e78fba16..278f31a2 100644 --- a/pre_commit/repository.py +++ b/pre_commit/repository.py @@ -43,7 +43,8 @@ def _read_state(prefix, venv): if not os.path.exists(filename): return None else: - return json.loads(io.open(filename).read()) + with io.open(filename) as f: + return json.loads(f.read()) def _write_state(prefix, venv, state): diff --git a/testing/fixtures.py b/testing/fixtures.py index fd5c7b43..cbcb7bb0 100644 --- a/testing/fixtures.py +++ b/testing/fixtures.py @@ -40,7 +40,8 @@ def modify_manifest(path): .pre-commit-hooks.yaml. """ manifest_path = os.path.join(path, C.MANIFEST_FILE) - manifest = ordered_load(io.open(manifest_path).read()) + with io.open(manifest_path) as f: + manifest = ordered_load(f.read()) yield manifest with io.open(manifest_path, 'w') as manifest_file: manifest_file.write(ordered_dump(manifest, **C.YAML_DUMP_KWARGS)) @@ -55,7 +56,8 @@ def modify_config(path='.', commit=True): .pre-commit-config.yaml """ config_path = os.path.join(path, C.CONFIG_FILE) - config = ordered_load(io.open(config_path).read()) + with io.open(config_path) as f: + config = ordered_load(f.read()) yield config with io.open(config_path, 'w', encoding='UTF-8') as config_file: config_file.write(ordered_dump(config, **C.YAML_DUMP_KWARGS)) @@ -100,7 +102,8 @@ def make_config_from_repo(repo_path, rev=None, hooks=None, check=True): def read_config(directory, config_file=C.CONFIG_FILE): config_path = os.path.join(directory, config_file) - config = ordered_load(io.open(config_path).read()) + with io.open(config_path) as f: + config = ordered_load(f.read()) return config diff --git a/tests/commands/autoupdate_test.py b/tests/commands/autoupdate_test.py index 5408d45a..3bfb62e0 100644 --- a/tests/commands/autoupdate_test.py +++ b/tests/commands/autoupdate_test.py @@ -42,10 +42,12 @@ def test_autoupdate_up_to_date_repo(up_to_date_repo, in_tmpdir, store): config = make_config_from_repo(up_to_date_repo, check=False) write_config('.', config) - before = open(C.CONFIG_FILE).read() + with open(C.CONFIG_FILE) as f: + before = f.read() assert '^$' not in before ret = autoupdate(Runner('.', C.CONFIG_FILE), store, tags_only=False) - after = open(C.CONFIG_FILE).read() + with open(C.CONFIG_FILE) as f: + after = f.read() assert ret == 0 assert before == after @@ -68,9 +70,11 @@ def test_autoupdate_old_revision_broken(tempdir_factory, in_tmpdir, store): config['rev'] = rev write_config('.', config) - before = open(C.CONFIG_FILE).read() + with open(C.CONFIG_FILE) as f: + before = f.read() ret = autoupdate(Runner('.', C.CONFIG_FILE), store, tags_only=False) - after = open(C.CONFIG_FILE).read() + with open(C.CONFIG_FILE) as f: + after = f.read() assert ret == 0 assert before != after assert update_rev in after @@ -106,9 +110,11 @@ def test_autoupdate_out_of_date_repo(out_of_date_repo, in_tmpdir, store): ) write_config('.', config) - before = open(C.CONFIG_FILE).read() + with open(C.CONFIG_FILE) as f: + before = f.read() ret = autoupdate(Runner('.', C.CONFIG_FILE), store, tags_only=False) - after = open(C.CONFIG_FILE).read() + with open(C.CONFIG_FILE) as f: + after = f.read() assert ret == 0 assert before != after # Make sure we don't add defaults @@ -128,10 +134,12 @@ def test_autoupdate_out_of_date_repo_with_correct_repo_name( write_config('.', config) runner = Runner('.', C.CONFIG_FILE) - before = open(C.CONFIG_FILE).read() + with open(C.CONFIG_FILE) as f: + before = f.read() repo_name = 'file://{}'.format(out_of_date_repo.path) ret = autoupdate(runner, store, tags_only=False, repos=(repo_name,)) - after = open(C.CONFIG_FILE).read() + with open(C.CONFIG_FILE) as f: + after = f.read() assert ret == 0 assert before != after assert out_of_date_repo.head_rev in after @@ -148,10 +156,12 @@ def test_autoupdate_out_of_date_repo_with_wrong_repo_name( write_config('.', config) runner = Runner('.', C.CONFIG_FILE) - before = open(C.CONFIG_FILE).read() + with open(C.CONFIG_FILE) as f: + before = f.read() # It will not update it, because the name doesn't match ret = autoupdate(runner, store, tags_only=False, repos=('dne',)) - after = open(C.CONFIG_FILE).read() + with open(C.CONFIG_FILE) as f: + after = f.read() assert ret == 0 assert before == after @@ -171,7 +181,8 @@ def test_does_not_reformat(in_tmpdir, out_of_date_repo, store): f.write(config) autoupdate(Runner('.', C.CONFIG_FILE), store, tags_only=False) - after = open(C.CONFIG_FILE).read() + with open(C.CONFIG_FILE) as f: + after = f.read() expected = fmt.format(out_of_date_repo.path, out_of_date_repo.head_rev) assert after == expected @@ -200,7 +211,8 @@ def test_loses_formatting_when_not_detectable( f.write(config) autoupdate(Runner('.', C.CONFIG_FILE), store, tags_only=False) - after = open(C.CONFIG_FILE).read() + with open(C.CONFIG_FILE) as f: + after = f.read() expected = ( 'repos:\n' '- repo: {}\n' @@ -225,7 +237,8 @@ def test_autoupdate_tagged_repo(tagged_repo, in_tmpdir, store): ret = autoupdate(Runner('.', C.CONFIG_FILE), store, tags_only=False) assert ret == 0 - assert 'v1.2.3' in open(C.CONFIG_FILE).read() + with open(C.CONFIG_FILE) as f: + assert 'v1.2.3' in f.read() @pytest.fixture @@ -243,7 +256,8 @@ def test_autoupdate_tags_only(tagged_repo_with_more_commits, in_tmpdir, store): ret = autoupdate(Runner('.', C.CONFIG_FILE), store, tags_only=True) assert ret == 0 - assert 'v1.2.3' in open(C.CONFIG_FILE).read() + with open(C.CONFIG_FILE) as f: + assert 'v1.2.3' in f.read() @pytest.fixture @@ -282,9 +296,11 @@ def test_autoupdate_hook_disappearing_repo( ) write_config('.', config) - before = open(C.CONFIG_FILE).read() + with open(C.CONFIG_FILE) as f: + before = f.read() ret = autoupdate(Runner('.', C.CONFIG_FILE), store, tags_only=False) - after = open(C.CONFIG_FILE).read() + with open(C.CONFIG_FILE) as f: + after = f.read() assert ret == 1 assert before == after diff --git a/tests/commands/install_uninstall_test.py b/tests/commands/install_uninstall_test.py index e6f0e417..40d9beea 100644 --- a/tests/commands/install_uninstall_test.py +++ b/tests/commands/install_uninstall_test.py @@ -415,7 +415,8 @@ def test_replace_old_commit_script(tempdir_factory, store): runner = Runner(path, C.CONFIG_FILE) # Install a script that looks like our old script - pre_commit_contents = io.open(resource_filename('hook-tmpl')).read() + with io.open(resource_filename('hook-tmpl')) as f: + pre_commit_contents = f.read() new_contents = pre_commit_contents.replace( CURRENT_HASH, PRIOR_HASHES[-1], ) diff --git a/tests/conftest.py b/tests/conftest.py index f56bb8f4..82daccd4 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -21,6 +21,26 @@ from testing.fixtures import write_config from testing.util import cwd +@pytest.fixture(autouse=True) +def no_warnings(recwarn): + yield + warnings = [] + for warning in recwarn: # pragma: no cover + message = str(warning.message) + # ImportWarning: Not importing directory '...' missing __init__(.py) + if not ( + isinstance(warning.message, ImportWarning) + and message.startswith('Not importing directory ') + and ' missing __init__' in message + ): + warnings.append('{}:{} {}'.format( + warning.filename, + warning.lineno, + message, + )) + assert not warnings + + @pytest.fixture def tempdir_factory(tmpdir): class TmpdirFactory(object): diff --git a/tests/error_handler_test.py b/tests/error_handler_test.py index 40299b14..6aebe5a3 100644 --- a/tests/error_handler_test.py +++ b/tests/error_handler_test.py @@ -87,11 +87,11 @@ def test_log_and_exit(cap_out, mock_store_dir): ) assert os.path.exists(log_file) - contents = io.open(log_file).read() - assert contents == ( - 'msg: FatalError: hai\n' - "I'm a stacktrace\n" - ) + with io.open(log_file) as f: + assert f.read() == ( + 'msg: FatalError: hai\n' + "I'm a stacktrace\n" + ) def test_error_handler_non_ascii_exception(mock_store_dir): diff --git a/tests/staged_files_only_test.py b/tests/staged_files_only_test.py index b2af9fed..f5c14668 100644 --- a/tests/staged_files_only_test.py +++ b/tests/staged_files_only_test.py @@ -48,7 +48,8 @@ def _test_foo_state( encoding='UTF-8', ): assert os.path.exists(path.foo_filename) - assert io.open(path.foo_filename, encoding=encoding).read() == foo_contents + with io.open(path.foo_filename, encoding=encoding) as f: + assert f.read() == foo_contents actual_status = get_short_git_status()['foo'] assert status == actual_status @@ -144,10 +145,9 @@ def img_staged(tempdir_factory): def _test_img_state(path, expected_file='img1.jpg', status='A'): assert os.path.exists(path.img_filename) - assert ( - io.open(path.img_filename, 'rb').read() == - io.open(get_resource_path(expected_file), 'rb').read() - ) + with io.open(path.img_filename, 'rb') as f1,\ + io.open(get_resource_path(expected_file), 'rb') as f2: + assert f1.read() == f2.read() actual_status = get_short_git_status()['img.jpg'] assert status == actual_status