Re: [PATCH 1/7] branch -h: show usage even in an invalid repository

Jonathan Nieder [off-list ref] writes:

[jn: simplified; new commit message, test]

Overall (not just this 1/7) the messages justify the changes a lot
better.  Thanks for a pleasant read ;-).

A handful of points that struck me (not just test, but taking tests from
other patches together) as slightly odd, all minor:

 - Why '[Uu]sage"?  Should we make the messages consistent further?

 - The final "grep" check may want to make sure that the message is free of
   "fatal" (or "error", "warning", etc.) as well?

 - Each test seems to anticipate a specific kind of breakage tailored for
   the command being tested (e.g. "branch" test does not corrupt config
   nor the index).  Perhaps "lib-corrupt.sh" helper to run the same test
   in variety of corrupt repositories would help improve coverage?  [*1*]

 - Some tests redirect both the standard output and the standard error
   (like this patch) and check the combined result, while some others
   (e.g. 2/7) check only the standard error stream.  Don't we want to be
   testing them more uniformly?

 test_expect_success \
     'git branch abc should create a branch' \
     'git branch abc && test -f .git/refs/heads/abc'

Don't we want to rather use resolve-ref or "rev-parse --verify", just in
case we may later change "git branch" to update packed-refs directly?


[Footnote]

*1* I am of two minds, as for example a corrupt "gc.pruneexpire" cannot
possibly matter for correct operation of "git branch", but am just
throwing out an idea to see if somebody else have clever ideas.