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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jeff King <peff@xxxxxxxx> writes:

> 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?

There are three cases with respect to ident:

 - There is a user-configured one;

 - We derive one from the system and that is syntactically correct,
   but we know from the past experience the system is often
   misconfigured.

 - We derive one from the system and that is empty.

Before your tightening commit, the latter two cases were treated the
same way and gave the reminder to the user.  After the tightening,
these were separated into two and give different results.

Perhaps the tightening was not such a good idea in the first place?
The user would have seen a bad committer ident in the log editing
session without the new code anyway, so perhaps we should have added
a messasge e.g. "Abort the editor session and do not edit further;
fix your ident first--this commit will fail anyway" there, or
something?

The second case can lead to commits that the user may have to redo
with filter-branch, as the command does not even error out.  And we
do want to make sure that the user is given a chance to verify that
the commit will carry a name that the user is happy with with this
test.  I think that is far more important than a user on a system
with broken GECOS field has to run the editor twice.


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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]