Change test API: Allow to specify author/committer for new patch sets
Extend the test API for changes so that the author/committer can be
specified when creating new patch sets.
The author/committer can either be specified as a PersonIdent or as a
account ID. Using the account ID is more convenient, but requires that
the author/committer has a Gerrit account. Supporting PersonIdent allows
to specify an author/committer that doesn't have a Gerrit account.
Trying to specify both, an author/committer as an account ID and as a
PersonIdent, is rejected.
Same as so far, if no author/committer is specified the author/committer
of the previous patch set is used as the author/committer.
Release-Notes: skip
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: I2f3f5e0cdea6177afe6be48ada8ab632c89dc182
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImpl.java b/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImpl.java
index a45dd8a..62ad7c4 100644
--- a/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImpl.java
+++ b/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImpl.java
@@ -20,6 +20,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Streams;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.PatchSet;
@@ -457,9 +458,18 @@
ObjectInserter objectInserter = repository.newObjectInserter();
RevWalk revWalk = new RevWalk(objectInserter.newReader())) {
Instant now = TimeUtil.now();
+ PersonIdent authorIdent = getAuthorIdent(now, patchsetCreation);
+ PersonIdent committerIdent = getCommitterIdent(now, patchsetCreation);
ObjectId newPatchsetCommit =
createPatchsetCommit(
- repository, revWalk, objectInserter, changeNotes, patchsetCreation, now);
+ repository,
+ revWalk,
+ objectInserter,
+ changeNotes,
+ patchsetCreation,
+ authorIdent,
+ committerIdent,
+ now);
PatchSet.Id patchsetId =
ChangeUtil.nextPatchSetId(repository, changeNotes.getCurrentPatchSet().id());
@@ -478,12 +488,44 @@
}
}
+ @Nullable
+ private PersonIdent getAuthorIdent(Instant when, TestPatchsetCreation patchsetCreation) {
+ if (patchsetCreation.authorIdent().isPresent()) {
+ return new PersonIdent(patchsetCreation.authorIdent().get(), when);
+ }
+
+ if (patchsetCreation.author().isPresent()) {
+ return userFactory
+ .create(patchsetCreation.author().get())
+ .newCommitterIdent(when, serverIdent.getZoneId());
+ }
+
+ return null;
+ }
+
+ @Nullable
+ private PersonIdent getCommitterIdent(Instant when, TestPatchsetCreation patchsetCreation) {
+ if (patchsetCreation.committerIdent().isPresent()) {
+ return new PersonIdent(patchsetCreation.committerIdent().get(), when);
+ }
+
+ if (patchsetCreation.committer().isPresent()) {
+ return userFactory
+ .create(patchsetCreation.committer().get())
+ .newCommitterIdent(when, serverIdent.getZoneId());
+ }
+
+ return null;
+ }
+
private ObjectId createPatchsetCommit(
Repository repository,
RevWalk revWalk,
ObjectInserter objectInserter,
ChangeNotes changeNotes,
TestPatchsetCreation patchsetCreation,
+ @Nullable PersonIdent author,
+ @Nullable PersonIdent committer,
Instant now)
throws IOException, BadRequestException {
ObjectId oldPatchsetCommitId = changeNotes.getCurrentPatchSet().commitId();
@@ -499,9 +541,13 @@
changeNotes.getChange().getKey().get(),
patchsetCreation.commitMessage().orElseGet(oldPatchsetCommit::getFullMessage));
- PersonIdent author = getAuthor(oldPatchsetCommit);
- PersonIdent committer = getCommitter(oldPatchsetCommit, now);
- return createCommit(objectInserter, tree, parentCommitIds, author, committer, commitMessage);
+ return createCommit(
+ objectInserter,
+ tree,
+ parentCommitIds,
+ Optional.ofNullable(author).orElse(getAuthor(oldPatchsetCommit)),
+ Optional.ofNullable(committer).orElse(getCommitter(oldPatchsetCommit, now)),
+ commitMessage);
}
private String correctCommitMessage(String oldChangeId, String desiredCommitMessage)
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/TestPatchsetCreation.java b/java/com/google/gerrit/acceptance/testsuite/change/TestPatchsetCreation.java
index 32731c1..f8ca977 100644
--- a/java/com/google/gerrit/acceptance/testsuite/change/TestPatchsetCreation.java
+++ b/java/com/google/gerrit/acceptance/testsuite/change/TestPatchsetCreation.java
@@ -14,6 +14,8 @@
package com.google.gerrit.acceptance.testsuite.change;
+import static com.google.common.base.Preconditions.checkState;
+
import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.acceptance.testsuite.ThrowingFunction;
@@ -21,6 +23,7 @@
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.server.edit.tree.TreeModification;
import java.util.Optional;
+import org.eclipse.jgit.lib.PersonIdent;
/** Initial attributes of the patchset. If not provided, arbitrary values will be used. */
@AutoValue
@@ -28,6 +31,14 @@
public abstract Optional<Account.Id> uploader();
+ public abstract Optional<Account.Id> author();
+
+ public abstract Optional<PersonIdent> authorIdent();
+
+ public abstract Optional<Account.Id> committer();
+
+ public abstract Optional<PersonIdent> committerIdent();
+
public abstract Optional<String> commitMessage();
public abstract ImmutableList<TreeModification> treeModifications();
@@ -44,11 +55,66 @@
@AutoValue.Builder
public abstract static class Builder {
/**
- * The uploader for the new patch set. If not set the new patch set is uploaded by the change
- * owner.
+ * The uploader for the new patch set.
+ *
+ * <p>Must be an existing user account.
+ *
+ * <p>If not set the new patch set is uploaded by the change owner.
*/
public abstract Builder uploader(Account.Id uploader);
+ /**
+ * The author of the commit for which the change is created.
+ *
+ * <p>Must be an existing user account.
+ *
+ * <p>Cannot be set together with {@link #authorIdent()} is set.
+ *
+ * <p>If neither {@link #author()} nor {@link #authorIdent()} is set the {@link
+ * TestChangeCreation#owner()} is used as the author.
+ */
+ public abstract Builder author(Account.Id author);
+
+ /**
+ * The author ident of the commit for which the change is created.
+ *
+ * <p>Cannot be set together with {@link #author()} is set.
+ *
+ * <p>If neither {@link #author()} nor {@link #authorIdent()} is set the {@link
+ * TestChangeCreation#owner()} is used as the author.
+ */
+ public abstract Builder authorIdent(PersonIdent authorIdent);
+
+ public abstract Optional<Account.Id> author();
+
+ public abstract Optional<PersonIdent> authorIdent();
+
+ /**
+ * The committer of the commit for which the change is created.
+ *
+ * <p>Must be an existing user account.
+ *
+ * <p>Cannot be set together with {@link #committerIdent()} is set.
+ *
+ * <p>If neither {@link #committer()} nor {@link #committerIdent()} is set the {@link
+ * TestChangeCreation#owner()} is used as the committer.
+ */
+ public abstract Builder committer(Account.Id committer);
+
+ /**
+ * The committer ident of the commit for which the change is created.
+ *
+ * <p>Cannot be set together with {@link #committer()} is set.
+ *
+ * <p>If neither {@link #committer()} nor {@link #committerIdent()} is set the {@link
+ * TestChangeCreation#owner()} is used as the committer.
+ */
+ public abstract Builder committerIdent(PersonIdent committerIdent);
+
+ public abstract Optional<Account.Id> committer();
+
+ public abstract Optional<PersonIdent> committerIdent();
+
public abstract Builder commitMessage(String commitMessage);
/** Modified file of the patchset. The file content is specified via the returned builder. */
@@ -100,13 +166,23 @@
abstract TestPatchsetCreation autoBuild();
+ public TestPatchsetCreation build() {
+ checkState(
+ author().isEmpty() || authorIdent().isEmpty(),
+ "author and authorIdent cannot be set together");
+ checkState(
+ committer().isEmpty() || committerIdent().isEmpty(),
+ "committer and committerIdent cannot be set together");
+ return autoBuild();
+ }
+
/**
* Creates the patchset.
*
* @return the {@code PatchSet.Id} of the created patchset
*/
public PatchSet.Id create() {
- TestPatchsetCreation patchsetCreation = autoBuild();
+ TestPatchsetCreation patchsetCreation = build();
return patchsetCreation.patchsetCreator().applyAndThrowSilently(patchsetCreation);
}
}
diff --git a/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java b/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java
index 5d2c96c..3b97372 100644
--- a/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java
+++ b/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java
@@ -963,6 +963,156 @@
}
@Test
+ public void createdPatchsetPreviousAuthorAsAuthor() throws Exception {
+ String authorName = "Author";
+ String authorEmail = "author@example.com";
+ Account.Id author =
+ accountOperations.newAccount().fullname(authorName).preferredEmail(authorEmail).create();
+ Change.Id changeId = changeOperations.newChange().author(author).create();
+ ChangeInfo change = getChangeFromServer(changeId);
+ RevisionInfo revision = change.revisions.get(change.currentRevision);
+ assertThat(revision.commit.author.name).isEqualTo(authorName);
+ assertThat(revision.commit.author.email).isEqualTo(authorEmail);
+
+ changeOperations.change(changeId).newPatchset().create();
+ change = getChangeFromServer(changeId);
+ revision = change.revisions.get(change.currentRevision);
+ assertThat(revision.commit.author.name).isEqualTo(authorName);
+ assertThat(revision.commit.author.email).isEqualTo(authorEmail);
+ }
+
+ @Test
+ public void createdPatchsetHasSpecifiedAuthor() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ String authorName = "Author";
+ String authorEmail = "author@example.com";
+ Account.Id author =
+ accountOperations.newAccount().fullname(authorName).preferredEmail(authorEmail).create();
+ changeOperations.change(changeId).newPatchset().author(author).create();
+
+ ChangeInfo change = getChangeFromServer(changeId);
+ assertThat(change.owner._accountId).isNotEqualTo(author.get());
+ RevisionInfo revision = change.revisions.get(change.currentRevision);
+ assertThat(revision.commit.author.name).isEqualTo(authorName);
+ assertThat(revision.commit.author.email).isEqualTo(authorEmail);
+ }
+
+ @Test
+ public void createdPatchsetHasSpecifiedAuthorIdent() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ PersonIdent authorIdent = new PersonIdent("Author", "author@example.com");
+ changeOperations.change(changeId).newPatchset().authorIdent(authorIdent).create();
+
+ ChangeInfo change = getChangeFromServer(changeId);
+ RevisionInfo revision = change.revisions.get(change.currentRevision);
+ assertThat(revision.commit.author.name).isEqualTo(authorIdent.getName());
+ assertThat(revision.commit.author.email).isEqualTo(authorIdent.getEmailAddress());
+ }
+
+ @Test
+ public void patchsetCannotBeCreatedWithAuthorAndAuthorIdent() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ Account.Id author = accountOperations.newAccount().create();
+ PersonIdent authorIdent = new PersonIdent("Author", "author@example.com");
+
+ IllegalStateException exception =
+ assertThrows(
+ IllegalStateException.class,
+ () ->
+ changeOperations
+ .change(changeId)
+ .newPatchset()
+ .author(author)
+ .authorIdent(authorIdent)
+ .create());
+ assertThat(exception)
+ .hasMessageThat()
+ .isEqualTo("author and authorIdent cannot be set together");
+ }
+
+ @Test
+ public void createdPatchsetPreviousCommitterAsCommitter() throws Exception {
+ String committerName = "Committer";
+ String committerEmail = "committer@example.com";
+ Account.Id committer =
+ accountOperations
+ .newAccount()
+ .fullname(committerName)
+ .preferredEmail(committerEmail)
+ .create();
+ Change.Id changeId = changeOperations.newChange().committer(committer).create();
+ ChangeInfo change = getChangeFromServer(changeId);
+ RevisionInfo revision = change.revisions.get(change.currentRevision);
+ assertThat(revision.commit.committer.name).isEqualTo(committerName);
+ assertThat(revision.commit.committer.email).isEqualTo(committerEmail);
+
+ changeOperations.change(changeId).newPatchset().create();
+ change = getChangeFromServer(changeId);
+ revision = change.revisions.get(change.currentRevision);
+ assertThat(revision.commit.committer.name).isEqualTo(committerName);
+ assertThat(revision.commit.committer.email).isEqualTo(committerEmail);
+ }
+
+ @Test
+ public void createdPatchsetHasSpecifiedCommitter() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ String committerName = "Committer";
+ String committerEmail = "committer@example.com";
+ Account.Id committer =
+ accountOperations
+ .newAccount()
+ .fullname(committerName)
+ .preferredEmail(committerEmail)
+ .create();
+ changeOperations.change(changeId).newPatchset().committer(committer).create();
+
+ ChangeInfo change = getChangeFromServer(changeId);
+ assertThat(change.owner._accountId).isNotEqualTo(committer.get());
+ RevisionInfo revision = change.revisions.get(change.currentRevision);
+ assertThat(revision.commit.committer.name).isEqualTo(committerName);
+ assertThat(revision.commit.committer.email).isEqualTo(committerEmail);
+ }
+
+ @Test
+ public void createdPatchsetHasSpecifiedCommitterIdent() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ PersonIdent committerIdent = new PersonIdent("Committer", "committer@example.com");
+ changeOperations.change(changeId).newPatchset().committerIdent(committerIdent).create();
+
+ ChangeInfo change = getChangeFromServer(changeId);
+ RevisionInfo revision = change.revisions.get(change.currentRevision);
+ assertThat(revision.commit.committer.name).isEqualTo(committerIdent.getName());
+ assertThat(revision.commit.committer.email).isEqualTo(committerIdent.getEmailAddress());
+ }
+
+ @Test
+ public void patchsetCannotBeCreatedWithCommitterAndCommitterIdent() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ Account.Id committer = accountOperations.newAccount().create();
+ PersonIdent committerIdent = new PersonIdent("Committer", "committer@example.com");
+
+ IllegalStateException exception =
+ assertThrows(
+ IllegalStateException.class,
+ () ->
+ changeOperations
+ .change(changeId)
+ .newPatchset()
+ .committer(committer)
+ .committerIdent(committerIdent)
+ .create());
+ assertThat(exception)
+ .hasMessageThat()
+ .isEqualTo("committer and committerIdent cannot be set together");
+ }
+
+ @Test
public void newPatchsetCanHaveUpdatedCommitMessage() throws Exception {
Change.Id changeId = changeOperations.newChange().commitMessage("Old message").create();
