From: Junio C Hamano <gitster@pobox•com>
To: Johannes Schindelin <Johannes.Schindelin@gmx•de>
Cc: Phillip Wood <phillip.wood@dunelm•org.uk>,
Elijah Newren <newren@gmail•com>,
Phillip Wood via GitGitGadget <gitgitgadget@gmail•com>,
Git Mailing List <git@vger•kernel.org>,
Victor Gambier <vgambier@excilys•com>
Subject: Re: [PATCH 1/3] t3403: fix commit authorship
Date: Sun, 15 Aug 2021 10:36:47 -0700 [thread overview]
Message-ID: <xmqqsfzasj80.fsf@gitster.g> (raw)
In-Reply-To: <nycvar.QRO.7.76.6.2108142350420.59@tvgsbejvaqbjf.bet> (Johannes Schindelin's message of "Sat, 14 Aug 2021 23:53:06 +0200 (CEST)")
Johannes Schindelin <Johannes.Schindelin@gmx•de> writes:
>> > Good point. The commit tagged with amended-goodbye is later used in
>> > some tests that ensure the author ident does not change across a
>> > rebase. If this commit gets created without authorship customized
>> > (i.e. before Phillip's fix), we would not catch a possible breakage
>> > to make rebase discard the original authorship information.
>> >
>> > But with this fix, we now can catch such a breakage.
>>
>> I'll expand the commit message to make that clear
>
> Maybe you could even add a `test another.author@example•com = $(git show
> -s --format=%ae HEAD)`?
The version I have from Phillip has updated log message already, but
not with such a regression prevention.
The test that the patch under discussion corrects does this:
test_expect_success 'correct authorship when committing empty pick' '
test_when_finished "git rebase --abort" &&
test_must_fail git rebase -i --onto goodbye \
amended-goodbye^ amended-goodbye &&
git commit --allow-empty &&
git log --pretty=format:"%an <%ae>%n%ad%B" -1 amended-goodbye >expect &&
git log --pretty=format:"%an <%ae>%n%ad%B" -1 HEAD >actual &&
test_cmp expect actual
'
to ensure that the authorship is the same between the original
(i.e. amended-goodbye) and the rebased (i.e. HEAD), with the
expectation that a bug may lose the authorship and instead use the
default one used in the test suite. What this test truly cares is
not that amended-goodbye was authored by another.author, but it was
not written by the default author.
We could test both, like the attached patch, for completeness. The
first half makes sure amended-goodbye (the original) was written by
the another.author, and the other one makes sure that author is not
the one we use to prepare commits for the tests by default.
I do not think the latter is actually a good idea ("As long as the
command produces a result different from THIS, any random garbage is
accepted" does not make a good test), so perhaps the first half
would be good enough.
t/t3403-rebase-skip.sh | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git c/t/t3403-rebase-skip.sh w/t/t3403-rebase-skip.sh
index e26762d0b2..1405720767 100755
--- c/t/t3403-rebase-skip.sh
+++ w/t/t3403-rebase-skip.sh
@@ -40,6 +40,14 @@ test_expect_success setup '
test_tick &&
git tag amended-goodbye &&
+ # Make sure the authorship info is different from the default one
+ echo "Another Author <another.author@example•com>" >expect &&
+ git log --pretty=format:"%an <%ae>" -1 amended-goodbye >actual &&
+ test_cmp expect actual &&
+
+ git log --pretty=format:"%an <%ae>" -1 goodbye >unexpect &&
+ ! test_cmp unexpect actual &&
+
git checkout -f skip-reference &&
echo moo > hello &&
git commit -a -m "we should skip this" &&
next prev parent reply other threads:[~2021-08-15 17:36 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-10 9:31 [PATCH 0/3] rebase --continue: remove .git/MERGE_MSG Phillip Wood via GitGitGadget
2021-08-10 9:31 ` [PATCH 1/3] t3403: fix commit authorship Phillip Wood via GitGitGadget
2021-08-10 17:01 ` Elijah Newren
2021-08-10 18:43 ` Junio C Hamano
2021-08-12 10:04 ` Phillip Wood
2021-08-14 21:53 ` Johannes Schindelin
2021-08-15 17:36 ` Junio C Hamano [this message]
2021-08-15 20:04 ` Phillip Wood
2021-08-16 16:36 ` Junio C Hamano
2021-08-17 10:05 ` Phillip Wood
2021-08-10 9:31 ` [PATCH 2/3] rebase --apply: restore some tests Phillip Wood via GitGitGadget
2021-08-10 16:58 ` Elijah Newren
2021-08-12 10:03 ` Phillip Wood
2021-08-10 9:31 ` [PATCH 3/3] rebase --continue: remove .git/MERGE_MSG Phillip Wood via GitGitGadget
2021-08-10 17:03 ` [PATCH 0/3] " Elijah Newren
2021-08-12 13:42 ` [PATCH v2 " Phillip Wood via GitGitGadget
2021-08-12 13:42 ` [PATCH v2 1/3] t3403: fix commit authorship Phillip Wood via GitGitGadget
2021-08-12 13:42 ` [PATCH v2 2/3] rebase --apply: restore some tests Phillip Wood via GitGitGadget
2021-08-12 13:42 ` [PATCH v2 3/3] rebase --continue: remove .git/MERGE_MSG Phillip Wood via GitGitGadget
2021-08-13 23:01 ` Junio C Hamano
2021-08-14 20:01 ` Phillip Wood
2021-08-13 0:46 ` [PATCH v2 0/3] " Elijah Newren
2021-08-13 15:31 ` Junio C Hamano
2021-08-13 17:21 ` Elijah Newren
2021-08-14 20:02 ` Phillip Wood
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=xmqqsfzasj80.fsf@gitster.g \
--to=gitster@pobox$(echo .)com \
--cc=Johannes.Schindelin@gmx$(echo .)de \
--cc=git@vger$(echo .)kernel.org \
--cc=gitgitgadget@gmail$(echo .)com \
--cc=newren@gmail$(echo .)com \
--cc=phillip.wood@dunelm$(echo .)org.uk \
--cc=vgambier@excilys$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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