Re: Git Test Coverage Report (v2.20.0-rc0)
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Nov 19, 2018, 21:37 UTC
- Message-ID
- <87va4szr2q.fsf@evledraar.gmail.com>
- In-Reply-To
- <6f0ff2e3-4019-1dcc-f61a-cd0919b9a247@gmail.com>
On Mon, Nov 19 2018, Derrick Stolee wrote:
Show 35 quoted lines
> 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@evledraar.gmail.com/
> 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.