Re: [PATCH 2/3] rebase --apply: restore some tests

From: Elijah Newren <newren@gmail.com>
Date: Aug 10, 2021, 16:58 UTC
Message-ID: <CABPp-BFJD77wkroL5NFsH++a-yn0h6s6KM=Xfr2Ad_PfPQrUuw@mail.gmail.com>
In-Reply-To: <f7722dab21dafb73e64754b1286913eaa826f865.1628587917.git.gitgitgadget@gmail.com>

On Tue, Aug 10, 2021 at 2:32 AM Phillip Wood via GitGitGadget <gitgitgadget@gmail.com> wrote:

Show 27 quoted lines

>
> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> 980b482d28 ("rebase tests: mark tests specific to the am-backend with
> --am", 2020-02-15) sought to prepare tests testing the "apply" backend
> in preparation for 2ac0d6273f ("rebase: change the default backend
> from "am" to "merge"", 2020-02-15). However some tests seem to have
> been missed leading to us testing the "merge" backend twice. This
> patch fixes some cases that I noticed while adding tests to these
> files, I have not audited all the other rebase test files.
>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
>  t/t3403-rebase-skip.sh     | 2 +-
>  t/t3418-rebase-continue.sh | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/t/t3403-rebase-skip.sh b/t/t3403-rebase-skip.sh
> index 6365c5af2f7..a44e68d0ffb 100755
> --- a/t/t3403-rebase-skip.sh
> +++ b/t/t3403-rebase-skip.sh
> @@ -52,7 +52,7 @@ test_expect_success setup '
>         '
>
>  test_expect_success 'rebase with git am -3 (default)' '
> -       test_must_fail git rebase main
> +       test_must_fail git rebase --apply main

Looks good.

Show 13 quoted lines

>  '
>
>  test_expect_success 'rebase --skip can not be used with other options' '
> diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh
> index f4c2ee02bc9..e4cb8be0418 100755
> --- a/t/t3418-rebase-continue.sh
> +++ b/t/t3418-rebase-continue.sh
> @@ -36,7 +36,7 @@ test_expect_success 'non-interactive rebase --continue works with touched file'
>         git reset --hard &&
>         git checkout main &&
>
> -       test_must_fail git rebase --onto main main topic &&
> +       test_must_fail git rebase --apply --onto main main topic &&

I think the point here is that you're noticing that "git rebase -i" and "git rebase" are both built on the same rebase backend (the merge one) and thus that testing interactive vs. non-interactive isn't much of a test. The real test we are interested in is merge-backend vs. apply-backend. Your code change here is the necessary one to do that, but it least the test descriptions still talking about interactive vs. non-interactive even though that's not what we're concentrating on anymore.

I'd say we'd at least want to change the description for this test, "non-interactive rebase ---continue works with..." => "rebase --continue with the apply backend works with...", but that we'd probably want to change the description of the test before it and maybe even just use rebase --merge rather than rebase -i.

Show 9 quoted lines

>         echo "Resolved" >F2 &&
>         git add F2 &&
>         test-tool chmtime =-60 F1 &&
> @@ -254,7 +254,7 @@ test_rerere_autoupdate () {
>         '
>  }
>
> -test_rerere_autoupdate
> +test_rerere_autoupdate --apply

Looks good.

Show 5 quoted lines

>  test_rerere_autoupdate -m
>  GIT_SEQUENCE_EDITOR=: && export GIT_SEQUENCE_EDITOR
>  test_rerere_autoupdate -i
> --
> gitgitgadget