Re: [PATCH] rebase: fix regression in rebase.useBuiltin=false test mode
- From
- Phillip Wood <phillip.wood@talktalk.net>
- Date
- Feb 14, 2019, 16:12 UTC
- Message-ID
- <af4ce5d4-a788-25a6-0372-13206b161101@talktalk.net>
- In-Reply-To
- <20190213214908.9459-1-avarab@gmail.com>
Hi Ævar
On 13/02/2019 21:49, Ævar Arnfjörð Bjarmason wrote:
Show 13 quoted lines
> Fix a recently introduced regression in c762aada1a ("rebase -x: sanity
> check command", 2019-01-29) triggered when running the tests with
> GIT_TEST_REBASE_USE_BUILTIN=false. See 62c23938fa ("tests: add a
> special setup where rebase.useBuiltin is off", 2018-11-14) for how
> that test mode works.
>
> As discussed on-list[1] it's not worth it to implement the sanity
> check in the legacy rebase code, we plan to remove it after the 2.21
> release. So let's do the bare minimum to make the tests pass under the
> GIT_TEST_REBASE_USE_BUILTIN=false special setup.
>
> 1. https://public-inbox.org/git/xmqqva1nbeno.fsf@gitster-ct.c.googlers.com/
> Thanks for doing this, when I wrote the original patch I decided it wasn't worth updating the legacy rebase but forgot about running the tests with rebase.useBuiltin off.
Best Wishes
Phillip
Show 53 quoted lines
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>
> On Wed, Feb 13 2019, Junio C Hamano wrote:
>
>> [...]
>> It would however be long enough to declare victory _immediately
>> after_ the upcoming release and start the next cycle without the
>> escape hatch. At that point we'd be committed to maintain only the
>> built-in version.
>>
>> The more important every-day features should still be covered by
>> tests, if the scripted ones are to be kept as escape hatch. So to
>> me #1 looks like the only sensible choice at this point, if you want
>> to see a test cycle with USE_BUILTIN=false to fully pass (i.e. by
>> skipping the ones that are known not to pass).
>
> Let's do this for 2.21 then and I'll re-submit the legacy removal
> patch after we start 2.22.
>
> t/t3404-rebase-interactive.sh | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index 52fa41c707..b60b11f9f2 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -149,10 +149,12 @@ test_expect_success 'rebase -i with the exec command checks tree cleanness' '
>
> test_expect_success 'rebase -x with empty command fails' '
> test_when_finished "git rebase --abort ||:" &&
> - test_must_fail git rebase -x "" @ 2>actual &&
> + test_must_fail env GIT_TEST_REBASE_USE_BUILTIN=true \
> + git rebase -x "" @ 2>actual &&
> test_write_lines "error: empty exec command" >expected &&
> test_i18ncmp expected actual &&
> - test_must_fail git rebase -x " " @ 2>actual &&
> + test_must_fail env GIT_TEST_REBASE_USE_BUILTIN=true \
> + git rebase -x " " @ 2>actual &&
> test_i18ncmp expected actual
> '
>
> @@ -160,7 +162,8 @@ LF='
> '
> test_expect_success 'rebase -x with newline in command fails' '
> test_when_finished "git rebase --abort ||:" &&
> - test_must_fail git rebase -x "a${LF}b" @ 2>actual &&
> + test_must_fail env GIT_TEST_REBASE_USE_BUILTIN=true \
> + git rebase -x "a${LF}b" @ 2>actual &&
> test_write_lines "error: exec commands cannot contain newlines" \
> >expected &&
> test_i18ncmp expected actual
>