Hi Eric, On Fri, 28 Mar 2025, Eric Sunshine wrote: > 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. The entire `FAKE_LINES` paradigm is broken, and since I suspect that it was me who introduced it, I apologize. A much better way to have done this would have been to write the string to a certain file, say, $(git rev-parse --git-path sequencer.pick-lines), and in the `fake-editor.sh`: - test for the existence of that file, and if it exists - use its contents - delete that file Ciao, Johannes
