Re: [PATCH v2 3/4] t4203: test mailmap functionality directly rather than indirectly

Eric Sunshine wrote:

On Thu, Jul 11, 2013 at 3:07 PM, Junio C Hamano [off-list ref] wrote:

quoted

Eric Sunshine [off-list ref] writes:

quoted

quoted

With the introduction of check-mailmap, it is now possible to check
.mailmap functionality directly rather than indirectly as a side-effect
of other commands (such as git-shortlog), therefore, do so.

Does this patch mean that we will now ignore future breakages in
shortlog and blame if their mailmap integration becomes buggy?

I am not convinced it is a good idea if that is what is going on.

I meant to mention in the cover letter that this patch was open for
debate, however, it does not eliminate all testing of these other
commands.

The tests in which git-check-mailmap is substituted for git-shortlog
all worked against a simplistic two-commit repository. Those tests
were checking the low-level mailmap functionality under various
conditions and configurations; they were not especially checking any
particular behavior of git-shortlog.

There still remain a final eight tests which cover git-shortlog,
git-log, and git-blame. These tests do check mailmap-related behavior
of those commands, and they do so using a more involved seven-commit
repository with "complex" mappings, so we have not necessarily lost
any checks of mailmap integration for those commands.

Would this patch become more palatable if I stated the above in the
commit message?

My current thinking is "no" --- the patch has as a justification "Now
we can test these aspects of .mailmap handling directly with a
low-level tool instead of using the tool most people will use, so do
so", which sounds an awful lot like "Reduce test coverage of commonly
used tools, because we can".

But I imagine the actual motivation was something other than "because
we can".  For example, maybe the idea is that after this patch, it
should be easier to make cosmetic improvements to the shortlog, log,
and blame output and only have to change those final 8 tests that are
adequately covering the output?  If you have such plans and this patch
makes them easier, it could sound like a good patch as a stepping
stone toward that.

Thanks and hope that helps,
Jonathan