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.
