Jeff King <p...@peff.net> writes:

> On Thu, Jul 26, 2012 at 02:27:52PM +0800, Jiang Xin wrote:
> ...
>> not ok - 21 committer is automatic
>> #
>> #
>> #               echo >>negative &&
>> #               (
>> #                       sane_unset GIT_COMMITTER_EMAIL &&
>> #                       sane_unset GIT_COMMITTER_NAME &&
>> #                       # must fail because there is no change
>> #                       test_must_fail git commit -e -m "sample"
>> #               ) &&
>> #               head -n 8 .git/COMMIT_EDITMSG | \
>> #               sed "s/^# Committer: .*/# Committer:/" >actual
>> #               test_i18ncmp expect actual
>> #
>
> Hmm. It doesn't fail here, but that is probably because I have my name
> set properly in /etc/passwd. I think the test is bogus, though. From the
> results you report:
>
>> Contents of file expect:
>> 
>>     sample
>> 
>>     # Please enter the commit message for your changes. Lines starting
>>     # with '#' will be ignored, and an empty message aborts the commit.
>>     #
>>     # Author:    A U Thor <aut...@example.com>
>>     # Committer:
>>     #
>> 
>> Contents of file actual:
>> 
>>     sample
>
> The test is expecting us to write out the commit template for the user
> to edit. But the whole point of f20f387 is to fail early, before we ask
> the user to edit the template. So the test is trying to check that we
> wrote _something_ into the Committer field, even though that something
> would not necessarily be used to make the commit object (because the
> code path for the commit object is going to be much stricter).
>
> I am not sure that the test is really all that useful. The point seems
> to be that we fall back to some kind of system-based ident, but that is
> not portable.

I think the point is to make sure that the "# Committer:" line is
given to the reader to remind that we took the codepath that comes
up with a committer ident by using untrustworthy heuristics.  You
are correct that the usefulness of the value of system-based ident
varies between systems (that is why it is stripped out with sed),
though.

You earlier gave a reason why f20f387 (commit: check committer identity
more strictly, 2012-07-23) does not have a test for it; I think the
same reason applies why this test is unworkable.

A related tangent; all the test vectors in this script seems to be
too wide, and we probably would want to narrow them for what each
test wants to see.  For example, the test in question only wants to
see "# Committer: <some system based ident>" and it does not matter
if the template was rewritten in future versions of Git so that it
does not begin with "# Please enter...".  Similarly, the one
previous only wants to see "# Author: <different from committer>".


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to