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

Reply via email to