Hi Stolee, On Mon, 19 Nov 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. > > What's the correct fix here? Force the builtin rebase in this test? Unify the > error message in the non-builtin case? I am obviously biased, but my take is that the correct fix is in https://public-inbox.org/git/pull.86.git.gitgitgadget@xxxxxxxxx Ciao, Dscho
