---- On Wed, 06 Aug 2025 21:19:57 +0800 Phillip Wood <phillip.wood123@xxxxxxxxx> wrote ---
> Hi Li
>
> I had a couple more thoughts about the tests ...
>
> On 06/08/2025 11:28, Phillip Wood wrote:
> > On 03/08/2025 16:00, Li Chen wrote:
> >> +create_expect() {
> >> + cat >"$1" <<-EOF
> >> + $2
> >> +
> >> + Reviewed-by: Dev <dev@xxxxxxxxxxx>
> >> + EOF
> >> +}
> >> +
> >> +test_expect_success 'setup repo with a small history' '
> >> [...]
> >> + create_expect third-signed "third" &&>> +
> create_expect conflict-signed "conflict"
> >
> > Normally we create the "expect" file in the test where it is used.
>
> Thinking about this some more, if we want to use test_commit_message
> then I think we can change create_expect to write to stdout and do
>
> test_commit_message HEAD <<-EOF
> $(create_expect first)
> EOF
>
> rather than having to create a file.
>
> >> +
> >> +test_expect_success 'reject empty --trailer argument' '
> >> [...]
> >> +test_expect_success 'reject trailer with missing key before separator' '
>
> Should we also test for a missing value or are trailers without a value
> allowed?
>
> >> + git rebase -m \
> >> + --trailer "Signed-off-by: Dev A <a@xxxxxx>" \
> >> + --trailer "Signed-off-by: Dev B <b@xxxxxx>" HEAD~1 &&
>
> Lets use example.com here rather than some random domain that might
> actually exist.
>
> >> +test_expect_success 'rebase -m --trailer adds trailer after conflicts' '
> >> + git reset --hard third &&
> >> + test_must_fail git rebase -m \
> >> + --trailer "Reviewed-by: Dev <dev@xxxxxxxxxxx>" \
> >> + second third &&
> >> + git checkout --theirs file &&
> >> + git add file &&
> >> + git rebase --continue &&
>
> This checks that the commit with conflicts has a trailer added but it
> does not check that the commits picked by "git rebase --continue" do. To
> check that we actually save the trailers and use them when continuing we
> need to add a fourth commit on top of third and check that has a trailer
> add here as well.
>
> A couple more thoughts:
>
> - We should check that
> git -c trailer.review.key=Reviewed-by rebase \
> --trailer=review="Dev <dev@xxxxxxxxxxx>"
> adds a "Reviewed-by:" trailer. We can do that by changing one of the
> tests in this patch rather than adding a new one. This checks that we
> accept '=' as a separator as well a respecting the config.
>
> - We should check that the todo list
> pick first
> fixup second
> adds the trailer as expected and that
> pick first
> fixup -C second
> also works. To do that we will need to source lib-rebase.sh at the
> start of the test file and add a test that uses set_replace_editor()
> which should be called in a subshell.
>
>
> Do please ask if you have any questions about these suggestions
>
> Thanks
>
> Phillip
>
>
Regards,
Li
