From: Phillip Wood <phillip.wood123@gmail.com>
Date: Mon, 11 Aug 2025 10:04:34 GMT
Subject: Re: [PATCH 1/3] t7005: use modern test style
Message-ID: <9cccf70a-26f2-424d-8d44-3b87dab97d65@gmail.com>
In-Reply-To: <20250810160323.49372-2-ben.knoble+github@gmail.com>

Hi Ben

On 10/08/2025 17:03, D. Ben Knoble wrote:
> 
> diff --git a/t/t7005-editor.sh b/t/t7005-editor.sh
> index 5fcf281dfb..2f59fc0549 100755
> --- a/t/t7005-editor.sh
> +++ b/t/t7005-editor.sh
> @@ -7,10 +7,8 @@
>   unset EDITOR VISUAL GIT_EDITOR
>   
>   test_expect_success 'determine default editor' '
> -
>   	vi=$(TERM=vt100 git var GIT_EDITOR) &&
>   	test -n "$vi"
> -
>   '
>   
>   if ! expr "$vi" : '[a-z]*$' >/dev/null

All of the code starting here that is outside of a test could usefully 
be moved inside the "setup" test below it. It could also be cleaned up 
to use write_script() to create the editor scripts as well.

Thanks

Phillip

> @@ -33,19 +31,16 @@
>   fi
>   
>   test_expect_success setup '
> -
>   	msg="Hand-edited" &&
>   	test_commit "$msg" &&
>   	echo "$msg" >expect &&
> -	git show -s --format=%s > actual &&
> +	git show -s --format=%s >actual &&
>   	test_cmp expect actual
> -
>   '
>   
>   TERM=dumb
>   export TERM
>   test_expect_success 'dumb should error out when falling back on vi' '
> -
>   	if git commit --amend
>   	then
>   		echo "Oops?"
> @@ -56,13 +51,13 @@
>   '
>   
>   test_expect_success 'dumb should prefer EDITOR to VISUAL' '
> -
>   	EDITOR=./e-EDITOR.sh &&
>   	VISUAL=./e-VISUAL.sh &&
>   	export EDITOR VISUAL &&
>   	git commit --amend &&
> -	test "$(git show -s --format=%s)" = "Edited by EDITOR"
> -
> +	echo "Edited by EDITOR" >expect &&
> +	git show -s --format=%s >actual &&
> +	test_cmp expect actual
>   '
>   
>   TERM=vt100
> @@ -83,8 +78,8 @@
>   	esac
>   	test_expect_success "Using $i" '
>   		git --exec-path=. commit --amend &&
> -		git show -s --pretty=oneline |
> -		sed -e "s/^[0-9a-f]* //" >actual &&
> +		git show -s --pretty=oneline >show &&
> +		<show sed -e "s/^[0-9a-f]* //" >actual &&
>   		test_cmp expect actual
>   	'
>   done
> @@ -105,8 +100,8 @@
>   	esac
>   	test_expect_success "Using $i (override)" '
>   		git --exec-path=. commit --amend &&
> -		git show -s --pretty=oneline |
> -		sed -e "s/^[0-9a-f]* //" >actual &&
> +		git show -s --pretty=oneline >show &&
> +		<show sed -e "s/^[0-9a-f]* //" >actual &&
>   		test_cmp expect actual
>   	'
>   done
> @@ -115,17 +110,18 @@
>   	echo "echo space >\"\$1\"" >"e space.sh" &&
>   	chmod a+x "e space.sh" &&
>   	GIT_EDITOR="./e\ space.sh" git commit --amend &&
> -	test space = "$(git show -s --pretty=format:%s)"
> -
> +	echo space >expect &&
> +	git show -s --pretty=tformat:%s >actual &&
> +	test_cmp expect actual
>   '
>   
>   unset GIT_EDITOR
>   test_expect_success 'core.editor with a space' '
> -
>   	git config core.editor \"./e\ space.sh\" &&
>   	git commit --amend &&
> -	test space = "$(git show -s --pretty=format:%s)"
> -
> +	echo space >expect &&
> +	git show -s --pretty=tformat:%s >actual &&
> +	test_cmp expect actual
>   '
>   
>   test_done