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

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 20, 2021, 18:08 UTC
Message-ID: <xmqqa6ps7rrc.fsf@gitster.g>
In-Reply-To: <20210420123435.35936-1-bagasdotme@gmail.com>

Bagas Sanjaya <bagasdotme@gmail.com> 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?

Show 11 quoted lines

> +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

Show 5 quoted lines

> +		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.

Show 7 quoted lines

> +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.