Change test API: Allow to specify author/committer for new changes
Extend the test API for changes so that the author/committer can be
specified when creating new changes.
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 change owner is
used as the author/committer.
Release-Notes: skip
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: If95226293a8092e3b0d2faad4d03f97a3c0e52b5
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImpl.java b/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImpl.java
index 0966bbe..a45dd8a 100644
--- a/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImpl.java
+++ b/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImpl.java
@@ -139,9 +139,10 @@
RevWalk revWalk = new RevWalk(objectInserter.newReader())) {
Instant now = TimeUtil.now();
IdentifiedUser changeOwner = getChangeOwner(changeCreation);
- PersonIdent authorAndCommitter = changeOwner.newCommitterIdent(now, serverIdent.getZoneId());
+ PersonIdent author = getAuthorIdent(now, changeCreation);
+ PersonIdent committer = getCommitterIdent(now, changeCreation);
ObjectId commitId =
- createCommit(repository, revWalk, objectInserter, changeCreation, authorAndCommitter);
+ createCommit(repository, revWalk, objectInserter, changeCreation, author, committer);
String refName = RefNames.fullName(changeCreation.branch());
ChangeInserter inserter = getChangeInserter(changeId, refName, commitId);
@@ -189,6 +190,30 @@
return getArbitraryUser();
}
+ private PersonIdent getAuthorIdent(Instant when, TestChangeCreation changeCreation)
+ throws IOException, ConfigInvalidException {
+ if (changeCreation.authorIdent().isPresent()) {
+ return new PersonIdent(changeCreation.authorIdent().get(), when);
+ }
+
+ return (changeCreation.author().isPresent()
+ ? userFactory.create(changeCreation.author().get())
+ : getChangeOwner(changeCreation))
+ .newCommitterIdent(when, serverIdent.getZoneId());
+ }
+
+ private PersonIdent getCommitterIdent(Instant when, TestChangeCreation changeCreation)
+ throws IOException, ConfigInvalidException {
+ if (changeCreation.committerIdent().isPresent()) {
+ return new PersonIdent(changeCreation.committerIdent().get(), when);
+ }
+
+ return (changeCreation.committer().isPresent()
+ ? userFactory.create(changeCreation.committer().get())
+ : getChangeOwner(changeCreation))
+ .newCommitterIdent(when, serverIdent.getZoneId());
+ }
+
private IdentifiedUser getArbitraryUser() throws ConfigInvalidException, IOException {
ImmutableSet<Account.Id> foundAccounts = resolver.resolveIgnoreVisibility("").asIdSet();
checkState(
@@ -202,15 +227,15 @@
RevWalk revWalk,
ObjectInserter objectInserter,
TestChangeCreation changeCreation,
- PersonIdent authorAndCommitter)
+ PersonIdent author,
+ PersonIdent committer)
throws IOException, BadRequestException {
ImmutableList<ObjectId> parentCommits = getParentCommits(repository, revWalk, changeCreation);
TreeCreator treeCreator =
getTreeCreator(objectInserter, parentCommits, changeCreation.mergeStrategy());
ObjectId tree = createNewTree(repository, treeCreator, changeCreation.treeModifications());
String commitMessage = correctCommitMessage(changeCreation.commitMessage());
- return createCommit(
- objectInserter, tree, parentCommits, authorAndCommitter, authorAndCommitter, commitMessage);
+ return createCommit(objectInserter, tree, parentCommits, author, committer, commitMessage);
}
private ImmutableList<ObjectId> getParentCommits(
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/TestChangeCreation.java b/java/com/google/gerrit/acceptance/testsuite/change/TestChangeCreation.java
index f01a138..a0746e2 100644
--- a/java/com/google/gerrit/acceptance/testsuite/change/TestChangeCreation.java
+++ b/java/com/google/gerrit/acceptance/testsuite/change/TestChangeCreation.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.common.collect.ImmutableMap;
@@ -24,6 +26,7 @@
import com.google.gerrit.server.edit.tree.TreeModification;
import java.util.Optional;
import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.merge.MergeStrategy;
/** Initial attributes of the change. If not provided, arbitrary values will be used. */
@@ -35,6 +38,14 @@
public abstract Optional<Account.Id> owner();
+ 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> topic();
public abstract ImmutableMap<String, Short> approvals();
@@ -69,9 +80,65 @@
*/
public abstract Builder branch(String branch);
- /** The change owner. Must be an existing user account. */
+ /**
+ * The change owner.
+ *
+ * <p>Must be an existing user account.
+ */
public abstract Builder owner(Account.Id owner);
+ /**
+ * 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();
+
/** The topic to add this change to. */
public abstract Builder topic(String topic);
@@ -156,13 +223,23 @@
abstract TestChangeCreation autoBuild();
+ public TestChangeCreation 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 change.
*
* @return the {@code Change.Id} of the created change
*/
public Change.Id create() {
- TestChangeCreation changeUpdate = autoBuild();
+ TestChangeCreation changeUpdate = build();
return changeUpdate.changeCreator().applyAndThrowSilently(changeUpdate);
}
}
diff --git a/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java b/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java
index fd5f6fc..5d2c96c 100644
--- a/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java
+++ b/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java
@@ -29,6 +29,7 @@
import com.google.common.truth.Correspondence;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.testsuite.account.AccountOperations;
+import com.google.gerrit.acceptance.testsuite.account.TestAccount;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.entities.Account;
@@ -49,6 +50,7 @@
import com.google.inject.Inject;
import java.util.Map;
import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.PersonIdent;
import org.junit.Test;
public class ChangeOperationsImplTest extends AbstractDaemonTest {
@@ -611,6 +613,155 @@
}
@Test
+ public void createdChangeHasOwnerAsAuthor() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ ChangeInfo change = getChangeFromServer(changeId);
+ TestAccount changeOwner = accountOperations.account(Account.id(change.owner._accountId)).get();
+ RevisionInfo revision = change.revisions.get(change.currentRevision);
+ assertThat(revision.commit.author.name).isEqualTo(changeOwner.fullname().get());
+ assertThat(revision.commit.author.email).isEqualTo(changeOwner.preferredEmail().get());
+ }
+
+ @Test
+ public void createdChangeHasSpecifiedOwnerAsAuthor() throws Exception {
+ String changeOwnerName = "Change Owner";
+ String changeOwnerEmail = "change-owner@example.com";
+ Account.Id changeOwner =
+ accountOperations
+ .newAccount()
+ .fullname(changeOwnerName)
+ .preferredEmail(changeOwnerEmail)
+ .create();
+ Change.Id changeId = changeOperations.newChange().owner(changeOwner).create();
+
+ ChangeInfo change = getChangeFromServer(changeId);
+ assertThat(change.owner._accountId).isEqualTo(changeOwner.get());
+ RevisionInfo revision = change.revisions.get(change.currentRevision);
+ assertThat(revision.commit.author.name).isEqualTo(changeOwnerName);
+ assertThat(revision.commit.author.email).isEqualTo(changeOwnerEmail);
+ }
+
+ @Test
+ public void createdChangeHasSpecifiedAuthor() 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);
+ 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 createdChangeHasSpecifiedAuthorIdent() throws Exception {
+ PersonIdent authorIdent = new PersonIdent("Author", "author@example.com");
+ Change.Id changeId = changeOperations.newChange().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 changeCannotBeCreatedWithAuthorAndAuthorIdent() throws Exception {
+ Account.Id author = accountOperations.newAccount().create();
+ PersonIdent authorIdent = new PersonIdent("Author", "author@example.com");
+
+ IllegalStateException exception =
+ assertThrows(
+ IllegalStateException.class,
+ () -> changeOperations.newChange().author(author).authorIdent(authorIdent).create());
+ assertThat(exception)
+ .hasMessageThat()
+ .isEqualTo("author and authorIdent cannot be set together");
+ }
+
+ @Test
+ public void createdChangeHasOwnerAsCommitter() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ ChangeInfo change = getChangeFromServer(changeId);
+ TestAccount changeOwner = accountOperations.account(Account.id(change.owner._accountId)).get();
+ RevisionInfo revision = change.revisions.get(change.currentRevision);
+ assertThat(revision.commit.committer.name).isEqualTo(changeOwner.fullname().get());
+ assertThat(revision.commit.committer.email).isEqualTo(changeOwner.preferredEmail().get());
+ }
+
+ @Test
+ public void createdChangeHasSpecifiedOwnerAsCommitter() throws Exception {
+ String changeOwnerName = "Change Owner";
+ String changeOwnerEmail = "change-owner@example.com";
+ Account.Id changeOwner =
+ accountOperations
+ .newAccount()
+ .fullname(changeOwnerName)
+ .preferredEmail(changeOwnerEmail)
+ .create();
+ Change.Id changeId = changeOperations.newChange().owner(changeOwner).create();
+
+ ChangeInfo change = getChangeFromServer(changeId);
+ assertThat(change.owner._accountId).isEqualTo(changeOwner.get());
+ RevisionInfo revision = change.revisions.get(change.currentRevision);
+ assertThat(revision.commit.committer.name).isEqualTo(changeOwnerName);
+ assertThat(revision.commit.committer.email).isEqualTo(changeOwnerEmail);
+ }
+
+ @Test
+ public void createdChangeHasSpecifiedCommitter() 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);
+ 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 createdChangeHasSpecifiedCommitterIdent() throws Exception {
+ PersonIdent committerIdent = new PersonIdent("Committer", "committer@example.com");
+ Change.Id changeId = changeOperations.newChange().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 changeCannotBeCreatedWithCommitterAndCommitterIdent() throws Exception {
+ Account.Id committer = accountOperations.newAccount().create();
+ PersonIdent committerIdent = new PersonIdent("Committer", "committer@example.com");
+
+ IllegalStateException exception =
+ assertThrows(
+ IllegalStateException.class,
+ () ->
+ changeOperations
+ .newChange()
+ .committer(committer)
+ .committerIdent(committerIdent)
+ .create());
+ assertThat(exception)
+ .hasMessageThat()
+ .isEqualTo("committer and committerIdent cannot be set together");
+ }
+
+ @Test
public void createdChangeHasSpecifiedTopic() throws Exception {
Change.Id changeId = changeOperations.newChange().topic("test-topic").create();
