Re: Test "t/t7502-commit.sh" failed

[Jeff King <peff@peff.net>]

On Thu, Jul 26, 2012 at 02:27:52PM +0800, Jiang Xin wrote:

> Test "t/t7502-commit.sh" failed. I guess it's commit
> v1.7.9.7-1-gf20f387 which breaks it.
> 
>     $ git log -1 --oneline --stat v1.7.9.7-1-gf20f387
>     f20f commit: check committer identity more strictly
>      builtin/commit.c | 2 +-
>      1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Result of t/t7502-commit.sh:
> 
> 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 <author@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. On some systems we can, and on some we can't, depending on
things like how /etc/passwd and the hostname are configured.

I'll see if I can make it more robust, but I think the right solution
may simply be to rip out the test.

-Peff