Subject: Re: [PATCH v6 2/6] t6030-bisect-porcelain: add test for bisect visualize

From: Junio C Hamano <[email-masked]>
Date: Thu, 02 Sep 2021 15:05:46 -0700

Miriam Rubio <[email-masked]> writes:

> Add a test to control breakages in bisect visualize command.
>
> Signed-off-by: Miriam Rubio <[email-masked]>
> ---
>  t/t6030-bisect-porcelain.sh | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
> index e61b8143fd..f13eeac9ce 100755
> --- a/t/t6030-bisect-porcelain.sh
> +++ b/t/t6030-bisect-porcelain.sh
> @@ -973,4 +973,11 @@ test_expect_success 'bisect run fails with exit code equals or greater than 128'
>  	test_must_fail git bisect run ./test_script.sh >> my_bisect_log.txt
>  '
>  
> +test_expect_success 'bisect visualize with a filename with dash and space' '
> +	echo "My test line" >> -hello\ 2 &&

The same style guide for redirection applies here.

Also, it makes sense to quote such an unusual filename for human
readers, i.e.

	echo "My test line" >"./-hello 2" &&

> +	git add -- -hello\ 2 &&
> +	git commit --quiet -m "Add test line" -- -hello\ 2 &&

Likewise.  

Especially since this is not a test for "git add" or "git commit",
instead of writing "-hello 2", "./-hello 2" may help human readers
better.

> +	git bisect visualize -p -- -hello\ 2 > my_bisect_log.txt

This one, if it is meant to test the pathspec parsing of the command
being tested (i.e. "git bisect"), is probably better to be left
without "./" prefix, i.e. "-hello 2".

The same comment applies to the redirection into my_bisect_log.txt
file.  It is better not to redirect this at all.

This is the first use of "git bisect visualize" in our tests.  How
are we making sure that we won't open gitk and leave it hanging and
doing silly things like that?

    ... goes and looks ...

Ah, OK.  "git bisect --help" makes it clear that giving an option
like "-p" tells us to run "git log", so we are OK.

THanks.

> +'
> +
>  test_done