Re: [PATCH v2] add -i tests: mark "TODO" depending on GIT_TEST_ADD_I_USE_BUILTIN

[Todd Zullinger <tmz@pobox.com>]

Ævar Arnfjörð Bjarmason wrote:
> Fix an issue that existed before 0527ccb1b55 (add -i: default to the
> built-in implementation, 2021-11-30), but which became the default
> with that change, we should not be marking tests that are known to
> pass as "TODO" tests.
[...]
> ---
> Just converting it to "test_expect_success" will break CI and other
> setups that are testing with GIT_TEST_ADD_I_USE_BUILTIN=false.
> 
> The below fixes it, however.

Nice catch.  FWIW, I tested w/GIT_TEST_ADD_I_USE_BUILTIN=0
and without.

> diff --git a/t/t2016-checkout-patch.sh b/t/t2016-checkout-patch.sh
> index bc3f69b4b1d..a5822e41af2 100755
> --- a/t/t2016-checkout-patch.sh
> +++ b/t/t2016-checkout-patch.sh
> @@ -4,7 +4,7 @@ test_description='git checkout --patch'
>  
>  . ./lib-patch-mode.sh
>  
> -if ! test_bool_env GIT_TEST_ADD_I_USE_BUILTIN true && ! test_have_prereq PERL
> +if ! test_have_prereq ADD_I_USE_BUILTIN && ! test_have_prereq PERL
>  then
>  	skip_all='skipping interactive add tests, PERL not set'

It's not the fault of this patch, but it makes it obvious
that the `skip_all` message is no longer accurate.  Perhaps
somethine like this?

    skip_all='skipping interactive add tests, missing ADD_I_USE_BUILTIN or PERL'

Maybe a separate `ADD_I` prereq would be better?  Though
without looking closer, I don't know if that would end up
being clearer to anyone running the tests without either
PERL or the add -i builtin enabled.

Thanks for the keen eye and attention to detail, Ævar,

-- 
Todd