Re: [PATCH 1/3] t3403: fix commit authorship
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 15, 2021, 17:36 UTC
- Message-ID
- <xmqqsfzasj80.fsf@gitster.g>
- In-Reply-To
- <nycvar.QRO.7.76.6.2108142350420.59@tvgsbejvaqbjf.bet>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 12 quoted lines
>> > 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" &&