From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Jiang Xin <worldhello.net@gmail.com>, Git List <git@vger.kernel.org>
Subject: Re: Test "t/t7502-commit.sh" failed
Date: Thu, 26 Jul 2012 13:12:56 -0400 [thread overview]
Message-ID: <20120726171256.GC13942@sigill.intra.peff.net> (raw)
In-Reply-To: <7vtxwu8orw.fsf@alter.siamese.dyndns.org>
On Thu, Jul 26, 2012 at 09:34:27AM -0700, Junio C Hamano wrote:
> >> not ok - 21 committer is automatic
> [...]
> > 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.
Ah, right. I was led astray by the crappy test title. When viewed with
the test immediately prior (which checks that "Author:" is shown in the
template), it makes more sense.
> 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.
Right. You can check this only when "git var GIT_COMMITTER_IDENT" works,
and you can check the f20f387 behavior only when it does _not_ work. So
we could do something like:
(sane_unset GIT_COMMITTER_NAME &&
sane_unset GIT_COMMITTER_EMAIL &&
git var GIT_COMMITTER_IDENT >/dev/null) &&
test_set_prereq AUTOIDENT ||
test_set_prereq NOAUTOIDENT
test_expect_success AUTOIDENT \
'mention auto ident in commit template'
'...'
test_expect_success NOAUTOIDENT \
'git rejects bogus ident before starting editor'
'...'
But it is somewhat unsatisfying to only get random test coverage
depending on how your system happens to be configured. I guess we
somewhat have that already with the case-insensitivity tests.
Do we want to go that route, or just drop this test completely?
> 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>".
Agreed. They should probably just i18ngrep for "^# Committer: " or
similar.
-Peff
next prev parent reply other threads:[~2012-07-26 17:13 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-07-26 6:27 Test "t/t7502-commit.sh" failed Jiang Xin
2012-07-26 13:03 ` Jeff King
2012-07-26 16:34 ` Junio C Hamano
2012-07-26 17:12 ` Jeff King [this message]
2012-07-26 19:25 ` Junio C Hamano
2012-07-26 19:33 ` Jeff King
2012-07-26 20:04 ` Junio C Hamano
2012-07-26 20:26 ` [PATCH 0/6] t7502 fixes Jeff King
2012-07-26 20:27 ` [PATCH 1/6] t7502: clean up fake_editor tests Jeff King
2012-07-26 20:28 ` [PATCH 2/6] t7502: properly quote GIT_EDITOR Jeff King
2012-07-26 20:30 ` [PATCH 3/6] t7502: narrow checks for author/committer name in template Jeff King
2012-07-26 20:31 ` [PATCH 4/6] t7502: drop confusing test_might_fail call Jeff King
2012-07-26 20:32 ` [PATCH 5/6] t7502: handle systems where auto-identity is broken Jeff King
2012-07-26 20:32 ` [PATCH 6/6] t7502: test early quit from commit with bad ident Jeff King
2012-07-26 21:15 ` [PATCH 0/6] t7502 fixes Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20120726171256.GC13942@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=worldhello.net@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).