Subject: Re: [PATCH v4 1/2] t7501: add tests for --include and --only

From: Junio C Hamano <[email-masked]>
Date: Fri, 12 Jan 2024 15:10:27 -0800

Ghanshyam Thakkar <[email-masked]> writes:

> @@ -92,6 +92,19 @@ test_expect_success '--long fails with nothing to commit' '
>  	test_must_fail git commit -m initial --long
>  '
>  
> +test_expect_success 'fail to commit untracked file' '
> +	echo content >baz &&
> +	test_must_fail git commit -m "baz" baz
> +'
> +
> +test_expect_success '--only also fail to commit untracked file' '
> +	test_must_fail git commit --only -m "baz" baz
> +'
> +
> +test_expect_success '--include also fail to commit untracked file' '
> +	test_must_fail git commit --include -m "baz" baz
> +'

As the latter two depends on the first one's side effect of leaving
an untracked 'baz' file in the working tree, I do not know if it is
sensible to split these into three tests.  An obvious alternative is
to have a single test

	test_expect_success 'pathspec that do not match any tracked path' '
		echo content >baz &&
		test_must_fail git commit -m baz baz &&
		test_must_fail git commit -o -m baz baz &&
		test_must_fail git commit -i -m baz baz
	'

By the way, I do not think presence of 'baz' in the working tree
matters in the failures from these tests all that much, as the
reason they fail is because the pathspec does not match any tracked
file, whose contents in the index to be updated before made into a
commit.

> @@ -117,6 +130,70 @@ test_expect_success '--long with stuff to commit returns ok' '
>  	git commit -m next -a --long
>  '
>  
> +test_expect_success 'only commit given path (also excluding additional staged changes)' '
> +	echo content >file &&
> +	echo content >baz &&
> +	git add baz &&
> +	git commit -m "file" file &&
> +
> +	git diff --name-only >actual &&
> +	test_must_be_empty actual &&
> +
> +	git diff --name-only --staged >actual &&
> +	test_cmp - actual <<-EOF &&
> +	baz
> +	EOF
> +
> +	git diff --name-only HEAD^ HEAD >actual &&
> +	test_cmp - actual <<-EOF
> +	file
> +	EOF
> +'

OK.  The change to baz is already in the index, but "commit file"
will skip over it and record only the changes to file in the commit.
Very much makes sense.

> +test_expect_success 'same as above with -o/--only' '
> +	echo change >file &&
> +	echo change >baz &&
> +	git add baz &&
> +	git commit --only -m "file" file &&
> +
> +	git diff --name-only >actual &&
> +	test_must_be_empty actual &&
> +
> +	git diff --name-only --staged >actual &&
> +	test_cmp - actual <<-EOF &&
> +	baz
> +	EOF
> +
> +	git diff --name-only HEAD^ HEAD >actual &&
> +	test_cmp - actual <<-EOF
> +	file
> +	EOF
> +'

Likewise.  An obvious thing to notice is that this cannot use the
same "contents" text as before, even though it claims to be "same as
above".  If the final contents of "file" and "baz" does not matter,
but it matters more that these files have been changed, it often is
a good idea to append to the file.  That way, you can ensure that
you will be making them different, no matter what the initial
condition was, i.e.,

	for opt in "" "-o" "--only"
	do
		test_expect_success 'skip over already added change' '
			echo more >>file &&
			echo more >>baz &&
			git add baz &&
			git commit $opt -m "file" file &&

			... ensure that changes to file are committed
			... and changes to baz is only in the index
		'
	done

let's you test all three combinations.

Thanks.