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
