Re: [PATCH 1/3] rebase -r: do create merge commit after empty resolution

> On Fri, Mar 28, 2025 at 1:14 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
> > On Fri, Mar 28, 2025 at 1:03 PM Philippe Blain via GitGitGadget
> > <gitgitgadget@gmail.com> wrote:
> > > diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh
> > > +test_expect_success '--continue creates merge commit after empty resolution' '
> > > +       [...]
> > > +       git commit --no-edit &&
> > > +       FAKE_LINES="1 2 3 5 6 7 8 9 10 11" &&
> > > +       export FAKE_LINES &&
> > > +       test_must_fail git rebase -ir main &&
> >
> > I don't think you want to be setting FAKE_LINES like this since doing
> > so will pollute the environment for all tests following this one. You
> > can find existing precedent in this script which demonstrates the
> > correct way to handle this case. Specifically, you'd want:
> >
> >     test_must_fail env FAKE_LINES="1 2 3 5 6 7 8 9 10 11" \
> >         git rebase -ir main &&
>
> To clarify, by "pollute", I mean that it can impact subsequent tests
> which don't take care to override FAKE_LINES as necessary. There
> certainly are test scripts which use the:
>
>     FAKE_LINES=... &&
>     export FAKE_LINES &&
>
> form successfully, but such scripts are careful to override/set
> FAKE_LINES in every test. This particular script (t3418), on the other
> hand, does not otherwise employ the form in which the variable is
> exported, so introducing it in a test which is inserted into the
> middle of the script feels dangerous.