From eefc46f901871c75770bb58c13f6cc063dacbfed Mon Sep 17 00:00:00 2001 From: Alexander Dupuy Date: Thu, 7 May 2015 18:20:42 +0200 Subject: [PATCH 1/2] Don't report markup titles as merge conflicts Several markup formats, such as Markdown or Re(Structured)Text can format titles as text with '=' characters as double underlining, like this: ``` My Page Title ============= Lorem ipsum... ``` Rather that considering any line starting with seven '=' as a conflict marker, require a space (or line-ending newline) after the equals. This could still create a false positive for a seven character title, like "Problem", but the markup formats generally allow extra '=' characters, so by formatting the text like this: ``` Problem ======== Not... ``` these pre-commit warnings can be avoided. Also updates the tests to add newlines for more realistic conflict files (while a file might not end with a newline, conflict markers will). Prevent false negative on test_does_not_care_when_not_in_a_conflict() by making sure that README.md contains a line identical to a conflict string (exactly seven '=' followed by a newline). --- pre_commit_hooks/check_merge_conflict.py | 3 ++- tests/check_merge_conflict_test.py | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/pre_commit_hooks/check_merge_conflict.py b/pre_commit_hooks/check_merge_conflict.py index 5e274a3f..f8bcadfc 100644 --- a/pre_commit_hooks/check_merge_conflict.py +++ b/pre_commit_hooks/check_merge_conflict.py @@ -6,7 +6,8 @@ CONFLICT_PATTERNS = [ '<<<<<<< ', - '=======', + '======= ', + '=======\n', '>>>>>>> ' ] WARNING_MSG = 'Merge conflict string "{0}" found in {1}:{2}' diff --git a/tests/check_merge_conflict_test.py b/tests/check_merge_conflict_test.py index f3bd935f..7e31313d 100644 --- a/tests/check_merge_conflict_test.py +++ b/tests/check_merge_conflict_test.py @@ -47,7 +47,7 @@ def f1_is_a_conflict_file(in_tmpdir): @pytest.mark.parametrize( - 'failing_contents', ('<<<<<<< HEAD', '=======', '>>>>>>> master'), + 'failing_contents', ('<<<<<<< HEAD\n', '=======\n', '>>>>>>> master\n'), ) @pytest.mark.usefixtures('f1_is_a_conflict_file') def test_merge_conflicts_failing(failing_contents): @@ -56,7 +56,7 @@ def test_merge_conflicts_failing(failing_contents): @pytest.mark.parametrize( - 'ok_contents', ('# <<<<<<< HEAD', '# =======', 'import my_module', ''), + 'ok_contents', ('# <<<<<<< HEAD\n', '# =======\n', 'import my_module', ''), ) @pytest.mark.usefixtures('f1_is_a_conflict_file') def test_merge_conflicts_ok(ok_contents): @@ -67,5 +67,5 @@ def test_merge_conflicts_ok(ok_contents): @pytest.mark.usefixtures('in_tmpdir') def test_does_not_care_when_not_in_a_conflict(): with io.open('README.md', 'w') as readme_file: - readme_file.write('pre-commit\n=================\n') + readme_file.write('problem\n=======\n') assert detect_merge_conflict(['README.md']) == 0 From 5c752935fd1065f4144dd1dc9164404369659bac Mon Sep 17 00:00:00 2001 From: Alexander Dupuy Date: Thu, 7 May 2015 23:04:15 +0200 Subject: [PATCH 2/2] Refactor check-merge-conflicts tests Do a straight test of detecting a real merge conflict as generated by git. Test artificial conflict detection while pending merge without a real conflict. Test artificial non-conflict non-detection in a resolved merge conflict. Rename test_does_not_care... function to reflect what we want to care about. Rename is_in_merge_conflict to is_in_merge since that is what it checks. --- pre_commit_hooks/check_merge_conflict.py | 4 +-- tests/check_merge_conflict_test.py | 44 +++++++++++++++++++++--- 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/pre_commit_hooks/check_merge_conflict.py b/pre_commit_hooks/check_merge_conflict.py index f8bcadfc..ec094734 100644 --- a/pre_commit_hooks/check_merge_conflict.py +++ b/pre_commit_hooks/check_merge_conflict.py @@ -13,7 +13,7 @@ WARNING_MSG = 'Merge conflict string "{0}" found in {1}:{2}' -def is_in_merge_conflict(): +def is_in_merge(): return ( os.path.exists(os.path.join('.git', 'MERGE_MSG')) and os.path.exists(os.path.join('.git', 'MERGE_HEAD')) @@ -25,7 +25,7 @@ def detect_merge_conflict(argv=None): parser.add_argument('filenames', nargs='*') args = parser.parse_args(argv) - if not is_in_merge_conflict(): + if not is_in_merge(): return 0 retcode = 0 diff --git a/tests/check_merge_conflict_test.py b/tests/check_merge_conflict_test.py index 7e31313d..3d719bd3 100644 --- a/tests/check_merge_conflict_test.py +++ b/tests/check_merge_conflict_test.py @@ -2,6 +2,7 @@ from __future__ import unicode_literals import io +import os import pytest @@ -43,16 +44,51 @@ def f1_is_a_conflict_file(in_tmpdir): 'parent\n' '>>>>>>>' ) + assert os.path.exists(os.path.join('.git', 'MERGE_MSG')) yield +@pytest.yield_fixture +def repository_is_pending_merge(in_tmpdir): + # Make a (non-conflicting) merge + cmd_output('git', 'init', 'repo1') + with cwd('repo1'): + io.open('f1', 'w').close() + cmd_output('git', 'add', 'f1') + cmd_output('git', 'commit', '-m' 'commit1') + + cmd_output('git', 'clone', 'repo1', 'repo2') + + # Commit in master + with cwd('repo1'): + write_file('f1', 'parent\n') + cmd_output('git', 'commit', '-am', 'master commit2') + + # Commit in clone and pull without committing + with cwd('repo2'): + write_file('f2', 'child\n') + cmd_output('git', 'add', 'f2') + cmd_output('git', 'commit', '-m', 'clone commit2') + cmd_output('git', 'pull', '--no-commit') + # We should end up in a pending merge + assert io.open('f1').read().startswith('parent\n') + assert io.open('f2').read().startswith('child\n') + assert os.path.exists(os.path.join('.git', 'MERGE_HEAD')) + yield + + +@pytest.mark.usefixtures('f1_is_a_conflict_file') +def test_merge_conflicts_git(): + assert detect_merge_conflict(['f1']) == 1 + + @pytest.mark.parametrize( 'failing_contents', ('<<<<<<< HEAD\n', '=======\n', '>>>>>>> master\n'), ) -@pytest.mark.usefixtures('f1_is_a_conflict_file') +@pytest.mark.usefixtures('repository_is_pending_merge') def test_merge_conflicts_failing(failing_contents): - write_file('f1', failing_contents) - assert detect_merge_conflict(['f1']) == 1 + write_file('f2', failing_contents) + assert detect_merge_conflict(['f2']) == 1 @pytest.mark.parametrize( @@ -65,7 +101,7 @@ def test_merge_conflicts_ok(ok_contents): @pytest.mark.usefixtures('in_tmpdir') -def test_does_not_care_when_not_in_a_conflict(): +def test_does_not_care_when_not_in_a_merge(): with io.open('README.md', 'w') as readme_file: readme_file.write('problem\n=======\n') assert detect_merge_conflict(['README.md']) == 0