Subject: Re: [PATCH] test: add test for git bisect skip with --term* arguments

From: Junio C Hamano <[email-masked]>
Date: Tue, 20 Apr 2021 11:08:55 -0700

Bagas Sanjaya <[email-masked]> writes:

> NOTE: this patch is not intended for integrating into git.git, but
> rather this patch is written to demonstrate this breakage. I hope that
> the test can be added to t6030-bisect-porcelain.sh, and make this patch
> redundant.  

Then perhaps add it to where you think it belongs to before you
post?

In any case, let's critique the patch a bit for future reference,
with a hope that you'd be contributing more ;-).

>  t/t6031-bisect-skip.sh | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
>  create mode 100755 t/t6031-bisect-skip.sh

Good; many new people forget to make these executable.

> +test_description='Tests git bisect --skip'
> +
> +exec </dev/null

Why?

> +GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
> +export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
> +
> +. ./test-lib.sh
> +
> +# hash variables
> +HASH_SKIPPED_FROM=
> +HASH_SKIPPED_TO=
> +
> +# initialize testing repo
> +init() {

Style.

> +	for i in `seq 1 20`; do

Use $(test_seq ...)

> +		echo $i >> test &&

Style

> +		git add test && git commit -m $i
> +	done
> +}
> +
> +init

We do that in the first "test_expect_success setup '... code ...'" block
to catch breakages in the initialization.

> +test_expect_success 'test moving HEAD when skip bisecting' '
> +	git bisect start --term-new=ok --term-old=whoops HEAD HEAD~9 &&
> +	HASH_SKIPPED_FROM=$(git rev-parse --verify HEAD) &&
> +	git bisect skip &&
> +	HASH_SKIPPED_TO=$(git rev-parse --verify HEAD) &&
> +	test $HASH_SKIPPED_FROM != $HASH_SKIPPED_TO
> +'

Missing "test_done".

Thanks.