From: Jonathan Nieder <jrnieder@gmail.com>
To: Felipe Contreras <felipe.contreras@gmail.com>
Cc: git@vger.kernel.org, Jeff King <peff@peff.net>
Subject: [PATCH v3 0/2] test: tests for the "double > from mailmap" bug
Date: Tue, 14 Feb 2012 14:34:31 -0600 [thread overview]
Message-ID: <20120214203431.GB13210@burratino> (raw)
In-Reply-To: <1329235894-20581-1-git-send-email-felipe.contreras@gmail.com>
Hi again,
Felipe Contreras wrote:
> I sent both the fix and the tests. Another fix was applied, but we are still
> missing the tests.
>
> These are good before, and after the fix.
To summarize the previous discussion[1]: some people had comments, and
you seem to have found value in exactly none of them. OK. CC-ing
Peff, since he at least probably has looked over this code before.
By the way, the address you are using for Marius is out of date.
> Felipe Contreras (2):
> t: mailmap: add 'git blame -e' tests
So that people don't destroy this test in later refactorings, I would
like to collect statements that we want the test to ensure remain
true.
Apparently the fix in f026358e ("mailmap: always return a plain mail
address from map_user()", 2012-02-05) was for the case of the name
changing and email address not changing due to mailmap mapping. Most
callers use a separate buffer for the email address so there is room
to modify the name in place, but "git blame" keeps angle brackets in
the same buffer for no obvious reason I can see. (Callers should
be able to add the brackets themselves instead of relying on
ci.author_mail to contain them, but that's a story for another day.)
Anyway, the existing tests for the returned email missed this since
it does not affect "git shortlog -e" but only "git blame -e".
Therefore this patch reuses the test data for shortlog -e and lets us
use it for blame, too. It is easier to understand after the other
patch, IMHO.
This is _not_ meant as a more general test for the "git blame -e"
format (which would belong somewhere near t8008) as far as I can tell.
It is just checking that mailmap doesn't screw up.
> t: mailmap: add simple name translation test
Before, I thought this might be a straightforward test for the bug
fixed by f026358e. That didn't justify the patch that touches
several different test assertions.
In fact it seems to be intended to test the case addressed by f026358e
(name changing, email not) in various mailmap callers: "git shortlog -e",
"git log --pretty", "git blame".
Here's a reroll.
Enjoy,
Jonathan
Felipe Contreras (2):
test: mailmap can change author name without changing email
test: check that "git blame -e" uses mailmap correctly
t/t4203-mailmap.sh | 28 ++++++++++++++++++++++++++++
1 files changed, 28 insertions(+), 0 deletions(-)
[1] http://thread.gmane.org/gmane.comp.version-control.git/189896
next prev parent reply other threads:[~2012-02-14 20:34 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-02-14 16:11 [PATCH 0/2] t: add blame -e tests for mailmap Felipe Contreras
2012-02-14 16:11 ` [PATCH 1/2] t: mailmap: add 'git blame -e' tests Felipe Contreras
2012-02-14 16:11 ` [PATCH 2/2] t: mailmap: add simple name translation test Felipe Contreras
2012-02-14 20:10 ` Junio C Hamano
2012-02-14 20:28 ` Felipe Contreras
2012-02-14 20:34 ` Jonathan Nieder [this message]
2012-02-14 20:35 ` [PATCH 1/2] test: mailmap can change author name without changing email Jonathan Nieder
2012-02-14 21:35 ` Felipe Contreras
2012-02-14 21:50 ` Jonathan Nieder
2012-02-14 22:48 ` Felipe Contreras
2012-02-14 20:36 ` [PATCH 2/2] test: check that "git blame -e" uses mailmap correctly Jonathan Nieder
2012-02-14 21:41 ` Felipe Contreras
2012-02-14 21:59 ` Jonathan Nieder
2012-02-14 22:56 ` Felipe Contreras
2012-02-14 21:06 ` [PATCH v3 0/2] test: tests for the "double > from mailmap" bug Felipe Contreras
2012-02-14 21:15 ` Jonathan Nieder
2012-02-14 22:09 ` Felipe Contreras
2012-02-14 22:21 ` Jonathan Nieder
2012-02-14 22:36 ` Felipe Contreras
2012-02-14 21:14 ` Jeff King
2012-02-14 21:27 ` Jonathan Nieder
2012-02-14 21:52 ` Felipe Contreras
2012-02-14 22:07 ` Jeff King
2012-02-14 22:22 ` Felipe Contreras
2012-02-14 22:35 ` Jeff King
2012-02-14 22:18 ` Junio C Hamano
2012-02-14 22:34 ` Felipe Contreras
2012-02-14 22:49 ` Junio C Hamano
2012-02-14 23:14 ` Felipe Contreras
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=20120214203431.GB13210@burratino \
--to=jrnieder@gmail.com \
--cc=felipe.contreras@gmail.com \
--cc=git@vger.kernel.org \
--cc=peff@peff.net \
/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).