On Mon, Nov 19 2018, Derrick Stolee wrote: > 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. Yup. Sorry should have mentioned that, it's broken in master. Reported it earlier today: https://public-inbox.org/git/874lcd1bub.fsf@xxxxxxxxxxxxxxxxxxx/ > What's the correct fix here? Force the builtin rebase in this test? > Unify the error message in the non-builtin case? Probably to just force the builtin, unless Johannes wants to also fix the bug for the shellscript version. I don't know if for 2.20 we're trying to maintain 100% compatibility.
