Re: [PATCH 1/2] t4151: document a pair of am --abort bugs

On Tue, Sep 7, 2021 at 10:13 PM Bagas Sanjaya [off-list ref] wrote:

On 08/09/21 09.17, Elijah Newren via GitGitGadget wrote:

quoted

From: Elijah Newren <redacted>

Signed-off-by: Elijah Newren <redacted>
---
  t/t4151-am-abort.sh | 31 +++++++++++++++++++++++++++++++
  1 file changed, 31 insertions(+)

diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh
index 9d8d3c72e7e..501a7a9d211 100755
--- a/t/t4151-am-abort.sh
+++ b/t/t4151-am-abort.sh

@@ -23,7 +23,11 @@ test_expect_success setup '
              test_tick &&
              git commit -a -m $i || return 1
      done &&
+     git branch changes &&
      git format-patch --no-numbered initial &&
+     git checkout -b conflicting initial &&
+     echo different >>file-1 &&
+     git commit -a -m different &&
      git checkout -b side initial &&
      echo local change >file-2-expect
  '

@@ -191,4 +195,31 @@ test_expect_success 'am --abort leaves index stat info alone' '
      git diff-files --exit-code --quiet
  '

+test_expect_failure 'git am --abort return failed exit status when it fails' '
+     test_when_finished "rm -rf file-2/ && git reset --hard" &&
+     git checkout changes &&
+     git format-patch -1 --stdout conflicting >changes.mbox &&
+     test_must_fail git am --3way changes.mbox &&
+
+     git rm file-2 &&
+     mkdir file-2 &&
+     echo precious >file-2/somefile &&
+     test_must_fail git am --abort &&
+     test_path_is_dir file-2/
+'
+
+test_expect_failure 'git am --abort returns us to a clean state' '
+     git checkout changes &&
+     git format-patch -1 --stdout conflicting >changes.mbox &&
+     test_must_fail git am --3way changes.mbox &&
+
+     # Make a change related to the rest of the am work
+     echo related change >>file-2 &&
+
+     # Abort, and expect the related change to go away too
+     git am --abort &&
+     git status --porcelain -uno >actual &&
+     test_must_be_empty actual
+'
+
  test_done

I expect BUGS section in git-am(1) to be added to describe known bugs
you demonstrated above, judging from the patch subject.

There's no point documenting the first one since it'll be fixed by the
next patch.  As for the second, as I noted in my cover letter, I'm not
quite sure that it really is a bug.  If it isn't, the second testcase
should be dropped.  However, if the second testcase represents an
actual bug rather than me just misjudging the intent, then your
suggestion certainly makes sense.