CM-55207: improve testing and cover some edge cases in commit pre-receive

Author: gotbadger

cycode/cli/files_collector/commit_range_documents.py

@@ -111,12 +111,20 @@ def calculate_pre_receive_commit_range(branch_update_details: str) -> Optional[s
     if end_commit == consts.EMPTY_COMMIT_SHA:
         return None
 
-    start_commit = _get_oldest_unupdated_commit_for_branch(end_commit)
+    repo = git_proxy.get_repo(os.getcwd())
+    start_commit = _get_oldest_unupdated_commit_for_branch(repo, end_commit)
 
     # no new commit to update found
     if not start_commit:
         return None
 
+    # If the oldest not-yet-updated commit has no parent (root commit or orphaned history),
+    # using '~1' will fail. In that case, scan from the end commit, which effectively
+    # includes the entire history reachable from it (which is exactly what we need here).
+
+    if not bool(repo.commit(start_commit).parents):
+        return f'{end_commit}'
+
     return f'{start_commit}~1...{end_commit}'
 
 
@@ -126,10 +134,10 @@ def _get_end_commit_from_branch_update_details(update_details: str) -> str:
     return end_commit
 
 
-def _get_oldest_unupdated_commit_for_branch(commit: str) -> Optional[str]:
+def _get_oldest_unupdated_commit_for_branch(repo: 'Repo', commit: str) -> Optional[str]:
     # get a list of commits by chronological order that are not in the remote repository yet
     # more info about rev-list command: https://git-scm.com/docs/git-rev-list
-    repo = git_proxy.get_repo(os.getcwd())
+
     not_updated_commits = repo.git.rev_list(commit, '--topo-order', '--reverse', '--not', '--all')
 
     commits = not_updated_commits.splitlines()

tests/cli/files_collector/test_commit_range_documents.py

@@ -11,10 +11,12 @@
 from cycode.cli import consts
 from cycode.cli.files_collector.commit_range_documents import (
     _get_default_branches_for_merge_base,
+    calculate_pre_receive_commit_range,
     calculate_pre_push_commit_range,
     get_diff_file_path,
     get_safe_head_reference_for_diff,
     parse_commit_range,
+    parse_pre_receive_input,
     parse_pre_push_input,
 )
 from cycode.cli.utils.path_utils import get_path_by_os
@@ -871,3 +873,129 @@ def test_single_commit_spec(self) -> None:
 
             parsed_from, parsed_to = parse_commit_range(a, temp_dir)
             assert (parsed_from, parsed_to) == (a, c)
+
+
+class TestParsePreReceiveInput:
+    """Test the parse_pre_receive_input function with various pre-receive hook input scenarios."""
+
+    def test_parse_single_update_input(self) -> None:
+        """Test parsing a single branch update input."""
+        pre_receive_input = '1111111111111111111111111111111111111111 2222222222222222222222222222222222222222 refs/heads/main'
+
+        with patch('sys.stdin', StringIO(pre_receive_input)):
+            result = parse_pre_receive_input()
+            assert result == pre_receive_input
+
+    def test_parse_multiple_update_input_returns_first_line(self) -> None:
+        """Test parsing multiple branch updates returns only the first line."""
+        pre_receive_input = """0000000000000000000000000000000000000000 aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa refs/heads/main
+bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb cccccccccccccccccccccccccccccccccccccccc refs/heads/feature"""
+
+        with patch('sys.stdin', StringIO(pre_receive_input)):
+            result = parse_pre_receive_input()
+            assert result == '0000000000000000000000000000000000000000 aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa refs/heads/main'
+
+    def test_parse_empty_input_raises_error(self) -> None:
+        """Test that empty input raises ValueError."""
+        with patch('sys.stdin', StringIO('')), pytest.raises(ValueError, match='Pre receive input was not found'):
+            parse_pre_receive_input()
+
+    def test_parse_whitespace_only_input_raises_error(self) -> None:
+        """Test that whitespace-only input raises ValueError."""
+        with patch('sys.stdin', StringIO('   \n\t  ')), pytest.raises(ValueError, match='Pre receive input was not found'):
+            parse_pre_receive_input()
+
+
+class TestCalculatePreReceiveCommitRange:
+    """Test the calculate_pre_receive_commit_range function with representative scenarios."""
+
+    def test_branch_deletion_returns_none(self) -> None:
+        """When end commit is all zeros (deletion), no scan is needed."""
+        update_details = f'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa {consts.EMPTY_COMMIT_SHA} refs/heads/feature'
+        assert calculate_pre_receive_commit_range(update_details) is None
+
+    def test_no_new_commits_returns_none(self, monkeypatch: pytest.MonkeyPatch) -> None:
+        """When there are no commits not in remote, return None."""
+        update_details = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb refs/heads/main'
+
+        class MockGit:
+            def rev_list(self, *args: str) -> str:
+                return ''  # no commits not in remote
+
+        class MockRepo:
+            git = MockGit()
+
+        monkeypatch.setenv('PWD', os.getcwd())
+        monkeypatch.setattr('cycode.cli.files_collector.commit_range_documents.git_proxy.get_repo', lambda path: MockRepo())
+
+        assert calculate_pre_receive_commit_range(update_details) is None
+
+    def test_returns_triple_dot_range_from_oldest_unupdated(self, monkeypatch: pytest.MonkeyPatch) -> None:
+        """Returns '<oldest>~1...<end>' when there are new commits to scan."""
+        end_sha = 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'
+        update_details = f'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa {end_sha} refs/heads/main'
+
+        class MockGit:
+            def rev_list(self, *args: str) -> str:
+                # first line is the oldest unupdated commit; others follow
+                return '1111111111111111111111111111111111111111\n2222222222222222222222222222222222222222'
+
+        class MockRepo:
+            git = MockGit()
+            def commit(self, _sha: str):
+                # Pretend it has a parent
+                return Mock(parents=['parent'])
+
+        monkeypatch.setattr('cycode.cli.files_collector.commit_range_documents.git_proxy.get_repo', lambda path: MockRepo())
+
+        result = calculate_pre_receive_commit_range(update_details)
+        assert result == '1111111111111111111111111111111111111111~1...bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'
+
+    def test_initial_oldest_commit_without_parent_returns_single_commit_range(
+        self, monkeypatch: pytest.MonkeyPatch
+    ) -> None:
+        """If oldest commit has no parent, avoid '~1' and scan from end commit only."""
+        end_sha = 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'
+        update_details = f'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa {end_sha} refs/heads/main'
+
+        class MockGit:
+            def rev_list(self, *args: str) -> str:
+                # Oldest unupdated commit is a root (no parent)
+                return '1111111111111111111111111111111111111111'
+
+        class MockRepo:
+            git = MockGit()
+            def commit(self, _sha: str):
+                return Mock(parents=[])
+
+        monkeypatch.setattr('cycode.cli.files_collector.commit_range_documents.git_proxy.get_repo', lambda path: MockRepo())
+
+        result = calculate_pre_receive_commit_range(update_details)
+        assert result == end_sha
+
+    def test_initial_oldest_commit_without_parent_with_two_commits_returns_single_commit_range(
+        self, monkeypatch: pytest.MonkeyPatch
+    ) -> None:
+        """If there are two new commits and the oldest has no parent, avoid '~1' and scan from end commit only."""
+        end_sha = 'dddddddddddddddddddddddddddddddddddddddd'
+        oldest_sha = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'
+        second_sha = 'cccccccccccccccccccccccccccccccccccccccc'
+        update_details = f'0000000000000000000000000000000000000000 {end_sha} refs/heads/main'
+
+        class MockGit:
+            def rev_list(self, *args: str) -> str:
+                # two commits not yet in remote: oldest first
+                return f'{oldest_sha}\n{second_sha}'
+
+        class MockRepo:
+            git = MockGit()
+            def commit(self, sha: str):
+                # the oldest is a root (no parent)
+                if sha == oldest_sha:
+                    return Mock(parents=[])
+                return Mock(parents=['parent'])
+
+        monkeypatch.setattr('cycode.cli.files_collector.commit_range_documents.git_proxy.get_repo', lambda path: MockRepo())
+
+        result = calculate_pre_receive_commit_range(update_details)
+        assert result == end_sha