On Tue, May 20, 2025 at 03:34:55PM -0400, D. Ben Knoble wrote: > Some of the editor tests manipulate the environment or config in ways > that affect future tests (because they test a sequence of overrides), > but those modifications are visible to future tests and create a footgun > for them. Use test_config and undo environment modifications once > finished. > > Signed-off-by: D. Ben Knoble <ben.knoble+github@xxxxxxxxx> > --- > t/t7005-editor.sh | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/t/t7005-editor.sh b/t/t7005-editor.sh > index 5fcf281dfb..06fa1ecd91 100755 > --- a/t/t7005-editor.sh > +++ b/t/t7005-editor.sh > @@ -111,6 +111,8 @@ > ' > done > > +unset EDITOR VISUAL GIT_EDITOR > +git config --unset-all core.editor > test_expect_success 'editor with a space' ' > echo "echo space >\"\$1\"" >"e space.sh" && > chmod a+x "e space.sh" && Could we maybe adapt the tests that set those envvars to use a `test_when_finished`? Something like this (untested) patch: diff --git a/t/t7005-editor.sh b/t/t7005-editor.sh index 5fcf281dfbf..a14ff4b38c4 100755 --- a/t/t7005-editor.sh +++ b/t/t7005-editor.sh @@ -93,17 +93,19 @@ unset EDITOR VISUAL GIT_EDITOR git config --unset-all core.editor for i in $vi EDITOR VISUAL core_editor GIT_EDITOR do - echo "Edited by $i" >expect - case "$i" in - core_editor) - git config core.editor ./e-core_editor.sh - ;; - [A-Z]*) - eval "$i=./e-$i.sh" - export $i - ;; - esac test_expect_success "Using $i (override)" ' + echo "Edited by $i" >expect && + case "$i" in + core_editor) + test_config core.editor ./e-core_editor.sh + ;; + [A-Z]*) + test_when_finished "unset $i" && + eval "$i=./e-$i.sh" && + export $i + ;; + esac && + git --exec-path=. commit --amend && git show -s --pretty=oneline | sed -e "s/^[0-9a-f]* //" >actual && > @@ -119,13 +121,10 @@ > > ' > > -unset GIT_EDITOR > test_expect_success 'core.editor with a space' ' > - > - git config core.editor \"./e\ space.sh\" && > + test_config core.editor \"./e\ space.sh\" && > git commit --amend && > test space = "$(git show -s --pretty=format:%s)" > - > ' This hunk looks good to me. Patrick
