Æ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
