Re: [PATCH] rebase: mark a test as failing with rebase.useBuiltin=false

From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>,
	Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>,
	Phillip Wood <phillip.wood@dunelm.org.uk>
Subject: Re: [PATCH] rebase: mark a test as failing with rebase.useBuiltin=false
Date: Tue, 20 Nov 2018 20:55:35 +0100 (STD)	[thread overview]
Message-ID: <nycvar.QRO.7.76.6.1811202029110.41@tvgsbejvaqbjf.bet> (raw)
In-Reply-To: <20181120114208.14251-1-avarab@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2428 bytes --]

Hi Ævar,

On Tue, 20 Nov 2018, Ævar Arnfjörð Bjarmason wrote:

> Mark a test added in 04519d7201 ("rebase: validate -C<n> and
> --whitespace=<mode> parameters early", 2018-11-14) as only succeeding
> with the builtin version of rebase. It would be nice if the
> shellscript version had the same fix, but it's on its way out, and the
> author is not interested in fixing it[1].

It's not that I am not interested in fixing it. It's more like I hoped
that I could work on Git for Windows v2.20.0-rc0 and rely on you to fix
this bug.

Now that Git for Windows v2.20.0-rc0 is out (see
https://github.com/git-for-windows/git/releases/tag/v2.20.0-rc0.windows.1),
expect a patch soon (see https://github.com/gitgitgadget/git/pull/86).

Ciao,
Dscho

> 
> This makes the entire test suite pass again with the
> GIT_TEST_REBASE_USE_BUILTIN=false mode added in my 62c23938fa ("tests:
> add a special setup where rebase.useBuiltin is off", 2018-11-14).
> 
> 1. https://public-inbox.org/git/nycvar.QRO.7.76.6.1811201157170.41@tvgsbejvaqbjf.bet/
> 
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> 
> On Tue, Nov 20 2018, Johannes Schindelin wrote:
> 
> > [...]
> > Maybe you can concoct a prereq for this test? Something like
> >
> > test_lazy_prereq BUILTIN_REBASE '
> >       test true = "${GIT_TEST_REBASE_USE_BUILTIN:-true}"
> > '
> 
> It's better to just mark the test as needing the prereq turned
> off. E.g. this is what we do for the split index tests & now for the
> gettext tests. That way we always run the test, but just indicate that
> it relies on GIT_TEST_REBASE_USE_BUILTIN being unset.
> 
>  t/t3406-rebase-message.sh | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh
> index 38bd876cab..77e5bbb3d5 100755
> --- a/t/t3406-rebase-message.sh
> +++ b/t/t3406-rebase-message.sh
> @@ -84,7 +84,8 @@ test_expect_success 'rebase --onto outputs the invalid ref' '
>  	test_i18ngrep "invalid-ref" err
>  '
>  
> -test_expect_success 'error out early upon -C<n> or --whitespace=<bad>' '
> +test_expect_success 'builtin rebase: error out early upon -C<n> or --whitespace=<bad>' '
> +	sane_unset GIT_TEST_REBASE_USE_BUILTIN &&
>  	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 &&
> -- 
> 2.20.0.rc0.387.gc7a69e6b6c
> 
> 