Merge pull request #195 from pre-commit/actually_print_when_installing

Actually print while installing

pre_commit/repository.py

@@ -1,5 +1,6 @@
 from __future__ import unicode_literals
 
+import logging
 import shutil
 
 from cached_property import cached_property
@@ -9,6 +10,9 @@ from pre_commit.manifest import Manifest
 from pre_commit.prefixed_command_runner import PrefixedCommandRunner
 
 
+logger = logging.getLogger('pre_commit')
+
+
 class Repository(object):
     def __init__(self, repo_config, repo_path_getter):
         self.repo_config = repo_config
@@ -62,14 +66,28 @@ class Repository(object):
 
     def install(self):
         """Install the hook repository."""
-        for language_name, language_version in self.languages:
+        def language_is_installed(language_name):
             language = languages[language_name]
-            if (
+            return (
                 language.ENVIRONMENT_DIR is None or
                 self.cmd_runner.exists(language.ENVIRONMENT_DIR, '.installed')
-            ):
-                # The language is already installed
+            )
+
+        if not all(
+            language_is_installed(language_name)
+            for language_name, _ in self.languages
+        ):
+            logger.info(
+                'Installing environment for {0}.'.format(self.repo_url)
+            )
+            logger.info('Once installed this environment will be reused.')
+            logger.info('This may take a few minutes...')
+
+        for language_name, language_version in self.languages:
+            language = languages[language_name]
+            if language_is_installed(language_name):
                 continue
+
             # There's potentially incomplete cleanup from previous runs
             # Clean it up!
             if self.cmd_runner.exists(language.ENVIRONMENT_DIR):

pre_commit/store.py

@@ -81,9 +81,7 @@ class Store(object):
         if os.path.exists(sha_path):
             return os.readlink(sha_path)
 
-        logger.info('Installing environment for {0}.'.format(url))
-        logger.info('Once installed this environment will be reused.')
-        logger.info('This may take a few minutes...')
+        logger.info('Initializing environment for {0}.'.format(url))
 
         dir = tempfile.mkdtemp(prefix='repo', dir=self.directory)
         with clean_path_on_failure(dir):

tests/commands/install_uninstall_test.py

@@ -128,9 +128,7 @@ FILES_CHANGED = (
 
 
 NORMAL_PRE_COMMIT_RUN = re.compile(
-    r'^\[INFO\] Installing environment for .+\.\n'
-    r'\[INFO\] Once installed this environment will be reused\.\n'
-    r'\[INFO\] This may take a few minutes\.\.\.\n'
+    r'^\[INFO\] Initializing environment for .+\.\n'
     r'Bash hook\.+Passed\n'
     r'\[master [a-f0-9]{7}\] Commit!\n' +
     FILES_CHANGED +
@@ -180,9 +178,7 @@ def test_environment_not_sourced(tmpdir_factory):
 
 
 FAILING_PRE_COMMIT_RUN = re.compile(
-    r'^\[INFO\] Installing environment for .+\.\n'
-    r'\[INFO\] Once installed this environment will be reused\.\n'
-    r'\[INFO\] This may take a few minutes\.\.\.\n'
+    r'^\[INFO\] Initializing environment for .+\.\n'
     r'Failing hook\.+Failed\n'
     r'hookid: failing_hook\n'
     r'\n'
@@ -258,9 +254,7 @@ def test_install_existing_hook_no_overwrite_idempotent(tmpdir_factory):
 
 FAIL_OLD_HOOK = re.compile(
     r'fail!\n'
-    r'\[INFO\] Installing environment for .+\.\n'
-    r'\[INFO\] Once installed this environment will be reused\.\n'
-    r'\[INFO\] This may take a few minutes\.\.\.\n'
+    r'\[INFO\] Initializing environment for .+\.\n'
     r'Bash hook\.+Passed\n'
 )
 

tests/conftest.py

@@ -2,6 +2,7 @@ from __future__ import absolute_import
 from __future__ import unicode_literals
 
 import io
+import logging
 import os
 import os.path
 
@@ -113,3 +114,9 @@ def cmd_runner(tmpdir_factory):
 @pytest.yield_fixture
 def runner_with_mocked_store(mock_out_store_directory):
     yield Runner('/')
+
+
+@pytest.yield_fixture
+def log_info_mock():
+    with mock.patch.object(logging.getLogger('pre_commit'), 'info') as mck:
+        yield mck

tests/repository_test.py

@@ -254,18 +254,21 @@ def test_languages(tmpdir_factory, store):
     assert repo.languages == set([('python', 'default')])
 
 
-def test_reinstall(tmpdir_factory, store):
+def test_reinstall(tmpdir_factory, store, log_info_mock):
     path = make_repo(tmpdir_factory, 'python_hooks_repo')
     config = make_config_from_repo(path)
     repo = Repository.create(config, store)
     repo.require_installed()
+    # We print some logging during clone (1) + install (3)
+    assert log_info_mock.call_count == 4
+    log_info_mock.reset_mock()
     # Reinstall with same repo should not trigger another install
-    # TODO: how to assert this?
     repo.require_installed()
+    assert log_info_mock.call_count == 0
     # Reinstall on another run should not trigger another install
-    # TODO: how to assert this?
     repo = Repository.create(config, store)
     repo.require_installed()
+    assert log_info_mock.call_count == 0
 
 
 def test_control_c_control_c_on_install(tmpdir_factory, store):

tests/store_test.py

@@ -11,7 +11,6 @@ import pytest
 
 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 pre_commit.util import cmd_output
 from pre_commit.util import cwd
@@ -80,12 +79,6 @@ def test_does_not_recreate_if_directory_already_exists(store):
     assert not os.path.exists(os.path.join(store.directory, 'README'))
 
 
-@pytest.yield_fixture
-def log_info_mock():
-    with mock.patch.object(logger, 'info', autospec=True) as info_mock:
-        yield info_mock
-
-
 def test_clone(store, tmpdir_factory, log_info_mock):
     path = git_dir(tmpdir_factory)
     with cwd(path):
@@ -95,7 +88,9 @@ def test_clone(store, tmpdir_factory, log_info_mock):
 
     ret = store.clone(path, sha)
     # Should have printed some stuff
-    log_info_mock.assert_called_with('This may take a few minutes...')
+    assert log_info_mock.call_args_list[0][0][0].startswith(
+        'Initializing environment for '
+    )
 
     # Should return a directory inside of the store
     assert os.path.exists(ret)