Re: [PATCH v3 1/2] t3418: add testcase showing problems with rebase -i and strategy options

[Elijah Newren <newren@gmail.com>]

HI SZEDER,

On Wed, Jul 11, 2018 at 3:56 AM, SZEDER Gábor <szeder.dev@gmail.com> wrote:
> On Wed, Jun 27, 2018 at 8:28 PM SZEDER Gábor <szeder.dev@gmail.com> wrote:
>>
>> > diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh
>> > index 03bf1b8a3b..11546d6e14 100755
>> > --- a/t/t3418-rebase-continue.sh
>> > +++ b/t/t3418-rebase-continue.sh
>> > @@ -74,6 +74,38 @@ test_expect_success 'rebase --continue remembers merge strategy and options' '
>> >       test -f funny.was.run
>> >  '
>> >
>> > +test_expect_failure 'rebase -i --continue handles merge strategy and options' '
>> > +     rm -fr .git/rebase-* &&
>> > +     git reset --hard commit-new-file-F2-on-topic-branch &&
>> > +     test_commit "commit-new-file-F3-on-topic-branch-for-dash-i" F3 32 &&
>> > +     test_when_finished "rm -fr test-bin funny.was.run funny.args" &&
>> > +     mkdir test-bin &&
>> > +     cat >test-bin/git-merge-funny <<-EOF &&
>> > +     #!$SHELL_PATH
>> > +     echo "\$@" >>funny.args
>> > +     case "\$1" in --opt) ;; *) exit 2 ;; esac
>> > +     case "\$2" in --foo) ;; *) exit 2 ;; esac
>> > +     case "\$4" in --) ;; *) exit 2 ;; esac
>> > +     shift 2 &&
>> > +     >funny.was.run &&
>> > +     exec git merge-recursive "\$@"
>> > +     EOF
>> > +     chmod +x test-bin/git-merge-funny &&
>>
>> You could use the 'write_script' helper function here.
>>
>> > +     (
>> > +             PATH=./test-bin:$PATH &&
>> > +             test_must_fail git rebase -i -s funny -Xopt -Xfoo master topic
>> > +     ) &&
>> > +     test -f funny.was.run &&
>
> And please use 'test_path_is_file' here ...
>
>> > +     rm funny.was.run &&
>> > +     echo "Resolved" >F2 &&
>> > +     git add F2 &&
>> > +     (
>> > +             PATH=./test-bin:$PATH &&
>> > +             git rebase --continue
>> > +     ) &&
>> > +     test -f funny.was.run
>
> ... and here.
>

Thanks for looking over things.  This particular test was a copy of
another from the same file, just for rebase -i instead of rebase -m.
I think it'd be nice to keep the two tests looking similar and either
fix up the rebase -m case with a preparatory patch added to this
series, or wait for this series to merge and let a future series clean
up both testcases.  For other suggested fixups to the same test, Junio
suggested just taking the latter approach[1], so I'm assuming that's
how I should handle these as well.

[1] https://public-inbox.org/git/xmqqmuvgdods.fsf@gitster-ct.c.googlers.com/