From 3aaeca9e1cb578f1586e0be15c7f6f16ee284c5e Mon Sep 17 00:00:00 2001
From: Martin von Zweigbergk <martinvonz@google.com>
Date: Fri, 1 Jul 2022 19:50:51 -0700
Subject: [PATCH] tests: add test for successful push

I think I had not added tests for successful push before because I
thought there was some issue with testing it with libgit2. There *is*
an issue, which is that libgit2 requires the remote to be bare when
it's on local disk, but we can very easily make the git repo bare in
this test.

I also updated the error handling in the `git` module to not let
follow-on errors hide the real error and to not fail if two branches
moved to the same commit.
---
 lib/src/git.rs         |  9 +++++-
 tests/test_git_push.rs | 62 ++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 68 insertions(+), 3 deletions(-)

diff --git a/lib/src/git.rs b/lib/src/git.rs
index 3d5d91651..c480d6506 100644
--- a/lib/src/git.rs
+++ b/lib/src/git.rs
@@ -390,7 +390,14 @@ pub fn push_updates(
     for mut temp_ref in temp_refs {
         // TODO: Figure out how to do the equivalent of absl::Cleanup for
         // temp_ref.delete().
-        temp_ref.delete()?;
+        if let Err(err) = temp_ref.delete() {
+            // Propagate error only if we don't already have an error to return and it's not
+            // NotFound (there may be duplicates if the list if multiple branches moved to
+            // the same commit).
+            if result.is_ok() && err.code() != git2::ErrorCode::NotFound {
+                return Err(GitPushError::InternalGitError(err));
+            }
+        }
     }
     result
 }
diff --git a/tests/test_git_push.rs b/tests/test_git_push.rs
index fc8c74e91..246cfeb50 100644
--- a/tests/test_git_push.rs
+++ b/tests/test_git_push.rs
@@ -21,11 +21,40 @@ pub mod common;
 fn set_up() -> (TestEnvironment, PathBuf) {
     let test_env = TestEnvironment::default();
     let git_repo_path = test_env.env_root().join("git-repo");
-    git2::Repository::init(&git_repo_path).unwrap();
+    let git_repo = git2::Repository::init_bare(&git_repo_path).unwrap();
+    let signature =
+        git2::Signature::new("Some One", "some.one@example.com", &git2::Time::new(0, 0)).unwrap();
+    let empty_tree_oid = git_repo.treebuilder(None).unwrap().write().unwrap();
+    let empty_tree = git_repo.find_tree(empty_tree_oid).unwrap();
+    git_repo
+        .commit(
+            Some("refs/heads/branch1"),
+            &signature,
+            &signature,
+            "description",
+            &empty_tree,
+            &[],
+        )
+        .unwrap();
+    git_repo
+        .commit(
+            Some("refs/heads/branch2"),
+            &signature,
+            &signature,
+            "description",
+            &empty_tree,
+            &[],
+        )
+        .unwrap();
 
     test_env.jj_cmd_success(
         test_env.env_root(),
-        &["git", "clone", "git-repo", "jj-repo"],
+        &[
+            "git",
+            "clone",
+            test_env.env_root().join("git-repo").to_str().unwrap(),
+            "jj-repo",
+        ],
     );
     let workspace_root = test_env.env_root().join("jj-repo");
     (test_env, workspace_root)
@@ -41,6 +70,35 @@ fn test_git_push_nothing() {
     "###);
 }
 
+#[test]
+fn test_git_push_success() {
+    let (test_env, workspace_root) = set_up();
+    test_env.jj_cmd_success(&workspace_root, &["branch", "delete", "branch1"]);
+    test_env.jj_cmd_success(
+        &workspace_root,
+        &["branch", "set", "--allow-backwards", "branch2"],
+    );
+    test_env.jj_cmd_success(&workspace_root, &["branch", "create", "my-branch"]);
+    test_env.jj_cmd_success(&workspace_root, &["describe", "-m", "foo"]);
+    // Check the setup
+    let stdout = test_env.jj_cmd_success(&workspace_root, &["branch", "list"]);
+    insta::assert_snapshot!(stdout, @r###"
+    branch1 (deleted)
+      @origin: 545acdb23f70 description
+    branch2: 7840c9885676 foo
+      @origin (ahead by 1 commits, behind by 1 commits): 545acdb23f70 description
+    my-branch: 7840c9885676 foo
+    "###);
+
+    let stdout = test_env.jj_cmd_success(&workspace_root, &["git", "push"]);
+    insta::assert_snapshot!(stdout, @"");
+    let stdout = test_env.jj_cmd_success(&workspace_root, &["branch", "list"]);
+    insta::assert_snapshot!(stdout, @r###"
+    branch2: 7840c9885676 foo
+    my-branch: 7840c9885676 foo
+    "###);
+}
+
 #[test]
 fn test_git_push_conflict() {
     let (test_env, workspace_root) = set_up();