Re: Test "t/t7502-commit.sh" failed
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 26, 2012, 16:34 UTC
- Message-ID
- <7vtxwu8orw.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <20120726130348.GA965@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 46 quoted lines
> 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 <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.
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>".