Subject: Re: [PATCH v3 1/5] t3200: improve test style

From: Junio C Hamano <[email-masked]>
Date: Mon, 04 Mar 2024 17:25:28 -0800

Kristoffer Haugsbakk <[email-masked]> writes:

> Also:
>
> • Remove a now-irrelevant comment about test placement and switch back
>   to `main` post-test.
> • Prefer indented literal heredocs (`-\EOF`) except for a block which
>   says that this is intentional
> • Move a `git config` command into the test and mark it as `setup` since
>   the next test depends on it
>

Especially the change to use "-\EOF" to make them align better
caused too many tests to be touched, but overall the result may have
become much easier to follow.  Good job.

> -mv .git/config .git/config-saved
> -
>  test_expect_success DEFAULT_REPO_FORMAT 'git branch -m q q2 without config should succeed' '
> +	test_when_finished mv .git/config-saved .git/config &&
> +	mv .git/config .git/config-saved &&
>  	git branch -m q q2 &&
>  	git branch -m q2 q
>  '
>  
> -mv .git/config-saved .git/config

The above is a truly valuable clean-up.

But I am not really sure if the paritcular condition is worth
testing in the first place these days.  No configuration file means
we cannot even read the repository format version, and working under
such a condition is quite a bad promise that we would rather not to
having to keep.  But that is an entirely different topic from what
this patch is doing.

> -git config branch.s/s.dummy Hello
> -
> -test_expect_success 'git branch -m s/s s should work when s/t is deleted' '
> +test_expect_success '(setup) git branch -m s/s s should work when s/t is deleted' '
> +	git config branch.s/s.dummy Hello &&
>  	git branch --create-reflog s/s &&
>  	git reflog exists refs/heads/s/s &&
>  	git branch --create-reflog s/t &&

I do not know if the change of the title is warranted.  It is doing
its own test, not just setup.  It may be merely donw for the side
effect of making the step unskippable, but still ....

> -# Keep this test last, as it changes the current branch

Yes, removal of this line is really appreciated ;-)

> -cat >expect <<EOF
> -$HEAD refs/heads/g/h/i@{0}: branch: Created from main
> -EOF
>  test_expect_success 'git checkout -b g/h/i -l should create a branch and a log' '
> +	test_when_finished git checkout main &&
>  	GIT_COMMITTER_DATE="2005-05-26 23:30" \
>  	git checkout -b g/h/i -l main &&
>  	test_ref_exists refs/heads/g/h/i &&
> +	cat >expect <<-EOF &&
> +	$HEAD refs/heads/g/h/i@{0}: branch: Created from main
> +	EOF
>  	git reflog show --no-abbrev-commit refs/heads/g/h/i >actual &&
>  	test_cmp expect actual
>  '

Thanks.