Re: Git Test Coverage Report (v2.20.0-rc0)

[Derrick Stolee <stolee@gmail.com>]

On 11/19/2018 1:33 PM, Ævar Arnfjörð Bjarmason wrote:
> On Mon, Nov 19 2018, Derrick Stolee wrote:
>
>> [...]
>> builtin/rebase.c
>> 62c23938fa 55) return env;
>> [...]
>> Ævar Arnfjörð Bjarmason 62c23938f: tests: add a special setup
>> where rebase.useBuiltin is off
> This one would be covered with
> GIT_TEST_REBASE_USE_BUILTIN=false. Obviously trivial, but I wonder if
> the rest of the coverage would look different when passed through the various GIT_TEST_* options.
>

Thanks for pointing out this GIT_TEST_* variable to me. I had been 
running builds with some of them enabled, but didn't know about this one.

Unfortunately, t3406-rebase-message.sh fails with 
GIT_TEST_REBASE_USE_BUILTIN=false and it bisects to 4520c2337: Merge 
branch 'ab/rebase-in-c-escape-hatch'.

The issue is that the commit 04519d72 "rebase: validate -C<n> and 
--whitespace=<mode> parameters early" introduced the following test that 
cares about error messages:

+test_expect_success 'error out early upon -C<n> or --whitespace=<bad>' '
+       test_must_fail git rebase -Cnot-a-number HEAD 2>err &&
+       test_i18ngrep "numerical value" err &&
+       test_must_fail git rebase --whitespace=bad HEAD 2>err &&
+       test_i18ngrep "Invalid whitespace option" err
+'

The merge commit then was the first place where this test could run with 
that variable.

What's the correct fix here? Force the builtin rebase in this test? 
Unify the error message in the non-builtin case?

Thanks,
-Stolee