diff mbox series

[4/6] t7500: add tests for --fixup[amend|reword] options

Message ID 20210217073725.16656-4-charvi077@gmail.com (mailing list archive)
State Superseded
Headers show
Series commit: Implementation of "amend!" commit | expand

Commit Message

Charvi Mendiratta Feb. 17, 2021, 7:37 a.m. UTC
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Mentored-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Charvi Mendiratta <charvi077@gmail.com>
---
 t/t7500-commit-template-squash-signoff.sh | 118 ++++++++++++++++++++++
 1 file changed, 118 insertions(+)

Comments

Junio C Hamano Feb. 17, 2021, 7:59 p.m. UTC | #1
Charvi Mendiratta <charvi077@gmail.com> writes:

> Subject: Re: [PATCH 4/6] t7500: add tests for --fixup[amend|reword] options

Isn't an equal '=' sign missing somewhere?

> +test_fixup_reword_opt () {
> +	test_expect_success C_LOCALE_OUTPUT "--fixup=reword: incompatible with $1" "
> +		echo 'fatal: cannot combine reword option of --fixup with $1' >expect &&
> +		test_must_fail git commit --fixup=reword:HEAD~ $1 2>actual &&
> +		test_cmp expect actual
> +	"
> +}
> +
> +for opt in --all --include --only
> +do
> +	test_fixup_reword_opt $opt
> +done

As I suspected earlier, a pathspec is not tested here, but it should
be.

> +test_expect_success '--fixup=reword: -F give error message' '
> +	echo "fatal: Only one of -c/-C/-F/--fixup can be used." >expect &&
> +	test_must_fail git commit --fixup=reword:HEAD~ -F msg  2>actual &&
> +	test_cmp expect actual
> +'

Why?  If you can use -m msg, you should be able to use -F msgfile,
too, no?

>  test_expect_success 'commit --squash works with -F' '
>  	commit_for_rebase_autosquash_setup &&
Charvi Mendiratta Feb. 18, 2021, 10:15 a.m. UTC | #2
On Thu, 18 Feb 2021 at 01:29, Junio C Hamano <gitster@pobox.com> wrote:
>
[...]
> > +for opt in --all --include --only
> > +do
> > +     test_fixup_reword_opt $opt
> > +done
>
> As I suspected earlier, a pathspec is not tested here, but it should
> be.
>

Okay, I will add it here.

> > +test_expect_success '--fixup=reword: -F give error message' '
> > +     echo "fatal: Only one of -c/-C/-F/--fixup can be used." >expect &&
> > +     test_must_fail git commit --fixup=reword:HEAD~ -F msg  2>actual &&
> > +     test_cmp expect actual
> > +'
>
> Why?  If you can use -m msg, you should be able to use -F msgfile,
> too, no?

Earlier I was thinking to let the `--fixup=amend:`  use the same options as of
current `--fixup=` . But yes I agree that there should be  -F option
also with `amend`
and `reword`.

Thanks for the corrections, will do all the changes and update in the
next revision.

Thanks and Regards,
Charvi
Junio C Hamano Feb. 18, 2021, 7:26 p.m. UTC | #3
Charvi Mendiratta <charvi077@gmail.com> writes:

>> > +test_expect_success '--fixup=reword: -F give error message' '
>> > +     echo "fatal: Only one of -c/-C/-F/--fixup can be used." >expect &&
>> > +     test_must_fail git commit --fixup=reword:HEAD~ -F msg  2>actual &&
>> > +     test_cmp expect actual
>> > +'
>>
>> Why?  If you can use -m msg, you should be able to use -F msgfile,
>> too, no?
>
> Earlier I was thinking to let the `--fixup=amend:`  use the same options as of
> current `--fixup=` . But yes I agree that there should be  -F option
> also with `amend`
> and `reword`.

Hmph, I was actually imagining the opposite---a context that does
not want to take -c/-C/-F would not want to take -m, either.

Why is -m so special, and a lot more importantly, what would a user
want to achieve by using "-m more-text" combined with this
"--fixup=reword:<commit>" or "--fixup=amend:<commit>" feature?
Charvi Mendiratta Feb. 19, 2021, 6:10 a.m. UTC | #4
On Fri, 19 Feb 2021 at 00:56, Junio C Hamano <gitster@pobox.com> wrote:
>
[...]
> Hmph, I was actually imagining the opposite---a context that does
> not want to take -c/-C/-F would not want to take -m, either.
>
> Why is -m so special, and a lot more importantly, what would a user
> want to achieve by using "-m more-text" combined with this
> "--fixup=reword:<commit>" or "--fixup=amend:<commit>" feature?

If we run without '-m' option like below:
$ git commit --fixup=reword:<commit>

Then it pops the editor with default "amend!" commit's message i.e:

amend! subject of <commit> we are fixing.

commit log message of <commit> we are fixing.

(Here the end-user is free to edit the above message body of "amend!" commit )

On the other hand, if used with -m option like below:
$ git commit --fixup=reword:<commit> -m "edited <commit> message"

Then it will not pop the editor and the prepared "amend!" commit is :

amend! subject of <commit> we are fixing.

edited <commit> message.

So, with the "-m" option users can do it with the command line only.
And in both the cases upon `git rebase --autosquash` the commit log
message of <commit> we are fixing, will automatically be replaced by
the commit message body of "amend!" commit.

Hope that explains the working and I also wonder if we can improve it
to make it more user friendly ?

Thanks and Regards,
Charvi
diff mbox series

Patch

diff --git a/t/t7500-commit-template-squash-signoff.sh b/t/t7500-commit-template-squash-signoff.sh
index 6d19ece05d..d2c34019c0 100755
--- a/t/t7500-commit-template-squash-signoff.sh
+++ b/t/t7500-commit-template-squash-signoff.sh
@@ -9,6 +9,8 @@  Tests for template, signoff, squash and -F functions.'
 
 . ./test-lib.sh
 
+. "$TEST_DIRECTORY"/lib-rebase.sh
+
 commit_msg_is () {
 	expect=commit_msg_is.expect
 	actual=commit_msg_is.actual
@@ -279,6 +281,122 @@  test_expect_success 'commit --fixup -m"something" -m"extra"' '
 
 extra"
 '
+get_commit_msg () {
+	rev="$1" &&
+	git log -1 --pretty=format:"%B" "$rev"
+}
+
+test_expect_success 'commit --fixup=amend: creates amend! commit' '
+	commit_for_rebase_autosquash_setup &&
+	cat >expected <<-EOF &&
+	amend! $(git log -1 --format=%s HEAD~)
+
+	$(get_commit_msg HEAD~)
+
+	edited
+	EOF
+	(
+		set_fake_editor &&
+		FAKE_COMMIT_AMEND="edited" \
+			git commit --fixup=amend:HEAD~
+	) &&
+	get_commit_msg HEAD >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success '--fixup=reword: does not commit staged changes' '
+	commit_for_rebase_autosquash_setup &&
+	cat >expected <<-EOF &&
+	amend! $(git log -1 --format=%s HEAD~)
+
+	$(get_commit_msg HEAD~)
+
+	edited
+	EOF
+	(
+		set_fake_editor &&
+		FAKE_COMMIT_AMEND="edited" \
+			git commit --fixup=reword:HEAD~
+	) &&
+	get_commit_msg HEAD >actual &&
+	test_cmp expected actual &&
+	test_cmp_rev HEAD@{1}^{tree} HEAD^{tree} &&
+	test_cmp_rev HEAD@{1} HEAD^ &&
+	test_expect_code 1 git diff --cached --exit-code &&
+	git cat-file blob :foo >actual &&
+	test_cmp foo actual
+'
+
+test_expect_success '--fixup=reword: works with -m option' '
+	commit_for_rebase_autosquash_setup &&
+	cat >expected <<-EOF &&
+	amend! target message subject line
+
+	reword commit message
+	EOF
+	git commit --fixup=reword:HEAD~ -m "reword commit message" &&
+	get_commit_msg HEAD >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'consecutive amend! commits remove amend! line from commit msg body' '
+	commit_for_rebase_autosquash_setup &&
+	cat >expected <<-EOF &&
+	amend! amend! $(git log -1 --format=%s HEAD~)
+
+	$(get_commit_msg HEAD~)
+
+	edited 1
+
+	edited 2
+	EOF
+	echo "reword new commit message" >actual &&
+	(
+		set_fake_editor &&
+		FAKE_COMMIT_AMEND="edited 1" \
+			git commit --fixup=reword:HEAD~ &&
+		FAKE_COMMIT_AMEND="edited 2" \
+			git commit --fixup=reword:HEAD
+	) &&
+	get_commit_msg HEAD >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'deny to create amend! commit if its commit msg body is empty' '
+	commit_for_rebase_autosquash_setup &&
+	echo "Aborting commit due to empty commit message body." >expected &&
+	test_must_fail git commit --fixup=amend:HEAD~ -m " " 2>actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'amend! commit allows empty commit msg body with --allow-empty-message' '
+	commit_for_rebase_autosquash_setup &&
+	cat >expected <<-EOF &&
+	amend! $(git log -1 --format=%s HEAD~)
+	EOF
+	git commit --fixup=amend:HEAD~ -m " " --allow-empty-message &&
+	get_commit_msg HEAD >actual &&
+	test_cmp expected actual
+'
+
+test_fixup_reword_opt () {
+	test_expect_success C_LOCALE_OUTPUT "--fixup=reword: incompatible with $1" "
+		echo 'fatal: cannot combine reword option of --fixup with $1' >expect &&
+		test_must_fail git commit --fixup=reword:HEAD~ $1 2>actual &&
+		test_cmp expect actual
+	"
+}
+
+for opt in --all --include --only
+do
+	test_fixup_reword_opt $opt
+done
+
+test_expect_success '--fixup=reword: -F give error message' '
+	echo "fatal: Only one of -c/-C/-F/--fixup can be used." >expect &&
+	test_must_fail git commit --fixup=reword:HEAD~ -F msg  2>actual &&
+	test_cmp expect actual
+'
 
 test_expect_success 'commit --squash works with -F' '
 	commit_for_rebase_autosquash_setup &&