Re: [PATCH v6 1/4] commit test: Use test_config instead of git-config

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Caleb Thompson <caleb@xxxxxxxxxxxxxxxx> writes:

> Some of the tests in t/t7507-commit-verbose.sh were still using
> git-config to set configuration. Change them to use the test_config
> helper.

"were still using" is only a half of the story, and we need to be
more careful than that, though.

> Signed-off-by: Caleb Thompson <caleb@xxxxxxxxxxxxxxxx>
> Reviewed-by: Jeremiah Mahler <jmmahler@xxxxxxxxx>
> ---
>  t/t7507-commit-verbose.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh
> index 2ddf28c..6d778ed 100755
> --- a/t/t7507-commit-verbose.sh
> +++ b/t/t7507-commit-verbose.sh
> @@ -43,7 +43,7 @@ test_expect_success 'verbose diff is stripped out' '
>  '
>  
>  test_expect_success 'verbose diff is stripped out (mnemonicprefix)' '
> -	git config diff.mnemonicprefix true &&
> +	test_config diff.mnemonicprefix true &&

By switching to test_config, you unconfigure the diff.mnemonicprefix
configuration after this test piece is done.  The next one, "diff in
message is retained with -v", used to show the change using c/ and
i/ as prefixes in the log editor, but now it should be showing a/
and b/.

Have you thought about the reason why the log message used in that
test uses c/ and i/ prefixes as a sample patch to be retained,
instead of a/ and b/ prefixes?  It is to make sure that this
in-message patch looks similar to the diff for "--verbose" option,
and we used to do the latter under mnemonicprefix.  That way, we
would be more sure that an incorrect implementation that cuts the
result given from the editor at "diff --git $srcprefix/" will fail
the test.

This is the kind of thing I mean by "we need to be more careful".

If somebody who is doing "git config to test_config" conversion
understands what he is doing, I would have expected to see that
these c/ and i/ prefixes in the sample log message are replaced
to use a/ and b/ prefixes.

>  	git commit --amend -v &&
>  	check_message message
>  '
> @@ -71,7 +71,7 @@ test_expect_success 'diff in message is retained with -v' '
>  '
>  
>  test_expect_success 'submodule log is stripped out too with -v' '
> -	git config diff.submodule log &&
> +	test_config diff.submodule log &&
>  	git submodule add ./. sub &&
>  	git commit -m "sub added" &&
>  	(

Can we make a similar reasoning on possible fallout from this
change?  An expected answer would be something like:

    The remaining test pieces after this one does not make any
    change before running "commit -a -v", so having diff.submodule
    set to log or unset does not make any difference to them.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]