Add integration tests for Graphite operations and missing attribution#1032
Add integration tests for Graphite operations and missing attribution#1032jwiegley wants to merge 1 commit intojohnw/review-silent-errorsfrom
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
ed02bb9 to
9e18ac5
Compare
93b3136 to
a4a730d
Compare
98654e8 to
68438da
Compare
1338d42 to
a09eb0b
Compare
68438da to
99815bc
Compare
99815bc to
b1dbda4
Compare
90c6d77 to
7c12fb9
Compare
b1dbda4 to
c51217f
Compare
7c12fb9 to
b3492b6
Compare
a0ba1b3 to
6a82d28
Compare
b3492b6 to
1d44bd3
Compare
6a82d28 to
b99641f
Compare
1d44bd3 to
4e85bd0
Compare
| pub fn new_worktree() -> Self { | ||
| Self::new_worktree_with_mode(GitTestMode::from_env()) | ||
| } | ||
| Self::new_worktree_with_mode(GitTestMode::from_env()) | ||
| } |
There was a problem hiding this comment.
🟡 Indentation regression in new_worktree() and new_with_remote() violates AGENTS.md formatting rule
The PR introduces incorrect indentation for new_worktree() at tests/integration/repos/test_repo.rs:1110-1112 and new_with_remote() with its doc comment at tests/integration/repos/test_repo.rs:1237-1254. Both methods are inside an impl TestRepo block but their bodies and closing braces are de-indented to module level. This violates the AGENTS.md rule: "Before opening a PR, make sure to run task lint and task fmt and resolve any formatting/lint issues as they will fail in CI." The code compiles correctly since Rust doesn't rely on indentation, but it will fail CI formatting checks.
| pub fn new_worktree() -> Self { | |
| Self::new_worktree_with_mode(GitTestMode::from_env()) | |
| } | |
| Self::new_worktree_with_mode(GitTestMode::from_env()) | |
| } | |
| pub fn new_worktree() -> Self { | |
| Self::new_worktree_with_mode(GitTestMode::from_env()) | |
| } |
Was this helpful? React with 👍 or 👎 to provide feedback.
b99641f to
3d146e5
Compare
4e85bd0 to
faff2e0
Compare
| /// The mirror is cloned from the upstream, so "origin" is automatically configured. | ||
| /// Returns (mirror, upstream) tuple. | ||
| /// | ||
| /// # Example | ||
| /// ```ignore | ||
| /// let (mirror, upstream) = TestRepo::new_with_remote(); | ||
| /// | ||
| /// // Make changes in mirror | ||
| /// mirror.filename("test.txt").write("hello").stage(); | ||
| /// mirror.commit("initial commit"); | ||
| /// | ||
| /// // Push to upstream | ||
| /// mirror.git(&["push", "origin", "main"]); | ||
| /// ``` | ||
| pub fn new_with_remote() -> (Self, Self) { | ||
| Self::new_with_remote_with_mode(GitTestMode::from_env()) | ||
| } |
There was a problem hiding this comment.
🟡 Formatting regression: new_with_remote() and its doc comments lost impl-block indentation
The new_with_remote() method, its doc comments (lines 1238-1251), function signature (line 1252), body (line 1253), and closing brace (line 1254) all lost their 4-space impl-block indentation. The old code had proper /// and pub fn indentation. AGENTS.md explicitly requires: "Before opening a PR, make sure to run task lint and task fmt and resolve any formatting/lint issues as they will fail in CI." This would fail rustfmt.
| /// The mirror is cloned from the upstream, so "origin" is automatically configured. | |
| /// Returns (mirror, upstream) tuple. | |
| /// | |
| /// # Example | |
| /// ```ignore | |
| /// let (mirror, upstream) = TestRepo::new_with_remote(); | |
| /// | |
| /// // Make changes in mirror | |
| /// mirror.filename("test.txt").write("hello").stage(); | |
| /// mirror.commit("initial commit"); | |
| /// | |
| /// // Push to upstream | |
| /// mirror.git(&["push", "origin", "main"]); | |
| /// ``` | |
| pub fn new_with_remote() -> (Self, Self) { | |
| Self::new_with_remote_with_mode(GitTestMode::from_env()) | |
| } | |
| /// The mirror is cloned from the upstream, so "origin" is automatically configured. | |
| /// Returns (mirror, upstream) tuple. | |
| /// | |
| /// # Example | |
| /// ```ignore | |
| /// let (mirror, upstream) = TestRepo::new_with_remote(); | |
| /// | |
| /// // Make changes in mirror | |
| /// mirror.filename("test.txt").write("hello").stage(); | |
| /// mirror.commit("initial commit"); | |
| /// | |
| /// // Push to upstream | |
| /// mirror.git(&["push", "origin", "main"]); | |
| /// ``` | |
| pub fn new_with_remote() -> (Self, Self) { | |
| Self::new_with_remote_with_mode(GitTestMode::from_env()) | |
| } |
Was this helpful? React with 👍 or 👎 to provide feedback.
3d146e5 to
dda323b
Compare
faff2e0 to
77355a3
Compare
New test suites covering two important scenarios: simulating Graphite CLI git operations (stack creation, rebase, amend workflows) and reproducing the intermittent bug where attribution incorrectly reports 100% human authorship. Adds allow_daemon_errors flag to test harness for controlled daemon failure testing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
dda323b to
f7384f2
Compare
77355a3 to
81a24be
Compare

New test suites covering two important scenarios: simulating Graphite
CLI git operations (stack creation, rebase, amend workflows) and
reproducing the intermittent bug where attribution incorrectly reports
100% human authorship. Adds allow_daemon_errors flag to test harness
for controlled daemon failure testing.
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com