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

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 12, 2024, 23:10 UTC
Message-ID: <xmqqbk9q6tcs.fsf@gitster.g>
In-Reply-To: <20240112180109.59350-2-shyamthakkar001@gmail.com>

Ghanshyam Thakkar <shyamthakkar001@gmail.com> writes:

Show 16 quoted lines

> @@ -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.

Show 23 quoted lines

> @@ -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.

Show 19 quoted lines

> +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.