From a98ba2f768a35bd4135075131c2685e20571b3ba Mon Sep 17 00:00:00 2001 From: gotbadger Date: Fri, 10 Oct 2025 14:08:41 +0100 Subject: [PATCH] CM-53811: unified commit range parse --- cycode/cli/apps/scan/commit_range_scanner.py | 7 +- .../files_collector/commit_range_documents.py | 17 +---- .../test_commit_range_documents.py | 72 ++++++++++++++++++- 3 files changed, 73 insertions(+), 23 deletions(-) diff --git a/cycode/cli/apps/scan/commit_range_scanner.py b/cycode/cli/apps/scan/commit_range_scanner.py index 335531c2..eb0296c8 100644 --- a/cycode/cli/apps/scan/commit_range_scanner.py +++ b/cycode/cli/apps/scan/commit_range_scanner.py @@ -26,8 +26,7 @@ get_diff_file_path, get_pre_commit_modified_documents, get_safe_head_reference_for_diff, - parse_commit_range_sast, - parse_commit_range_sca, + parse_commit_range, ) from cycode.cli.files_collector.documents_walk_ignore import filter_documents_with_cycodeignore from cycode.cli.files_collector.file_excluder import excluder @@ -187,7 +186,7 @@ def _scan_commit_range_documents( def _scan_sca_commit_range(ctx: typer.Context, repo_path: str, commit_range: str, **_) -> None: scan_parameters = get_scan_parameters(ctx, (repo_path,)) - from_commit_rev, to_commit_rev = parse_commit_range_sca(commit_range, repo_path) + from_commit_rev, to_commit_rev = parse_commit_range(commit_range, repo_path) from_commit_documents, to_commit_documents, _ = get_commit_range_modified_documents( ctx.obj['progress_bar'], ScanProgressBarSection.PREPARE_LOCAL_FILES, repo_path, from_commit_rev, to_commit_rev ) @@ -228,7 +227,7 @@ def _scan_secret_commit_range( def _scan_sast_commit_range(ctx: typer.Context, repo_path: str, commit_range: str, **_) -> None: scan_parameters = get_scan_parameters(ctx, (repo_path,)) - from_commit_rev, to_commit_rev = parse_commit_range_sast(commit_range, repo_path) + from_commit_rev, to_commit_rev = parse_commit_range(commit_range, repo_path) _, commit_documents, diff_documents = get_commit_range_modified_documents( ctx.obj['progress_bar'], ScanProgressBarSection.PREPARE_LOCAL_FILES, diff --git a/cycode/cli/files_collector/commit_range_documents.py b/cycode/cli/files_collector/commit_range_documents.py index 3d527498..8f8eccac 100644 --- a/cycode/cli/files_collector/commit_range_documents.py +++ b/cycode/cli/files_collector/commit_range_documents.py @@ -408,22 +408,7 @@ def get_pre_commit_modified_documents( return git_head_documents, pre_committed_documents, diff_documents -def parse_commit_range_sca(commit_range: str, path: str) -> tuple[Optional[str], Optional[str]]: - # FIXME(MarshalX): i truly believe that this function does NOT work as expected - # it does not handle cases like 'A..B' correctly - # i leave it as it for SCA to not break anything - # the more correct approach is implemented for SAST - from_commit_rev = to_commit_rev = None - - for commit in git_proxy.get_repo(path).iter_commits(rev=commit_range): - if not to_commit_rev: - to_commit_rev = commit.hexsha - from_commit_rev = commit.hexsha - - return from_commit_rev, to_commit_rev - - -def parse_commit_range_sast(commit_range: str, path: str) -> tuple[Optional[str], Optional[str]]: +def parse_commit_range(commit_range: str, path: str) -> tuple[Optional[str], Optional[str]]: """Parses a git commit range string and returns the full SHAs for the 'from' and 'to' commits. Supports: diff --git a/tests/cli/files_collector/test_commit_range_documents.py b/tests/cli/files_collector/test_commit_range_documents.py index 568b1bec..cd30d1eb 100644 --- a/tests/cli/files_collector/test_commit_range_documents.py +++ b/tests/cli/files_collector/test_commit_range_documents.py @@ -14,6 +14,7 @@ calculate_pre_push_commit_range, get_diff_file_path, get_safe_head_reference_for_diff, + parse_commit_range, parse_pre_push_input, ) from cycode.cli.utils.path_utils import get_path_by_os @@ -22,7 +23,8 @@ @contextmanager def git_repository(path: str) -> Generator[Repo, None, None]: """Context manager for Git repositories that ensures proper cleanup on Windows.""" - repo = Repo.init(path) + # Ensure the initialized repository uses 'main' as the default branch + repo = Repo.init(path, b='main') try: yield repo finally: @@ -539,8 +541,8 @@ def test_calculate_range_for_new_branch_with_merge_base(self) -> None: repo.index.add(['feature.py']) feature_commit = repo.index.commit('Add feature') - # Switch back to master to simulate we're pushing a feature branch - repo.heads.master.checkout() + # Switch back to the default branch to simulate pushing the feature branch + repo.heads.main.checkout() # Test new branch push push_details = f'refs/heads/feature {feature_commit.hexsha} refs/heads/feature {consts.EMPTY_COMMIT_SHA}' @@ -805,3 +807,67 @@ def test_simulate_pre_push_hook_input_format(self) -> None: parts = push_input.split() expected_range = f'{parts[3]}..{parts[1]}' assert commit_range == expected_range + + +class TestParseCommitRange: + """Tests to validate unified parse_commit_range behavior matches git semantics.""" + + def _make_linear_history(self, repo: Repo, base_dir: str) -> tuple[str, str, str]: + """Create three linear commits A -> B -> C and return their SHAs.""" + a_file = os.path.join(base_dir, 'a.txt') + with open(a_file, 'w') as f: + f.write('A') + repo.index.add(['a.txt']) + a = repo.index.commit('A') + + with open(a_file, 'a') as f: + f.write('B') + repo.index.add(['a.txt']) + b = repo.index.commit('B') + + with open(a_file, 'a') as f: + f.write('C') + repo.index.add(['a.txt']) + c = repo.index.commit('C') + + return a.hexsha, b.hexsha, c.hexsha + + def test_two_dot_linear_history(self) -> None: + """For 'A..C', expect (A,C) in linear history.""" + with temporary_git_repository() as (temp_dir, repo): + a, b, c = self._make_linear_history(repo, temp_dir) + + parsed_from, parsed_to = parse_commit_range(f'{a}..{c}', temp_dir) + assert (parsed_from, parsed_to) == (a, c) + + def test_three_dot_linear_history(self) -> None: + """For 'A...C' in linear history, expect (A,C).""" + with temporary_git_repository() as (temp_dir, repo): + a, b, c = self._make_linear_history(repo, temp_dir) + + parsed_from, parsed_to = parse_commit_range(f'{a}...{c}', temp_dir) + assert (parsed_from, parsed_to) == (a, c) + + def test_open_right_linear_history(self) -> None: + """For 'A..', expect (A,HEAD=C).""" + with temporary_git_repository() as (temp_dir, repo): + a, b, c = self._make_linear_history(repo, temp_dir) + + parsed_from, parsed_to = parse_commit_range(f'{a}..', temp_dir) + assert (parsed_from, parsed_to) == (a, c) + + def test_open_left_linear_history(self) -> None: + """For '..C' where HEAD==C, expect (HEAD=C,C).""" + with temporary_git_repository() as (temp_dir, repo): + a, b, c = self._make_linear_history(repo, temp_dir) + + parsed_from, parsed_to = parse_commit_range(f'..{c}', temp_dir) + assert (parsed_from, parsed_to) == (c, c) + + def test_single_commit_spec(self) -> None: + """For 'A', expect (A,HEAD=C).""" + with temporary_git_repository() as (temp_dir, repo): + a, b, c = self._make_linear_history(repo, temp_dir) + + parsed_from, parsed_to = parse_commit_range(a, temp_dir) + assert (parsed_from, parsed_to) == (a, c)