On Fri, Mar 28, 2025 at 1:14 PM Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
> On Fri, Mar 28, 2025 at 1:03 PM Philippe Blain via GitGitGadget
> <gitgitgadget@xxxxxxxxx> 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.
