Allow to specify the commit message of a new patchset of the test API

Change-Id: I9568fba0138ccccb16e64fc154ca11c412ed2a1e

java/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImpl.java

@@ -348,7 +348,7 @@ public class ChangeOperationsImpl implements ChangeOperations {
         ChangeNotes changeNotes,
         TestPatchsetCreation patchsetCreation,
         Timestamp now)
-        throws IOException {
+        throws IOException, BadRequestException {
       ObjectId oldPatchsetCommitId = changeNotes.getCurrentPatchSet().commitId();
       RevCommit oldPatchsetCommit = repository.parseCommit(oldPatchsetCommitId);
 
@@ -356,7 +356,10 @@ public class ChangeOperationsImpl implements ChangeOperations {
           createNewTree(
               repository, revWalk, oldPatchsetCommitId, patchsetCreation.treeModifications());
 
-      String commitMessage = oldPatchsetCommit.getFullMessage();
+      String commitMessage =
+          correctCommitMessage(
+              changeNotes.getChange().getKey().get(),
+              patchsetCreation.commitMessage().orElseGet(oldPatchsetCommit::getFullMessage));
 
       ImmutableList<ObjectId> parentCommitIds = getParents(oldPatchsetCommit);
       PersonIdent author = getAuthor(oldPatchsetCommit);
@@ -364,6 +367,19 @@ public class ChangeOperationsImpl implements ChangeOperations {
       return createCommit(objectInserter, tree, parentCommitIds, author, committer, commitMessage);
     }
 
+    private String correctCommitMessage(String oldChangeId, String desiredCommitMessage)
+        throws BadRequestException {
+      String commitMessage = CommitMessageUtil.checkAndSanitizeCommitMessage(desiredCommitMessage);
+
+      // Remove initial 'I' and treat the rest as ObjectId. This is not the cleanest approach but
+      // unfortunately, we don't seem to have other utility code which takes the string-based
+      // change-id and ensures that it is part of the commit message.
+      ObjectId id = ObjectId.fromString(oldChangeId.substring(1));
+      commitMessage = ChangeIdUtil.insertId(commitMessage, id, false);
+
+      return commitMessage;
+    }
+
     private PersonIdent getAuthor(RevCommit oldPatchsetCommit) {
       return Optional.ofNullable(oldPatchsetCommit.getAuthorIdent()).orElse(serverIdent);
     }

java/com/google/gerrit/acceptance/testsuite/change/TestPatchsetCreation.java

@@ -19,11 +19,14 @@ import com.google.common.collect.ImmutableList;
 import com.google.gerrit.acceptance.testsuite.ThrowingFunction;
 import com.google.gerrit.entities.PatchSet;
 import com.google.gerrit.server.edit.tree.TreeModification;
+import java.util.Optional;
 
 /** Initial attributes of the patchset. If not provided, arbitrary values will be used. */
 @AutoValue
 public abstract class TestPatchsetCreation {
 
+  public abstract Optional<String> commitMessage();
+
   public abstract ImmutableList<TreeModification> treeModifications();
 
   abstract ThrowingFunction<TestPatchsetCreation, PatchSet.Id> patchsetCreator();
@@ -36,6 +39,8 @@ public abstract class TestPatchsetCreation {
   @AutoValue.Builder
   public abstract static class Builder {
 
+    public abstract Builder commitMessage(String commitMessage);
+
     /** Modified file of the patchset. The file content is specified via the returned builder. */
     public FileContentBuilder<Builder> file(String filePath) {
       return new FileContentBuilder<>(this, filePath, treeModificationsBuilder()::add);

javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java

@@ -362,6 +362,58 @@ public class ChangeOperationsImplTest extends AbstractDaemonTest {
     assertThat(patchsetRevision.kind).isEqualTo(ChangeKind.NO_CHANGE);
   }
 
+  @Test
+  public void newPatchsetCanHaveUpdatedCommitMessage() throws Exception {
+    Change.Id changeId = changeOperations.newChange().commitMessage("Old message").create();
+
+    changeOperations.change(changeId).newPatchset().commitMessage("New message").create();
+
+    ChangeInfo change = getChangeFromServer(changeId);
+    CommitInfo currentPatchsetCommit = change.revisions.get(change.currentRevision).commit;
+    assertThat(currentPatchsetCommit).message().startsWith("New message");
+  }
+
+  @Test
+  public void updatedCommitMessageOfNewPatchsetAutomaticallyKeepsChangeId() throws Exception {
+    Change.Id numericChangeId = changeOperations.newChange().commitMessage("Old message").create();
+    String changeId = changeOperations.change(numericChangeId).get().changeId();
+
+    changeOperations.change(numericChangeId).newPatchset().commitMessage("New message").create();
+
+    ChangeInfo change = getChangeFromServer(numericChangeId);
+    CommitInfo currentPatchsetCommit = change.revisions.get(change.currentRevision).commit;
+    assertThat(currentPatchsetCommit).message().contains("Change-Id: " + changeId);
+  }
+
+  @Test
+  public void newPatchsetCanHaveDifferentChangeIdFooter() throws Exception {
+    Change.Id numericChangeId =
+        changeOperations
+            .newChange()
+            .commitMessage("Old message\n\nChange-Id: I1111111111111111111111111111111111111111")
+            .create();
+
+    // Specifying another change-id is not an officially supported behavior of Gerrit but we might
+    // need this for some test scenarios and hence we support it in the test API.
+    changeOperations
+        .change(numericChangeId)
+        .newPatchset()
+        .commitMessage("New message\n\nChange-Id: I0123456789012345678901234567890123456789")
+        .create();
+
+    ChangeInfo change = getChangeFromServer(numericChangeId);
+    CommitInfo currentPatchsetCommit = change.revisions.get(change.currentRevision).commit;
+    assertThat(currentPatchsetCommit)
+        .message()
+        .contains("Change-Id: I0123456789012345678901234567890123456789");
+    assertThat(currentPatchsetCommit)
+        .message()
+        .doesNotContain("Change-Id: I1111111111111111111111111111111111111111");
+    // Actual change-id should not have been updated.
+    String changeId = changeOperations.change(numericChangeId).get().changeId();
+    assertThat(changeId).isEqualTo("I1111111111111111111111111111111111111111");
+  }
+
   @Test
   public void newPatchsetCanHaveReplacedFileContent() throws Exception {
     Change.Id changeId = changeOperations.newChange().file("file1").content("Line 1").create();