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

["Kristoffer Haugsbakk" <code@khaugsbakk.name>]

On Tue, Mar 5, 2024, at 02:25, Junio C Hamano wrote:
> 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.

I reckon that this can be worth doing now as long as no other topics in
`next` or `seen` happen to touch the same code. What do you think? I can
evict hunks if they happen to overlap with other in-flight topics.

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

Okay. I could undo this change and remove the test in its own commit?

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

Sure, I’ll remove `(setup)`. The test name suggests that the test
depends on the previous one in any case.