Re: [PATCH 1/2] t0014: factor out choice of deprecated commands

Jeff King <[email protected]> writes:

> We have a few tests related to aliasing deprecated commands which use
> "whatchanged" and "pack-redundant", as these are the only two deprecated
> commands we have. Let's pull those names into variables so that we can
> refactor the tests without relying on the specific names.
>
> Signed-off-by: Jeff King <[email protected]>
> ---
> I pulled this into its own patch because it's so noisy, but it could be
> squashed with the subsequent one.

The knee-jerk reaction I got after reading the above explanation
before the morning caffeine fully taking effect and without looking
at [2/2] is "we may have parameterized the exact command names, but
I cannot tell what value this change has, as the fact that we have
exactly two deprecated commands is still hardcoded in the test".

If the point of this change is that even if we ever deprecated a
third command, this test does not need to care about it, then I can
understand it is perfectly fine to have the hardcoded "this test
uses two deprecated commands" while parameterizing which two
commands are used.  But then the log message may be a bit
misleading.  I dunno.

I am very sure that I will be enlightened when I read [2/2], though
;-)

>  t/t0014-alias.sh | 23 +++++++++++++----------
>  1 file changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/t/t0014-alias.sh b/t/t0014-alias.sh
> index 5144b0effd..9d7c737355 100755
> --- a/t/t0014-alias.sh
> +++ b/t/t0014-alias.sh
> @@ -27,17 +27,20 @@ test_expect_success 'looping aliases - internal execution' '
>  	test_grep "^fatal: alias loop detected: expansion of" output
>  '
>  
> +deprecated1=whatchanged
> +deprecated2=pack-redundant
> +
>  test_expect_success 'looping aliases - deprecated builtins' '
> -	test_config alias.whatchanged pack-redundant &&
> -	test_config alias.pack-redundant whatchanged &&
> +	test_config alias.$deprecated1 $deprecated2 &&
> +	test_config alias.$deprecated2 $deprecated1 &&
>  	cat >expect <<-EOF &&
> -	${SQ}whatchanged${SQ} is aliased to ${SQ}pack-redundant${SQ}
> -	${SQ}pack-redundant${SQ} is aliased to ${SQ}whatchanged${SQ}
> -	fatal: alias loop detected: expansion of ${SQ}whatchanged${SQ} does not terminate:
> -	  whatchanged <==
> -	  pack-redundant ==>
> +	${SQ}$deprecated1${SQ} is aliased to ${SQ}$deprecated2${SQ}
> +	${SQ}$deprecated2${SQ} is aliased to ${SQ}$deprecated1${SQ}
> +	fatal: alias loop detected: expansion of ${SQ}$deprecated1${SQ} does not terminate:
> +	  $deprecated1 <==
> +	  $deprecated2 ==>
>  	EOF
> -	test_must_fail git whatchanged -h 2>actual &&
> +	test_must_fail git $deprecated1 -h 2>actual &&
>  	test_cmp expect actual
>  '
>  
> @@ -90,8 +93,8 @@ test_expect_success 'can alias-shadow via two deprecated builtins' '
>  	# some git(1) commands will fail... (see above)
>  	test_might_fail git status -h >expect &&
>  	test_file_not_empty expect &&
> -	test_might_fail git -c alias.whatchanged=pack-redundant \
> -		-c alias.pack-redundant=status whatchanged -h >actual &&
> +	test_might_fail git -c alias.$deprecated1=$deprecated2 \
> +		-c alias.$deprecated2=status $deprecated1 -h >actual &&
>  	test_cmp expect actual
>  '