gabriel / muse public
test_push_branch_have.py python
331 lines 12.1 KB
Raw
sha256:79ffe87f5fe2ec146e35f05521218bbf54dffdb0440c07f970bad05f16efb89f chore: merge main — carry all urllib/typing/test fixes from dev Sonnet 4.6 minor ⚠ breaking 21 days ago
1 """TDD — Phase 1: branch_have uses all remote branch heads unconditionally.
2
3 Root cause (issue #56)
4 -----------------------
5 ``branch_have`` — the BFS stop set passed to ``walk_commits`` / ``build_mpack``
6 — only contains the target branch's remote HEAD for non-merge commits.
7
8 When pushing a second branch (e.g. ``dev`` after ``main``), the client should
9 stop the DAG walk at any commit already on the remote, regardless of which
10 remote branch contains it. Without this, the client walks back to the
11 repository root and packs every commit the remote already holds under
12 ``main``.
13
14 Scenario
15 --------
16 Repo history::
17
18 A ← B ← C ← D (main)
19 ↑
20 E (dev, 1 commit ahead)
21
22 Remote state after first push::
23 main → D
24
25 Correct second push (dev)::
26 branch_have = [D] # D is main's remote head — walk stops there
27 new_commits = [E] # only E is new
28
29 Current buggy second push (dev)::
30 branch_have = [] # dev has no remote head yet → target head is null
31 new_commits = [E, D, C, B, A] # entire history re-sent
32
33 Coverage
34 --------
35 BH-1 branch_have includes all remote branch heads, not just target branch.
36 BH-2 Non-merge commit: walk stops at any remote branch head, not just target.
37 BH-3 walk_commits BFS count equals expected new commits only.
38 BH-4 build_mpack sends only genuinely new commits + objects.
39 BH-5 mpack size is small (no redundant commits) on second-branch push.
40 """
41 from __future__ import annotations
42
43 import datetime
44 import pathlib
45
46 import pytest
47
48 from muse.core.commits import write_commit, CommitRecord
49 from muse.core.types import Manifest
50 from muse.core.mpack import walk_commits, build_mpack_from_walk
51 from muse.core.object_store import write_object
52 from muse.core.refs import write_branch_ref
53 from muse.core.snapshot import compute_commit_id, compute_snapshot_id
54 from muse.core.snapshots import write_snapshot, SnapshotRecord
55
56
57 # ---------------------------------------------------------------------------
58 # Helpers
59 # ---------------------------------------------------------------------------
60
61 _TS = datetime.datetime(2026, 1, 1, tzinfo=datetime.timezone.utc)
62
63
64 def _obj(root: pathlib.Path, content: bytes) -> str:
65 from muse.core.types import blob_id
66 oid = blob_id(content)
67 write_object(root, oid, content)
68 return oid
69
70
71 def _snap(root: pathlib.Path, manifest: "Manifest", dirs: list[str] | None = None) -> str:
72 sid = compute_snapshot_id(manifest, dirs)
73 write_snapshot(root, SnapshotRecord(
74 snapshot_id=sid,
75 manifest=manifest,
76 directories=dirs or [],
77 ))
78 return sid
79
80
81 def _commit(
82 root: pathlib.Path,
83 message: str,
84 snapshot_id: str,
85 parent: str | None = None,
86 author: str = "test",
87 ) -> str:
88 cid = compute_commit_id(
89 parent_ids=[parent] if parent else [],
90 snapshot_id=snapshot_id,
91 message=message,
92 committed_at_iso=_TS.isoformat(),
93 author=author,
94 )
95 write_commit(root, CommitRecord(
96 commit_id=cid,
97 branch="main",
98 snapshot_id=snapshot_id,
99 message=message,
100 committed_at=_TS,
101 parent_commit_id=parent,
102 author=author,
103 ))
104 return cid
105
106
107 def _build_linear_chain(root: pathlib.Path, n: int) -> list[str]:
108 """Build n commits A→B→…→N, each adding a file. Returns commit IDs oldest first."""
109 commits: list[str] = []
110 manifest: dict[str, str] = {}
111 prev: str | None = None
112 for i in range(n):
113 content = f"file {i}".encode()
114 oid = _obj(root, content)
115 manifest[f"file{i}.txt"] = oid
116 sid = _snap(root, dict(manifest))
117 cid = _commit(root, f"commit {i}", sid, parent=prev)
118 commits.append(cid)
119 prev = cid
120 return commits
121
122
123 # ---------------------------------------------------------------------------
124 # BH-1 branch_have includes ALL remote branch heads
125 # ---------------------------------------------------------------------------
126
127 def test_BH1_branch_have_includes_all_remote_heads(
128 tmp_path: pathlib.Path, monkeypatch: pytest.MonkeyPatch
129 ) -> None:
130 """BH-1: branch_have from _push_mpack includes all remote branch heads."""
131 from muse.cli.commands import push as push_mod
132
133 # Simulate remote_branch_heads returned by fetch_remote_refs
134 remote_heads = {
135 "main": "sha256:" + "a" * 64,
136 "dev": "sha256:" + "b" * 64,
137 "feat": "sha256:" + "c" * 64,
138 }
139 local_head = "sha256:" + "d" * 64
140 push_branch = "newbranch"
141
142 # The branch_have logic lives in the run() function.
143 # We extract just the computation: all remote heads that are valid commit IDs.
144 from muse.cli.commands.push import _is_valid_commit_id
145
146 # Current (buggy) logic: only uses remote_head for target branch
147 target_remote_head = remote_heads.get(push_branch) # None — new branch
148 buggy_branch_have = (
149 [target_remote_head]
150 if target_remote_head and _is_valid_commit_id(target_remote_head)
151 else []
152 )
153 assert buggy_branch_have == [], "Buggy logic: empty when pushing new branch"
154
155 # Fixed logic: use ALL remote heads unconditionally
156 fixed_branch_have = [
157 h for h in remote_heads.values()
158 if _is_valid_commit_id(h)
159 ]
160 assert len(fixed_branch_have) == 3, (
161 f"Fixed logic must include all {len(remote_heads)} remote heads, "
162 f"got {len(fixed_branch_have)}"
163 )
164 assert "sha256:" + "a" * 64 in fixed_branch_have
165 assert "sha256:" + "b" * 64 in fixed_branch_have
166 assert "sha256:" + "c" * 64 in fixed_branch_have
167
168
169 # ---------------------------------------------------------------------------
170 # BH-2 Non-merge push stops at remote branches other than target
171 # ---------------------------------------------------------------------------
172
173 def test_BH2_walk_stops_at_any_remote_head(tmp_path: pathlib.Path) -> None:
174 """BH-2: walk_commits stops at a commit on a different remote branch."""
175 monkeypatch = None # not needed — we test walk_commits directly
176
177 from muse.core.mpack import walk_commits as _walk
178
179 root = tmp_path
180 from muse.core.paths import init_repo_dirs as init_repo
181 init_repo(root)
182
183 # Build: A → B → C → D (main), D → E (dev)
184 commits = _build_linear_chain(root, 5) # A B C D E
185 A, B, C, D, E = commits
186
187 # Simulate: main is at D on remote, dev has E locally but no remote head
188 # Fixed branch_have: [D] (main's remote head)
189 branch_have_fixed = [D]
190
191 result = _walk(root, [E], have=branch_have_fixed)
192 new_commit_ids = [c.commit_id for c in result["commits"]]
193
194 # Only E should be new — D and below are already on remote (via main)
195 assert E in new_commit_ids, "E (new dev commit) must be in walk result"
196 assert D not in new_commit_ids, (
197 "D must NOT be in walk result — it's already on remote under main"
198 )
199 assert len(new_commit_ids) == 1, (
200 f"Only 1 new commit (E), got {len(new_commit_ids)}: {new_commit_ids}"
201 )
202
203
204 # ---------------------------------------------------------------------------
205 # BH-3 Buggy logic walks entire history for new branch
206 # ---------------------------------------------------------------------------
207
208 def test_BH3_buggy_branch_have_walks_entire_history(tmp_path: pathlib.Path) -> None:
209 """BH-3: Without the fix, pushing a new branch re-sends the entire DAG."""
210 from muse.core.mpack import walk_commits as _walk
211 from muse.core.paths import init_repo_dirs as init_repo
212
213 root = tmp_path
214 init_repo(root)
215
216 commits = _build_linear_chain(root, 5)
217 A, B, C, D, E = commits
218
219 # Buggy branch_have: [] (target branch has no remote head yet)
220 branch_have_buggy: list[str] = []
221
222 result = _walk(root, [E], have=branch_have_buggy)
223 new_commit_ids = [c.commit_id for c in result["commits"]]
224
225 # All 5 commits are sent — the entire history
226 assert len(new_commit_ids) == 5, (
227 f"Buggy logic must walk all 5 commits, got {len(new_commit_ids)}"
228 )
229
230
231 # ---------------------------------------------------------------------------
232 # BH-4 build_mpack_from_walk sends only new commits + objects
233 # ---------------------------------------------------------------------------
234
235 def test_BH4_build_mpack_only_contains_new_commits(tmp_path: pathlib.Path) -> None:
236 """BH-4: With fixed branch_have, mpack only contains E's commit and objects."""
237 from muse.core.mpack import walk_commits as _walk, build_mpack_from_walk
238 from muse.core.paths import init_repo_dirs as init_repo
239
240 root = tmp_path
241 init_repo(root)
242
243 commits = _build_linear_chain(root, 5)
244 A, B, C, D, E = commits
245
246 branch_have_fixed = [D]
247 result = _walk(root, [E], have=branch_have_fixed)
248 mpack = build_mpack_from_walk(root, result)
249
250 commit_ids_in_mpack = [c["commit_id"] if isinstance(c, dict) else c.commit_id for c in mpack.get("commits", [])]
251 assert E in commit_ids_in_mpack
252 assert D not in commit_ids_in_mpack, "D already on remote — must not be in mpack"
253 assert len(commit_ids_in_mpack) == 1, (
254 f"Mpack must contain exactly 1 commit (E), got {len(commit_ids_in_mpack)}"
255 )
256
257 # Blobs: only the object added in commit E (file4.txt), not earlier files
258 blob_ids = [b["object_id"] for b in mpack.get("blobs", [])]
259 assert len(blob_ids) <= 1, (
260 f"Mpack must contain at most 1 new blob (file4.txt), got {len(blob_ids)}"
261 )
262
263
264 # ---------------------------------------------------------------------------
265 # BH-5 Fixed push.py branch_have is unconditional for all commits
266 # ---------------------------------------------------------------------------
267
268 def test_BH5_push_run_uses_all_remote_heads_as_branch_have(
269 tmp_path: pathlib.Path, monkeypatch: pytest.MonkeyPatch
270 ) -> None:
271 """BH-5: push.run() builds branch_have from ALL remote branch heads."""
272 from muse.core.paths import init_repo_dirs as init_repo
273 from muse.cli.commands.push import _push_mpack, _is_valid_commit_id
274
275 root = tmp_path
276 init_repo(root)
277
278 commits = _build_linear_chain(root, 3)
279 A, B, C = commits
280 write_branch_ref(root, "main", C)
281
282 captured_branch_have: list[list[str]] = []
283
284 original_push_mpack = _push_mpack
285
286 def _spy_push_mpack(*args: "str | bytes | bool | None", branch_have: list[str] | None = None, **kwargs: "str | bytes | bool | None") -> None:
287 captured_branch_have.append(list(branch_have or []))
288 raise SystemExit(0) # abort early — we just need branch_have
289
290 monkeypatch.setattr("muse.cli.commands.push._push_mpack", _spy_push_mpack)
291
292 import argparse
293 from unittest.mock import MagicMock, patch
294
295 remote_heads = {
296 "main": "sha256:" + "a" * 64,
297 "staging": "sha256:" + "b" * 64,
298 }
299
300 mock_info = remote_heads # push.py accesses info["branch_heads"] — return a plain dict
301 mock_transport = MagicMock()
302 mock_transport.fetch_remote_info.return_value = {"branch_heads": remote_heads}
303
304 with patch("muse.cli.commands.push.make_transport", return_value=mock_transport), \
305 patch("muse.cli.commands.push.get_remote", return_value="https://example.com/repo"), \
306 patch("muse.cli.commands.push.get_signing_identity", return_value=None), \
307 patch("muse.cli.commands.push.get_remote_head", return_value=None), \
308 patch("muse.cli.commands.push.require_repo", return_value=root), \
309 patch("muse.cli.commands.push.read_current_branch", return_value="newbranch"), \
310 patch("muse.cli.commands.push.get_head_commit_id", return_value=C):
311 try:
312 args = argparse.Namespace(
313 remote="origin", branch=None, force=False,
314 force_with_lease=False, dry_run=False, delete=False,
315 json_out=False, upstream=False, workers=4,
316 set_upstream_flag=False,
317 )
318 from muse.cli.commands.push import run
319 run(args)
320 except SystemExit:
321 pass
322
323 assert captured_branch_have, "branch_have was never captured — spy not called"
324 bh = captured_branch_have[0]
325
326 # Fixed: ALL remote heads must be in branch_have
327 for remote_head in remote_heads.values():
328 assert remote_head in bh, (
329 f"Fixed branch_have must include all remote heads. "
330 f"Missing {remote_head[:20]}. Got: {[h[:20] for h in bh]}"
331 )
File History 3 commits
sha256:79ffe87f5fe2ec146e35f05521218bbf54dffdb0440c07f970bad05f16efb89f chore: merge main — carry all urllib/typing/test fixes from dev Sonnet 4.6 minor ⚠ 21 days ago
sha256:0bea7600d1eee83e87950be49933b1006fa9dc2c71e7c4ee748d324f61138156 chore: bump version to 0.2.0rc11; fix typing audit violatio… Sonnet 4.6 minor ⚠ 21 days ago
sha256:b1447dbe2ef78eb6ec67b8ec4cc0e9c29472382f4390741d6ce069cdf5efa792 fix: branch_have uses all remote heads unconditionally (Pha… Sonnet 4.6 patch 22 days ago