Re: [PATCH 2/2] t7501: Add test for amending commit to add signoff.

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Jan 9, 2024, 10:44 UTC
Message-ID: <2113e178-149b-49aa-9d78-ff1c480f754c@gmail.com>
In-Reply-To: <20240109060417.1144647-4-shyamthakkar001@gmail.com>

Hi Ghanshyam

On 09/01/2024 06:04, Ghanshyam Thakkar wrote:

> This commit adds test for amending the latest commit to add
> Signed-off-by trailer at the end of commit message.

If we're not already testing this then it seems like a useful addition, thanks for working on it. It would also be helpful to check that "git commit --amend --signoff" does not add a Signed-off-by: trailer if it already exists.

> +test_expect_success 'amend commit to add signoff' '
> +
> +	test_when_finished "rm -rf testdir" &&
> +	git init testdir &&

As Christian said about the other patch in this series I don't think we need a new repository here. In our test files we use the same repository for the whole file unless there is a compelling reason not to.

> +	echo content >testdir/file &&
> +	git -C testdir add file &&
> +	git -C testdir commit -m "file" &&

I think these three lines can be replaced by

	test_commit --no-tag file file content

> +	git -C testdir commit --amend --signoff &&

Show 8 quoted lines

> +	git -C testdir log -1 --pretty=format:%B >actual &&
> +	(
> +		echo file &&
> +		echo &&
> +		git -C testdir var GIT_COMMITTER_IDENT >ident &&
> +		sed -e "s/>.*/>/" -e "s/^/Signed-off-by: /" ident
> +	) >expected &&
> +	test_cmp expected actual

This section of the test can be improved by using test_commit_message

	test_commit_message HEAD <<-EOF
	file
	
	Signed-off-by: $GIT_COMMITER_NAME <$GIT_COMMITTER_EMAIL>
	EOF

Best Wishes

Phillip