Subject: Re: [PATCH v2 05/21] t6030: generalize test to not rely on current implementation

From: Torsten Bögershausen <[email-masked]>
Date: Sun, 10 Apr 2016 15:47:27 +0200

On 10.04.16 15:18, Stephan Beyer wrote:
Some nit-comments inline

> ---
>  t/t6030-bisect-porcelain.sh | 167 ++++++++++++++++++++++----------------------
>  1 file changed, 85 insertions(+), 82 deletions(-)
> 
> diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
> index 05bc639..645ccd9 100755
> --- a/t/t6030-bisect-porcelain.sh
> +++ b/t/t6030-bisect-porcelain.sh
> @@ -10,36 +10,34 @@ exec </dev/null
> +	if [ -f "$_file" ]; then
I know that the old code did the same, is there a chance
to adopt to the git-style:
	if test -f "$_file" ; then
> +		echo "$_line" >> $_file || return $?

[]
> +test_expect_success '"git bisect run" simple case' '
> +	echo "#"\!"/bin/sh" > test_script.sh &&
> +	echo "grep Another hello > /dev/null" >> test_script.sh &&
> +	echo "test \$? -ne 0" >> test_script.sh &&
> +	chmod +x test_script.sh &&
> +	git bisect start &&
> +	git bisect good $HASH1 &&
> +	git bisect bad $HASH4 &&
> +	git bisect run ./test_script.sh > my_bisect_log.txt &&
> +	grep "$HASH3 is the first bad commit" my_bisect_log.txt &&
> +	git bisect reset
> +'
Portabily:
Since yesterday/yesterweek the usage of hard-coded
#!/bin/sh had shown to be problematic
Junio posted an update like this:
-	printf "#!/bin/sh\n" >diff &&
-	printf "printf \"\$GIT_PREFIX\"" >>diff &&
-	chmod +x diff &&
+	write_script diff <<-\EOF &&
+	printf "%s" "$GIT_PREFIX"
+	EOF

(Same for the scripts below)

>
> +test_expect_success '"git bisect run" with more complex "git bisect start"' '
> +	echo "#"\!"/bin/sh" > test_script.sh &&
> +	echo "grep Ciao hello > /dev/null" >> test_script.sh &&
> +	echo "test \$? -ne 0" >> test_script.sh &&
Style nit, please no ' ' after ">>":
echo "test \$? -ne 0" >>test_script.sh &&

(and more below)