From d0b8edd24c16693d89cbf8940cdc94c8c016505f Mon Sep 17 00:00:00 2001 From: "Myeongjin (Daniel)" Date: Fri, 30 Oct 2026 18:57:45 -0400 Subject: [PATCH] Polish log output and CLI errors --- README.md | 18 ++- minigit/cli.py | 9 +- minigit/commits.py | 183 +++++++++++++++++++------ minigit/errors.py | 8 ++ tests/test_cli.py | 227 ++++++++++++++++++++++++++++++++ tests/test_commits.py | 199 +++++++++++++++++++++++++++- tests/test_week3_integration.py | 2 +- 7 files changed, 600 insertions(+), 46 deletions(-) create mode 100644 tests/test_cli.py diff --git a/README.md b/README.md index f018a56..a664959 100644 --- a/README.md +++ b/README.md @@ -50,13 +50,15 @@ minigit init printf 'A\n' > file.txt minigit add file.txt minigit commit -m "A" +minigit log # full ID, author, timezone-explicit date, message +minigit log --oneline # compact ID and first message line minigit branch feature minigit checkout feature printf 'B\n' > file.txt minigit add file.txt minigit commit -m "B" minigit checkout main # file.txt contains A -minigit merge feature # fast-forward; file.txt contains B +minigit merge feature # reports a fast-forward; file.txt contains B minigit status # clean ``` @@ -70,6 +72,20 @@ unchanged. Edit each conflicted file, run `minigit add `, then add its path. Pending conflicts survive new CLI processes and block another merge, checkout, or commit until explicitly resolved. +`minigit log` follows both parents of merge commits. It prints the requested +tip first, then visits parents in their stored order, and prints shared +ancestors once. This ordering is deterministic and never uses commit +timestamps to decide ancestry. Detailed output includes full commit and parent +IDs; use `--oneline` for compact summaries. `log` and branch listing are +read-only, including while a merge is pending. + +Branch, checkout, and merge commands distinguish created/already-current, +up-to-date, fast-forward, clean-merge, and conflict outcomes. Expected failures +use a concise `minigit: ...` message on stderr and a nonzero exit code; argument +usage errors keep argparse's usage output and exit code 2. After a conflict, +resolve each path, run `minigit add `, then run +`minigit commit -m "Resolve merge"`. + `minigit diff` shows line changes between HEAD and working files, including untracked files, binary changes, executable modes, and missing final newlines. `minigit status` retains its staged / not staged / untracked sections. Both diff --git a/minigit/cli.py b/minigit/cli.py index a554979..a053dc4 100644 --- a/minigit/cli.py +++ b/minigit/cli.py @@ -13,7 +13,7 @@ from pathlib import Path from minigit import __version__, commits -from minigit.errors import MiniGitError +from minigit.errors import MergeConflictError, MiniGitError from minigit.index import register_index_commands from minigit.objects import register_subcommands as register_object_commands from minigit.remote import register_subcommands as register_remote_commands @@ -89,6 +89,13 @@ def main(argv: Sequence[str] | None = None) -> int: try: return handler(args) + except MergeConflictError as exc: + print(f"minigit: {exc}", file=sys.stderr) + print( + "resolve conflicts, run 'minigit add ', then 'minigit commit -m '", + file=sys.stderr, + ) + return 1 except MiniGitError as exc: print(f"minigit: {exc}", file=sys.stderr) return 1 diff --git a/minigit/commits.py b/minigit/commits.py index 1448ebd..435ecda 100644 --- a/minigit/commits.py +++ b/minigit/commits.py @@ -13,6 +13,7 @@ import tempfile import time from dataclasses import dataclass +from datetime import UTC, datetime from minigit.errors import ( MergeConflictError, @@ -64,6 +65,7 @@ def _cmd_branch(args) -> int: if commit_hash is None: raise MiniGitError(f"branch '{args.name}': '{current}' has no commits yet") manager.create_branch(args.name, commit_hash) + print(f"Created branch '{args.name}' at {commit_hash[:7]}.") else: for branch in manager.list_branches(): if branch == current: @@ -77,12 +79,12 @@ def _cmd_branch(args) -> int: def _cmd_log(args) -> int: """Handle the log command""" m = CommitManager() - lines = m.log() + lines = m.log(oneline=args.oneline) if not lines: print("no commits yet") return 0 - for line in lines: - print(line) + separator = "\n" if args.oneline else "\n\n" + print(separator.join(lines)) return 0 @@ -91,15 +93,25 @@ def _cmd_checkout(args) -> int: Handle 'minigit checkout command' """ manager = CommitManager() + already_current = manager._current_branch() == args.name manager.switch_branch(args.name) - print(f"Switched to branch '{args.name}'") + if already_current: + print(f"Already on branch '{args.name}'.") + else: + print(f"Switched to branch '{args.name}'.") return 0 def _cmd_merge(args) -> int: manager = CommitManager() - manager.merge(args.branch) - print(f"Merged '{args.branch}' into '{manager._current_branch()}'") + current = manager._current_branch() + result = manager.merge_with_result(args.branch) + if result.outcome == "up-to-date": + print("Already up to date.") + elif result.outcome == "fast-forward": + print(f"Fast-forwarded '{current}' to '{args.branch}'.") + else: + print(f"Merged '{args.branch}' into '{current}' as {result.commit_hash[:7]}.") return 0 @@ -120,6 +132,7 @@ def register_subcommands(subparsers) -> None: checkout_parser.set_defaults(handler=_cmd_checkout) log_parser = subparsers.add_parser("log", help="show commit history") + log_parser.add_argument("--oneline", action="store_true", help="show compact summaries") log_parser.set_defaults(handler=_cmd_log) merge_parser = subparsers.add_parser("merge", help="merge a branch into the current branch") @@ -128,12 +141,63 @@ def register_subcommands(subparsers) -> None: class CommitData: - def __init__(self, tree: str, parents: list[str], author: str, committer: str, message: str): + def __init__( + self, + tree: str, + parents: list[str], + author: str, + committer: str, + message: str, + author_timestamp: int = 0, + committer_timestamp: int = 0, + ): self.tree = tree self.parents = parents self.author = author self.committer = committer self.message = message + self.author_timestamp = author_timestamp + self.committer_timestamp = committer_timestamp + + +@dataclass(frozen=True) +class MergeResult: + """Outcome details for callers that need more than merge()'s legacy return value.""" + + outcome: str + commit_hash: str | None = None + + +@dataclass(frozen=True) +class MergeStatus: + """Read-only view of a pending merge owned by Module 3.""" + + ours: str + theirs: str + message: str + unresolved_paths: tuple[str, ...] + + +def format_commit_summary(commit_hash: str, commit: CommitData) -> str: + """Return the compact, single-line presentation of a commit.""" + summary = commit.message.splitlines()[0] if commit.message else "" + return f"{commit_hash[:7]} {summary}" + + +def format_commit_details(commit_hash: str, commit: CommitData) -> str: + """Return the detailed presentation of a commit without reading repository state.""" + lines = [f"commit {commit_hash}"] + if commit.parents: + lines.append(f"Parents: {' '.join(commit.parents)}") + lines.extend( + [ + f"Author: {commit.author}", + f"Date: {datetime.fromtimestamp(commit.author_timestamp, UTC).isoformat()}", + "", + ] + ) + lines.extend(f" {line}" for line in commit.message.split("\n")) + return "\n".join(lines) class CommitManager: @@ -181,8 +245,15 @@ def read_ref(self, name: str) -> str | None: ref_path = self._ref_path(name) if not os.path.exists(ref_path): return None - with open(ref_path) as f: - return f.read().strip() + try: + with open(ref_path, encoding="utf-8") as f: + commit_hash = f.read().strip() + except (OSError, UnicodeDecodeError) as error: + raise MiniGitError(f"cannot read ref '{name}'") from error + if len(commit_hash) != 40 or any(c not in "0123456789abcdef" for c in commit_hash): + display = commit_hash or "" + raise MiniGitError(f"invalid ref '{name}': {display}") + return commit_hash @staticmethod def validate_branch_name(name: str) -> None: @@ -308,7 +379,7 @@ def switch_branch(self, name) -> None: if self._read_pending_merge() is not None: raise MiniGitError("merge in progress") if not os.path.exists(self._ref_path(name)): - raise RefNotFoundError(name) + raise RefNotFoundError(f"branch not found: {name}") self._require_no_staged_changes() commit = self.read_commit(self.read_ref(name)) self.tree.checkout(commit.tree) @@ -334,27 +405,36 @@ def read_commit(self, commit_hash: str) -> CommitData: i += 1 if not lines[i].startswith("author "): raise ValueError("missing author line") - author = lines[i][len("author ") :].rsplit(" ", 1)[0] + author, author_timestamp_text = lines[i][len("author ") :].rsplit(" ", 1) i += 1 if not lines[i].startswith("committer "): raise ValueError("missing committer line") - committer = lines[i][len("committer ") :].rsplit(" ", 1)[0] + committer, committer_timestamp_text = lines[i][len("committer ") :].rsplit(" ", 1) if i != len(lines) - 1: raise ValueError("unexpected commit headers") for object_hash in [tree, *parents]: if len(object_hash) != 40 or any(c not in "0123456789abcdef" for c in object_hash): raise ValueError("invalid object hash") - for identity_line in lines[-2:]: - identity, timestamp = identity_line.split(" ", 1)[1].rsplit(" ", 1) - if not identity.strip(): - raise ValueError("missing identity") - int(timestamp) - - except (UnicodeDecodeError, IndexError, ValueError) as error: + if not author.strip() or not committer.strip(): + raise ValueError("missing identity") + author_timestamp = int(author_timestamp_text) + committer_timestamp = int(committer_timestamp_text) + datetime.fromtimestamp(author_timestamp, UTC) + datetime.fromtimestamp(committer_timestamp, UTC) + + except (OSError, OverflowError, UnicodeDecodeError, IndexError, ValueError) as error: raise ObjectCorruptError(commit_hash) from error - return CommitData(tree, parents, author, committer, message) + return CommitData( + tree, + parents, + author, + committer, + message, + author_timestamp, + committer_timestamp, + ) def get_head_tree(self) -> str | None: head = self._current_branch() @@ -368,6 +448,16 @@ def get_head_tree(self) -> str | None: return self.read_commit(commit_hash).tree def walk_history(self, start_hash: str) -> list[str]: + """Return reachable hashes in pre-order, following stored parent order.""" + return [commit_hash for commit_hash, _ in self._walk_history_data(start_hash)] + + def _walk_history_data(self, start_hash: str) -> list[tuple[str, CommitData]]: + """Return each reachable commit and its parsed data exactly once. + + Ordering is deterministic pre-order depth-first traversal: a commit is + emitted before its parents, and parents are visited in their stored + order. Timestamps do not participate in traversal. + """ visited = [] seen = set() stack = [start_hash] @@ -377,10 +467,9 @@ def walk_history(self, start_hash: str) -> list[str]: if commit_hash in seen: continue seen.add(commit_hash) - visited.append(commit_hash) - - parents = self.read_commit(commit_hash).parents - stack.extend(reversed(parents)) + commit = self.read_commit(commit_hash) + visited.append((commit_hash, commit)) + stack.extend(reversed(commit.parents)) return visited @@ -409,7 +498,7 @@ def create_commit(self, tree_hash, parents, author, message) -> str: def create_branch(self, name, commit_hash) -> None: """Create a new branch that points at commit_hash""" if os.path.exists(self._ref_path(name)): - raise RefExistsError(name) + raise RefExistsError(f"branch already exists: {name}") self.write_ref(name, commit_hash) def list_branches(self) -> list[str]: @@ -436,10 +525,19 @@ def merge(self, branch_name) -> str | None: Fast-forward and no-op cases return None. A clean three-way merge returns the new commit hash. A text conflict raises MergeConflictError. """ + result = self.merge_with_result(branch_name) + return result.commit_hash if result.outcome == "merged" else None + + def merge_with_result(self, branch_name: str) -> MergeResult: + """Merge a branch and describe whether it was current, advanced, or merged. + + This additive helper leaves merge()'s established return contract intact. + Text conflicts still raise MergeConflictError with its paths preserved. + """ if self._read_pending_merge() is not None: raise MiniGitError("merge in progress") if not os.path.exists(self._ref_path(branch_name)): - raise RefNotFoundError(branch_name) + raise RefNotFoundError(f"branch not found: {branch_name}") target = self.read_ref(branch_name) current = self._current_branch() @@ -448,14 +546,15 @@ def merge(self, branch_name) -> str | None: if current_hash is not None and ( target == current_hash or self.is_ancestor(target, current_hash) ): - return None + return MergeResult("up-to-date") if current_hash is None or self.is_ancestor(current_hash, target): self._require_no_staged_changes() self.tree.checkout(self.read_commit(target).tree) self.write_ref(current, target) - return None + return MergeResult("fast-forward", target) - return self._three_way_merge(current_hash, target, branch_name) + commit_hash = self._three_way_merge(current_hash, target, branch_name) + return MergeResult("merged", commit_hash) def integrate_fetched_tip(self, branch_name: str, commit_hash: str) -> str | None: """Integrate one fetched commit into a named local branch. @@ -742,28 +841,30 @@ def read(path: str) -> str: unresolved, ) + def get_merge_status(self) -> MergeStatus | None: + """Return an immutable snapshot of pending merge state for read-only callers.""" + pending = self._read_pending_merge() + if pending is None: + return None + ours, theirs, message, unresolved = pending + return MergeStatus(ours, theirs, message, tuple(unresolved)) + def _clear_pending_merge(self) -> None: """Delete pending-merge files after a completed merge commit.""" for path in self._merge_state_paths().values(): if os.path.exists(path): os.remove(path) - def log(self) -> list[str]: - """Return one summary line per commit reachable from HEAD, newest first""" + def log(self, oneline: bool = False) -> list[str]: + """Return formatted commits reachable from HEAD in deterministic graph order.""" commit_hash = self.read_ref(self._current_branch()) if not commit_hash: return [] - lines = [] - for i in self.walk_history(commit_hash): - msg = self.read_commit(i).message - if msg: - summary = msg.splitlines()[0] - else: - summary = "" - lines.append(f"{i[:7]} {summary}") - - return lines + formatter = format_commit_summary if oneline else format_commit_details + return [ + formatter(obj_hash, commit) for obj_hash, commit in self._walk_history_data(commit_hash) + ] @dataclass diff --git a/minigit/errors.py b/minigit/errors.py index 3ffd5e6..3cf1a6c 100644 --- a/minigit/errors.py +++ b/minigit/errors.py @@ -13,10 +13,18 @@ class MiniGitError(Exception): class ObjectNotFoundError(MiniGitError): """No object with the requested hash exists in the object store.""" + def __init__(self, object_hash: str) -> None: + self.object_hash = object_hash + super().__init__(f"object not found: {object_hash}") + class ObjectCorruptError(MiniGitError): """An object was found, but its bytes do not match its hash.""" + def __init__(self, object_hash: str) -> None: + self.object_hash = object_hash + super().__init__(f"object corrupt: {object_hash}") + class RefNotFoundError(MiniGitError): """A ref (branch, HEAD target, tag) does not exist.""" diff --git a/tests/test_cli.py b/tests/test_cli.py new file mode 100644 index 0000000..c9bbf36 --- /dev/null +++ b/tests/test_cli.py @@ -0,0 +1,227 @@ +import subprocess +import sys + +from minigit.commits import CommitManager + + +def run_cli(repo, *args): + return subprocess.run( + [sys.executable, "-m", "minigit.cli", *args], + cwd=repo, + text=True, + capture_output=True, + check=False, + ) + + +def init_repo(repo): + result = run_cli(repo, "init") + assert result.returncode == 0 + assert result.stderr == "" + + +def commit_file(repo, content, message): + (repo / "file.txt").write_text(content) + assert run_cli(repo, "add", "file.txt").returncode == 0 + result = run_cli(repo, "commit", "-m", message) + assert result.returncode == 0 + assert result.stderr == "" + return CommitManager(repo).read_ref("main") + + +def test_log_subprocess_detailed_oneline_and_unborn(tmp_path): + init_repo(tmp_path) + unborn = run_cli(tmp_path, "log") + assert (unborn.returncode, unborn.stdout, unborn.stderr) == (0, "no commits yet\n", "") + commit_hash = commit_file(tmp_path, "hello\n", "héllo\nbody") + + detailed = run_cli(tmp_path, "log") + assert detailed.returncode == 0 + assert detailed.stderr == "" + assert f"commit {commit_hash}\n" in detailed.stdout + assert "Author: minigit \n" in detailed.stdout + assert "Date: " in detailed.stdout and "+00:00\n" in detailed.stdout + assert " héllo\n body\n" in detailed.stdout + + compact = run_cli(tmp_path, "log", "--oneline") + assert (compact.returncode, compact.stdout, compact.stderr) == ( + 0, + f"{commit_hash[:7]} héllo\n", + "", + ) + + +def test_branch_and_checkout_subprocess_messages_and_errors(tmp_path): + init_repo(tmp_path) + tip = commit_file(tmp_path, "base\n", "base") + + created = run_cli(tmp_path, "branch", "feature") + assert (created.returncode, created.stdout, created.stderr) == ( + 0, + f"Created branch 'feature' at {tip[:7]}.\n", + "", + ) + listing = run_cli(tmp_path, "branch") + assert (listing.returncode, listing.stdout, listing.stderr) == ( + 0, + " feature\n* main\n", + "", + ) + + existing = run_cli(tmp_path, "branch", "feature") + assert existing.returncode == 1 + assert existing.stdout == "" + assert existing.stderr == "minigit: branch already exists: feature\n" + + same = run_cli(tmp_path, "checkout", "main") + assert (same.returncode, same.stdout, same.stderr) == ( + 0, + "Already on branch 'main'.\n", + "", + ) + switched = run_cli(tmp_path, "checkout", "feature") + assert (switched.returncode, switched.stdout, switched.stderr) == ( + 0, + "Switched to branch 'feature'.\n", + "", + ) + + missing = run_cli(tmp_path, "checkout", "missing") + assert missing.returncode == 1 + assert missing.stdout == "" + assert missing.stderr == "minigit: branch not found: missing\n" + + invalid = run_cli(tmp_path, "branch", "../escape") + assert invalid.returncode == 1 + assert invalid.stdout == "" + assert "invalid branch name" in invalid.stderr + assert "Traceback" not in invalid.stderr + + +def test_merge_subprocess_reports_fast_forward_and_up_to_date(tmp_path): + init_repo(tmp_path) + commit_file(tmp_path, "base\n", "base") + assert run_cli(tmp_path, "branch", "feature").returncode == 0 + assert run_cli(tmp_path, "checkout", "feature").returncode == 0 + (tmp_path / "file.txt").write_text("feature\n") + assert run_cli(tmp_path, "add", "file.txt").returncode == 0 + assert run_cli(tmp_path, "commit", "-m", "feature").returncode == 0 + assert run_cli(tmp_path, "checkout", "main").returncode == 0 + + advanced = run_cli(tmp_path, "merge", "feature") + assert (advanced.returncode, advanced.stdout, advanced.stderr) == ( + 0, + "Fast-forwarded 'main' to 'feature'.\n", + "", + ) + current = run_cli(tmp_path, "merge", "feature") + assert (current.returncode, current.stdout, current.stderr) == ( + 0, + "Already up to date.\n", + "", + ) + + +def test_merge_conflict_subprocess_preserves_paths_and_explains_recovery(tmp_path): + init_repo(tmp_path) + commit_file(tmp_path, "base\n", "base") + assert run_cli(tmp_path, "branch", "feature").returncode == 0 + commit_file(tmp_path, "main\n", "main") + assert run_cli(tmp_path, "checkout", "feature").returncode == 0 + (tmp_path / "file.txt").write_text("feature\n") + assert run_cli(tmp_path, "add", "file.txt").returncode == 0 + assert run_cli(tmp_path, "commit", "-m", "feature").returncode == 0 + assert run_cli(tmp_path, "checkout", "main").returncode == 0 + + conflict = run_cli(tmp_path, "merge", "feature") + + assert conflict.returncode == 1 + assert conflict.stdout == "" + assert "minigit: merge conflict in: file.txt" in conflict.stderr + assert "resolve conflicts, run 'minigit add ', then 'minigit commit -m '" in ( + conflict.stderr + ) + assert "Traceback" not in conflict.stderr + assert CommitManager(tmp_path).get_merge_status().unresolved_paths == ("file.txt",) + + blocked = run_cli(tmp_path, "commit", "-m", "too soon") + assert blocked.returncode == 1 + assert blocked.stdout == "" + assert "unresolved merge paths: file.txt" in blocked.stderr + + +def test_merge_subprocess_reports_clean_two_parent_merge(tmp_path): + init_repo(tmp_path) + commit_file(tmp_path, "base\n", "base") + assert run_cli(tmp_path, "branch", "feature").returncode == 0 + (tmp_path / "main.txt").write_text("main\n") + assert run_cli(tmp_path, "add", "main.txt").returncode == 0 + assert run_cli(tmp_path, "commit", "-m", "main").returncode == 0 + main_tip = CommitManager(tmp_path).read_ref("main") + assert run_cli(tmp_path, "checkout", "feature").returncode == 0 + (tmp_path / "feature.txt").write_text("feature\n") + assert run_cli(tmp_path, "add", "feature.txt").returncode == 0 + assert run_cli(tmp_path, "commit", "-m", "feature").returncode == 0 + feature_tip = CommitManager(tmp_path).read_ref("feature") + assert run_cli(tmp_path, "checkout", "main").returncode == 0 + + merged = run_cli(tmp_path, "merge", "feature") + + manager = CommitManager(tmp_path) + merge_tip = manager.read_ref("main") + assert (merged.returncode, merged.stdout, merged.stderr) == ( + 0, + f"Merged 'feature' into 'main' as {merge_tip[:7]}.\n", + "", + ) + assert manager.read_commit(merge_tip).parents == [main_tip, feature_tip] + + +def test_log_expected_object_and_ref_failures_are_concise(tmp_path): + init_repo(tmp_path) + commit_hash = commit_file(tmp_path, "base\n", "base") + ref = tmp_path / ".minigit" / "refs" / "heads" / "main" + + ref.write_text("f" * 40 + "\n") + missing = run_cli(tmp_path, "log") + assert missing.returncode == 1 + assert missing.stdout == "" + assert missing.stderr == f"minigit: object not found: {'f' * 40}\n" + assert "Traceback" not in missing.stderr + + ref.write_text("not-a-hash\n") + invalid = run_cli(tmp_path, "log") + assert invalid.returncode == 1 + assert invalid.stdout == "" + assert invalid.stderr == "minigit: invalid ref 'main': not-a-hash\n" + assert "Traceback" not in invalid.stderr + + ref.write_text(commit_hash + "\n") + object_path = tmp_path / ".minigit" / "objects" / commit_hash[:2] / commit_hash[2:] + object_path.write_bytes(b"corrupt") + corrupt = run_cli(tmp_path, "log") + assert corrupt.returncode == 1 + assert corrupt.stdout == "" + assert corrupt.stderr == f"minigit: object corrupt: {commit_hash}\n" + assert "Traceback" not in corrupt.stderr + + +def test_argparse_usage_errors_keep_exit_code_two(tmp_path): + result = run_cli(tmp_path, "log", "--unknown") + + assert result.returncode == 2 + assert result.stdout == "" + assert "usage: minigit" in result.stderr + assert "unrecognized arguments: --unknown" in result.stderr + + +def test_network_failure_remains_distinct_expected_cli_error(tmp_path): + init_repo(tmp_path) + commit_file(tmp_path, "base\n", "base") + + result = run_cli(tmp_path, "push", "127.0.0.1:1", "main", "--token", "test-token") + + assert result.returncode == 1 + assert result.stdout == "" + assert result.stderr.startswith("minigit: could not connect to 127.0.0.1:1") + assert "Traceback" not in result.stderr diff --git a/tests/test_commits.py b/tests/test_commits.py index 8138c0f..e758db3 100644 --- a/tests/test_commits.py +++ b/tests/test_commits.py @@ -6,7 +6,7 @@ import pytest from minigit.cli import main -from minigit.commits import CommitManager +from minigit.commits import CommitManager, MergeResult, MergeStatus from minigit.errors import ( MergeConflictError, MiniGitError, @@ -185,6 +185,8 @@ def test_read_commit_round_trips_fields(tmp_path): assert c.parents == [] assert c.author == AUTHOR assert c.committer == AUTHOR + assert isinstance(c.author_timestamp, int) + assert c.committer_timestamp == c.author_timestamp assert c.message == "init" @@ -218,6 +220,17 @@ def test_read_commit_malformed_body_raises(tmp_path): m.read_commit(bad) +def test_read_commit_rejects_timestamp_outside_supported_date_range(tmp_path): + m = make_manager(tmp_path) + tree = make_tree(m) + timestamp = "9" * 100 + body = f"tree {tree}\nauthor {AUTHOR} {timestamp}\ncommitter {AUTHOR} {timestamp}\n\nmessage" + commit_hash = m.store.write_object(body.encode(), "commit") + + with pytest.raises(ObjectCorruptError): + m.read_commit(commit_hash) + + # testing get_head_tree def test_get_head_tree_unborn_branch_returns_none(tmp_path): m = make_manager(tmp_path) @@ -313,7 +326,7 @@ def test_branches_point_at_different_hashes_after_switch(tmp_path): # testing log -def test_log_has_one_line_per_commit_newest_first(tmp_path): +def test_log_has_one_entry_per_commit_newest_first(tmp_path): m = make_manager(tmp_path) tree = make_tree(m) first = m.create_commit(tree, [], AUTHOR, "first") @@ -336,6 +349,88 @@ def test_log_shows_both_sides_of_merge_without_duplicates(tmp_path): assert sum("base" in line for line in lines) == 1 +def test_log_detailed_output_has_full_metadata_and_unicode_message(tmp_path, monkeypatch): + monkeypatch.setattr("minigit.commits.time.time", lambda: 0) + m = make_manager(tmp_path) + commit_hash = m.create_commit( + make_tree(m), [], "Zoë ", "Résumé summary\n\nSecond line" + ) + + assert m.log() == [ + f"commit {commit_hash}\n" + "Author: Zoë \n" + "Date: 1970-01-01T00:00:00+00:00\n" + "\n" + " Résumé summary\n" + " \n" + " Second line" + ] + + +def test_log_detailed_merge_lists_both_full_parent_ids_in_order(tmp_path, monkeypatch): + monkeypatch.setattr("minigit.commits.time.time", lambda: 1) + m = make_manager(tmp_path) + tree = make_tree(m) + base = m.create_commit(tree, [], AUTHOR, "base") + left = m.create_commit(tree, [base], AUTHOR, "left") + right = m.create_commit(tree, [base], AUTHOR, "right") + merge = m.create_commit(tree, [left, right], AUTHOR, "merge") + + assert f"Parents: {left} {right}" in m.log()[0] + assert m.log()[0].startswith(f"commit {merge}\n") + + +def test_log_oneline_keeps_compact_summaries(tmp_path): + m = make_manager(tmp_path) + tree = make_tree(m) + first = m.create_commit(tree, [], AUTHOR, "first\nbody") + second = m.create_commit(tree, [first], AUTHOR, "second") + + assert m.log(oneline=True) == [f"{second[:7]} second", f"{first[:7]} first"] + + +def test_log_order_ignores_equal_and_skewed_timestamps(tmp_path, monkeypatch): + timestamps = iter([100, 100, 1_000_000, -50]) + monkeypatch.setattr("minigit.commits.time.time", lambda: next(timestamps)) + m = make_manager(tmp_path) + tree = make_tree(m) + base = m.create_commit(tree, [], AUTHOR, "base") + left = m.create_commit(tree, [base], AUTHOR, "left") + right = m.create_commit(tree, [base], AUTHOR, "right") + merge = m.create_commit(tree, [left, right], AUTHOR, "merge") + + assert m.walk_history(merge) == [merge, left, base, right] + assert [entry.splitlines()[-1].strip() for entry in m.log()] == [ + "merge", + "left", + "base", + "right", + ] + + +def test_log_parses_each_reachable_commit_once(tmp_path, monkeypatch): + m = make_manager(tmp_path) + tree = make_tree(m) + base = m.create_commit(tree, [], AUTHOR, "base") + left = m.create_commit(tree, [base], AUTHOR, "left") + right = m.create_commit(tree, [base], AUTHOR, "right") + m.create_commit(tree, [left, right], AUTHOR, "merge") + original_read_object = m.store.read_object + commit_reads = 0 + + def counted_read_object(object_hash): + nonlocal commit_reads + result = original_read_object(object_hash) + if result[0] == "commit": + commit_reads += 1 + return result + + monkeypatch.setattr(m.store, "read_object", counted_read_object) + + assert len(m.log()) == 4 + assert commit_reads == 4 + + def test_fresh_manager_reads_same_history(tmp_path): m1 = make_manager(tmp_path) tree = make_tree(m1) @@ -541,6 +636,37 @@ def test_merge_same_tip_changes_nothing(tmp_path): assert m.tree.checked_out == [] +def test_merge_with_result_reports_up_to_date(tmp_path): + m = make_manager(tmp_path) + tip = m.create_commit(make_tree(m), [], AUTHOR, "tip") + m.create_branch("feature", tip) + + assert m.merge_with_result("feature") == MergeResult("up-to-date") + + +def test_merge_with_result_reports_fast_forward(tmp_path): + m = make_manager(tmp_path) + _, target = setup_fast_forward(m) + + assert m.merge_with_result("feature") == MergeResult("fast-forward", target) + + +def test_merge_with_result_reports_clean_merge(tmp_path): + m = make_manager(tmp_path) + ours, theirs = setup_diverged( + m, + {"shared.txt": "base\n"}, + {"shared.txt": "base\n", "ours.txt": "ours\n"}, + {"shared.txt": "base\n", "theirs.txt": "theirs\n"}, + ) + + result = m.merge_with_result("feature") + + assert result.outcome == "merged" + assert result.commit_hash is not None + assert m.read_commit(result.commit_hash).parents == [ours, theirs] + + def test_merge_already_merged_target_changes_nothing(tmp_path): """Merging an ancestor of the current tip is a no-op.""" m = make_manager(tmp_path) @@ -906,6 +1032,75 @@ def test_cli_conflict_then_resolve_add_commit(tmp_path, monkeypatch): assert not (tmp_path / ".minigit" / "MERGE_HEAD").exists() +def test_get_merge_status_is_public_read_only_view(tmp_path): + m = make_manager(tmp_path) + ours, theirs = setup_diverged( + m, + {"file.txt": "base\n"}, + {"file.txt": "ours\n"}, + {"file.txt": "theirs\n"}, + ) + + assert m.get_merge_status() is None + with pytest.raises(MergeConflictError) as info: + m.merge_with_result("feature") + + assert info.value.paths == ["file.txt"] + status = m.get_merge_status() + assert status == MergeStatus( + ours=ours, + theirs=theirs, + message="Merge branch 'feature'", + unresolved_paths=("file.txt",), + ) + with pytest.raises(AttributeError): + status.message = "changed" + + +def test_get_merge_status_updates_after_path_is_resolved(tmp_path): + m = make_manager(tmp_path) + setup_diverged( + m, + {"file.txt": "base\n"}, + {"file.txt": "ours\n"}, + {"file.txt": "theirs\n"}, + ) + with pytest.raises(MergeConflictError): + m.merge("feature") + + m.mark_path_resolved("file.txt") + + assert m.get_merge_status().unresolved_paths == () + + +def test_log_and_branch_listing_leave_all_repository_state_unchanged(tmp_path, monkeypatch, capsys): + monkeypatch.chdir(tmp_path) + m = make_real_manager(tmp_path) + base = commit_file(m, "file.txt", "base\n", "base") + m.create_branch("feature", base) + commit_file(m, "file.txt", "main\n", "main") + m.switch_branch("feature") + commit_file(m, "file.txt", "feature\n", "feature") + m.switch_branch("main") + with pytest.raises(MergeConflictError): + m.merge("feature") + + def snapshot(): + return { + path.relative_to(tmp_path).as_posix(): path.read_bytes() + for path in tmp_path.rglob("*") + if path.is_file() + } + + before = snapshot() + assert main(["log"]) == 0 + assert main(["log", "--oneline"]) == 0 + assert main(["branch"]) == 0 + capsys.readouterr() + + assert snapshot() == before + + # fetched-tip integration diff --git a/tests/test_week3_integration.py b/tests/test_week3_integration.py index bbc9b69..63eaba9 100644 --- a/tests/test_week3_integration.py +++ b/tests/test_week3_integration.py @@ -37,7 +37,7 @@ def test_cli_commit_status_and_history(tmp_path, monkeypatch, capsys): assert manager.read_commit(second).parents == [first] assert manager.store.read_object(entries[0].hash) == ("blob", b"first") capsys.readouterr() - assert main(["log"]) == 0 + assert main(["log", "--oneline"]) == 0 assert capsys.readouterr().out == f"{second[:7]} second\n{first[:7]} first\n"