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

[Eric Sunshine <sunshine@sunshineco.com>]

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.