Fix (hopefully) bug where an empty patch was generated. Closes pre-commit#76

asottile · asottile · commit f94f56addfb0 · 2014-04-13T21:22:08.000-07:00
diff --git a/pre_commit/staged_files_only.py b/pre_commit/staged_files_only.py
@@ -17,19 +17,19 @@ def staged_files_only(cmd_runner):
         cmd_runner - PrefixedCommandRunner
     """
     # Determine if there are unstaged files
-    retcode, _, _ = cmd_runner.run(
-        ['git', 'diff-files', '--quiet'],
+    retcode, diff_stdout, _ = cmd_runner.run(
+        ['git', 'diff', '--ignore-submodules', '--binary', '--exit-code'],
         retcode=None,
     )
-    if retcode:
+    if retcode and diff_stdout.strip():
         patch_filename = cmd_runner.path('patch{0}'.format(int(time.time())))
         logger.warning('Unstaged files detected.')
         logger.info(
             'Stashing unstaged files to {0}.'.format(patch_filename),
         )
         # Save the current unstaged changes as a patch
         with open(patch_filename, 'w') as patch_file:
-            cmd_runner.run(['git', 'diff', '--binary'], stdout=patch_file)
+            patch_file.write(diff_stdout)
 
         # Clear the working directory of unstaged changes
         cmd_runner.run(['git', 'checkout', '--', '.'])
diff --git a/tests/staged_files_only_test.py b/tests/staged_files_only_test.py
@@ -1,3 +1,5 @@
+import logging
+import mock
 import os.path
 import pytest
 import shutil
@@ -215,3 +217,32 @@ def test_sub_something_unstaged(sub_staged, cmd_runner):
         _test_sub_state(sub_staged, 'sha2', 'AM')
 
     _test_sub_state(sub_staged, 'sha2', 'AM')
+
+
+@pytest.yield_fixture
+def fake_logging_handler():
+    class FakeHandler(logging.Handler):
+        def __init__(self):
+            logging.Handler.__init__(self)
+            self.logs = []
+
+        def emit(self, record):
+            self.logs.append(record)
+
+    pre_commit_logger = logging.getLogger('pre_commit')
+    original_level = pre_commit_logger.getEffectiveLevel()
+    handler = FakeHandler()
+    pre_commit_logger.addHandler(handler)
+    pre_commit_logger.setLevel(logging.WARNING)
+    yield handler
+    pre_commit_logger.setLevel(original_level)
+    pre_commit_logger.removeHandler(handler)
+
+
+def test_diff_returns_1_no_diff_though(fake_logging_handler, foo_staged):
+    cmd_runner = mock.Mock()
+    cmd_runner.run.return_value = (1, '', '')
+    cmd_runner.path.return_value = '.pre-commit-files_patch'
+    with staged_files_only(cmd_runner):
+        pass
+    assert not fake_logging_handler.logs
