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

From: Derrick Stolee <stolee@gmail.com>
Date: Nov 19, 2018, 21:31 UTC
Message-ID: <6f0ff2e3-4019-1dcc-f61a-cd0919b9a247@gmail.com>
In-Reply-To: <871s7g29zy.fsf@evledraar.gmail.com>

On 11/19/2018 1:33 PM, Ævar Arnfjörð Bjarmason wrote:

Show 12 quoted lines

> 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