Get rid of AbstractDaemonTest#get

This variant includes ListChangesOptions, most of which are not needed
at any given point. With static imports, it's not so bad to have callers
request just the options they want, and if they really want the full
ChangeInfo, the underlying extension API is still available to them.

This probably saves a few ms per invocation.

Change-Id: Iaab7789312362be804fa978c6d8fdf5c3e583b58
This commit is contained in:
Dave Borowitz
2017-10-20 15:41:33 -04:00
committed by Changcheng Xiao
parent 0f421c3a7f
commit 15c7fda556
14 changed files with 54 additions and 41 deletions

View File

@@ -722,10 +722,6 @@ public abstract class AbstractDaemonTest {
return gApi.changes().id(id).info();
}
protected ChangeInfo get(String id) throws RestApiException {
return gApi.changes().id(id).get();
}
protected Optional<EditInfo> getEdit(String id) throws RestApiException {
return gApi.changes().id(id).edit().get();
}

View File

@@ -15,6 +15,7 @@
package com.google.gerrit.acceptance.api.change;
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.extensions.client.ListChangesOption.MESSAGES;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static java.util.concurrent.TimeUnit.HOURS;
import static java.util.concurrent.TimeUnit.SECONDS;
@@ -50,7 +51,7 @@ public class AbandonIT extends AbstractDaemonTest {
String changeId = r.getChangeId();
assertThat(info(changeId).status).isEqualTo(ChangeStatus.NEW);
gApi.changes().id(changeId).abandon();
ChangeInfo info = get(changeId);
ChangeInfo info = get(changeId, MESSAGES);
assertThat(info.status).isEqualTo(ChangeStatus.ABANDONED);
assertThat(Iterables.getLast(info.messages).message.toLowerCase()).contains("abandoned");
@@ -68,12 +69,12 @@ public class AbandonIT extends AbstractDaemonTest {
changeAbandoner.batchAbandon(
batchUpdateFactory, a.getChange().project(), user, list, "deadbeef");
ChangeInfo info = get(a.getChangeId());
ChangeInfo info = get(a.getChangeId(), MESSAGES);
assertThat(info.status).isEqualTo(ChangeStatus.ABANDONED);
assertThat(Iterables.getLast(info.messages).message.toLowerCase()).contains("abandoned");
assertThat(Iterables.getLast(info.messages).message.toLowerCase()).contains("deadbeef");
info = get(b.getChangeId());
info = get(b.getChangeId(), MESSAGES);
assertThat(info.status).isEqualTo(ChangeStatus.ABANDONED);
assertThat(Iterables.getLast(info.messages).message.toLowerCase()).contains("abandoned");
assertThat(Iterables.getLast(info.messages).message.toLowerCase()).contains("deadbeef");
@@ -155,7 +156,7 @@ public class AbandonIT extends AbstractDaemonTest {
assertThat(info(changeId).status).isEqualTo(ChangeStatus.ABANDONED);
gApi.changes().id(changeId).restore();
ChangeInfo info = get(changeId);
ChangeInfo info = get(changeId, MESSAGES);
assertThat(info.status).isEqualTo(ChangeStatus.NEW);
assertThat(Iterables.getLast(info.messages).message.toLowerCase()).contains("restored");

View File

@@ -1107,7 +1107,7 @@ public class ChangeIT extends AbstractDaemonTest {
String changeId = r.getChangeId();
assertThat(info(changeId).status).isEqualTo(ChangeStatus.NEW);
gApi.changes().id(changeId).abandon();
ChangeInfo info = get(changeId);
ChangeInfo info = info(changeId);
assertThat(info.status).isEqualTo(ChangeStatus.ABANDONED);
exception.expect(ResourceConflictException.class);
@@ -1126,7 +1126,7 @@ public class ChangeIT extends AbstractDaemonTest {
String changeId = r.getChangeId();
assertThat(info(changeId).status).isEqualTo(ChangeStatus.NEW);
gApi.changes().id(changeId).abandon();
ChangeInfo info = get(changeId);
ChangeInfo info = info(changeId);
assertThat(info.status).isEqualTo(ChangeStatus.ABANDONED);
RebaseInput ri = new RebaseInput();
@@ -2109,10 +2109,10 @@ public class ChangeIT extends AbstractDaemonTest {
gApi.changes().id(r.getChangeId()).addReviewer(in);
setApiUser(user);
assertThat(get(r.getChangeId()).reviewed).isNull();
assertThat(get(r.getChangeId(), REVIEWED).reviewed).isNull();
revision(r).review(ReviewInput.recommend());
assertThat(get(r.getChangeId()).reviewed).isTrue();
assertThat(get(r.getChangeId(), REVIEWED).reviewed).isTrue();
}
@Test

View File

@@ -21,6 +21,7 @@ import static com.google.gerrit.acceptance.PushOneCommit.FILE_NAME;
import static com.google.gerrit.acceptance.PushOneCommit.PATCH;
import static com.google.gerrit.acceptance.PushOneCommit.PATCH_FILE_ONLY;
import static com.google.gerrit.acceptance.PushOneCommit.SUBJECT;
import static com.google.gerrit.extensions.client.ListChangesOption.ALL_REVISIONS;
import static com.google.gerrit.extensions.client.ListChangesOption.DETAILED_LABELS;
import static com.google.gerrit.reviewdb.client.Patch.COMMIT_MSG;
import static com.google.gerrit.reviewdb.client.Patch.MERGE_LIST;
@@ -523,7 +524,7 @@ public class RevisionIT extends AbstractDaemonTest {
gApi.changes().id(t2).restore();
gApi.changes().id(t1).current().cherryPick(in);
assertThat(get(t2).revisions).hasSize(2);
assertThat(get(t2, ALL_REVISIONS).revisions).hasSize(2);
assertThat(gApi.changes().id(t2).current().file(FILE_NAME).content().asString()).isEqualTo("a");
}

View File

@@ -16,6 +16,10 @@ package com.google.gerrit.acceptance.edit;
import static com.google.common.collect.Iterables.getOnlyElement;
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_COMMIT;
import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_REVISION;
import static com.google.gerrit.extensions.client.ListChangesOption.DETAILED_LABELS;
import static com.google.gerrit.extensions.client.ListChangesOption.MESSAGES;
import static com.google.gerrit.extensions.common.EditInfoSubject.assertThat;
import static com.google.gerrit.extensions.restapi.BinaryResultSubject.assertThat;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
@@ -166,7 +170,7 @@ public class ChangeEditIT extends AbstractDaemonTest {
// The tag for the publish edit change message should vary according
// to whether the change was WIP at the time of publishing.
ChangeInfo info = get(changeId);
ChangeInfo info = get(changeId, MESSAGES);
assertThat(info.messages).isNotEmpty();
assertThat(Iterables.getLast(info.messages).tag)
.isEqualTo(ChangeMessagesUtil.TAG_UPLOADED_PATCH_SET);
@@ -176,7 +180,7 @@ public class ChangeEditIT extends AbstractDaemonTest {
createEmptyEditFor(changeId);
gApi.changes().id(changeId).edit().modifyFile(FILE_NAME, RawInputUtil.create(CONTENT_NEW2));
gApi.changes().id(changeId).edit().publish();
info = get(changeId);
info = get(changeId, MESSAGES);
assertThat(info.messages).isNotEmpty();
assertThat(Iterables.getLast(info.messages).tag)
.isEqualTo(ChangeMessagesUtil.TAG_UPLOADED_WIP_PATCH_SET);
@@ -415,7 +419,7 @@ public class ChangeEditIT extends AbstractDaemonTest {
adminRestSession.get(urlEdit(changeId)).assertNoContent();
createArbitraryEditFor(changeId);
EditInfo editInfo = getEditInfo(changeId, false);
ChangeInfo changeInfo = get(changeId);
ChangeInfo changeInfo = get(changeId, CURRENT_REVISION, CURRENT_COMMIT);
assertThat(editInfo.commit.commit).isNotEqualTo(changeInfo.currentRevision);
assertThat(editInfo).commit().parents().hasSize(1);
assertThat(editInfo).baseRevision().isEqualTo(changeInfo.currentRevision);
@@ -615,7 +619,7 @@ public class ChangeEditIT extends AbstractDaemonTest {
publishInput.notify = NotifyHandling.NONE;
gApi.changes().id(changeId).edit().publish(publishInput);
ChangeInfo info = get(changeId);
ChangeInfo info = get(changeId, DETAILED_LABELS);
assertThat(info.subject).isEqualTo(newSubj);
List<ApprovalInfo> approvals = info.labels.get(cr).all;
assertThat(approvals).hasSize(1);
@@ -839,7 +843,7 @@ public class ChangeEditIT extends AbstractDaemonTest {
private void assertChangeMessages(String changeId, List<String> expectedMessages)
throws Exception {
ChangeInfo ci = get(changeId);
ChangeInfo ci = get(changeId, MESSAGES);
assertThat(ci.messages).isNotNull();
assertThat(ci.messages).hasSize(expectedMessages.size());
List<String> actualMessages =

View File

@@ -23,6 +23,9 @@ import static com.google.gerrit.acceptance.GitUtil.pushHead;
import static com.google.gerrit.acceptance.PushOneCommit.FILE_NAME;
import static com.google.gerrit.common.FooterConstants.CHANGE_ID;
import static com.google.gerrit.extensions.client.ListChangesOption.ALL_REVISIONS;
import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_REVISION;
import static com.google.gerrit.extensions.client.ListChangesOption.DETAILED_ACCOUNTS;
import static com.google.gerrit.extensions.client.ListChangesOption.DETAILED_LABELS;
import static com.google.gerrit.extensions.client.ListChangesOption.MESSAGES;
import static com.google.gerrit.extensions.common.EditInfoSubject.assertThat;
import static com.google.gerrit.server.git.receive.ReceiveConstants.PUSH_OPTION_SKIP_VALIDATION;
@@ -672,7 +675,7 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
PushOneCommit.Result r = pushTo("refs/for/master/%m=my_test_message");
r.assertOkStatus();
r.assertChange(Change.Status.NEW, null);
ChangeInfo ci = get(r.getChangeId());
ChangeInfo ci = get(r.getChangeId(), MESSAGES, ALL_REVISIONS);
Collection<ChangeMessageInfo> changeMessages = ci.messages;
assertThat(changeMessages).hasSize(1);
for (ChangeMessageInfo cm : changeMessages) {
@@ -707,7 +710,7 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
r = push.to("refs/for/master/%m=new_test_message");
r.assertOkStatus();
ChangeInfo ci = get(r.getChangeId());
ChangeInfo ci = get(r.getChangeId(), ALL_REVISIONS);
Collection<RevisionInfo> revisions = ci.revisions.values();
assertThat(revisions).hasSize(2);
for (RevisionInfo ri : revisions) {
@@ -723,7 +726,7 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
public void pushForMasterWithApprovals() throws Exception {
PushOneCommit.Result r = pushTo("refs/for/master/%l=Code-Review");
r.assertOkStatus();
ChangeInfo ci = get(r.getChangeId());
ChangeInfo ci = get(r.getChangeId(), DETAILED_LABELS, MESSAGES, DETAILED_ACCOUNTS);
LabelInfo cr = ci.labels.get("Code-Review");
assertThat(cr.all).hasSize(1);
assertThat(cr.all.get(0).name).isEqualTo("Administrator");
@@ -742,7 +745,7 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
r.getChangeId());
r = push.to("refs/for/master/%l=Code-Review+2");
ci = get(r.getChangeId());
ci = get(r.getChangeId(), DETAILED_LABELS, MESSAGES, DETAILED_ACCOUNTS);
cr = ci.labels.get("Code-Review");
assertThat(Iterables.getLast(ci.messages).message)
.isEqualTo("Uploaded patch set 2: Code-Review+2.");
@@ -763,7 +766,7 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
"moreContent",
r.getChangeId());
r = push.to("refs/for/master/%l=Code-Review+2");
ci = get(r.getChangeId());
ci = get(r.getChangeId(), MESSAGES);
assertThat(Iterables.getLast(ci.messages).message).isEqualTo("Uploaded patch set 3.");
}
@@ -783,7 +786,7 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
r.getChangeId());
r = push.to("refs/for/master/%l=Code-Review+2");
ChangeInfo ci = get(r.getChangeId());
ChangeInfo ci = get(r.getChangeId(), DETAILED_LABELS, MESSAGES, DETAILED_ACCOUNTS);
LabelInfo cr = ci.labels.get("Code-Review");
assertThat(Iterables.getLast(ci.messages).message)
.isEqualTo("Uploaded patch set 2: Code-Review+2.");
@@ -828,7 +831,8 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
// 2. +1 from Administrator (uploader):
// On push Code-Review+1 was specified, hence we expect a +1 vote from
// the uploader.
ChangeInfo ci = get(GitUtil.getChangeId(testRepo, c).get());
ChangeInfo ci =
get(GitUtil.getChangeId(testRepo, c).get(), DETAILED_LABELS, MESSAGES, DETAILED_ACCOUNTS);
LabelInfo cr = ci.labels.get("Code-Review");
assertThat(cr.all).hasSize(2);
int indexAdmin = admin.fullName.equals(cr.all.get(0).name) ? 0 : 1;
@@ -864,7 +868,7 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
pushHead(testRepo, "refs/for/master/%l=Code-Review+1,l=Custom-Label-1", false);
ChangeInfo ci = get(GitUtil.getChangeId(testRepo, c).get());
ChangeInfo ci = get(GitUtil.getChangeId(testRepo, c).get(), DETAILED_LABELS, DETAILED_ACCOUNTS);
LabelInfo cr = ci.labels.get("Code-Review");
assertThat(cr.all).hasSize(1);
cr = ci.labels.get("Custom-Label");
@@ -1161,8 +1165,8 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
private void assertTwoChangesWithSameRevision(PushOneCommit.Result result) throws Exception {
List<ChangeInfo> changes = query(result.getCommit().name());
assertThat(changes).hasSize(2);
ChangeInfo c1 = get(changes.get(0).id);
ChangeInfo c2 = get(changes.get(1).id);
ChangeInfo c1 = get(changes.get(0).id, CURRENT_REVISION);
ChangeInfo c2 = get(changes.get(1).id, CURRENT_REVISION);
assertThat(c1.project).isEqualTo(c2.project);
assertThat(c1.branch).isNotEqualTo(c2.branch);
assertThat(c1.changeId).isEqualTo(c2.changeId);

View File

@@ -1112,7 +1112,7 @@ public abstract class AbstractSubmit extends AbstractDaemonTest {
}
protected void assertNew(String changeId) throws Exception {
assertThat(get(changeId).status).isEqualTo(ChangeStatus.NEW);
assertThat(info(changeId).status).isEqualTo(ChangeStatus.NEW);
}
protected void assertApproved(String changeId) throws Exception {

View File

@@ -403,7 +403,7 @@ public abstract class AbstractSubmitByRebase extends AbstractSubmit {
assertThat(headAfterChange2.getShortMessage()).isEqualTo("Change 2");
assertThat(headAfterChange1).isEqualTo(headAfterChange2.getParent(0));
ChangeInfo info2 = get(change2.getChangeId());
ChangeInfo info2 = info(change2.getChangeId());
assertThat(info2.status).isEqualTo(ChangeStatus.MERGED);
}

View File

@@ -16,6 +16,7 @@ package com.google.gerrit.acceptance.rest.change;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.TruthJUnit.assume;
import static com.google.gerrit.extensions.client.ListChangesOption.DETAILED_LABELS;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static java.util.concurrent.TimeUnit.SECONDS;
@@ -178,7 +179,7 @@ public class AssigneeIT extends AbstractDaemonTest {
private Iterable<AccountInfo> getReviewers(PushOneCommit.Result r, ReviewerState state)
throws Exception {
return get(r.getChangeId()).reviewers.get(state);
return get(r.getChangeId(), DETAILED_LABELS).reviewers.get(state);
}
private AccountInfo setAssignee(PushOneCommit.Result r, String identifieer) throws Exception {

View File

@@ -15,6 +15,7 @@
package com.google.gerrit.acceptance.rest.change;
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.extensions.client.ListChangesOption.MESSAGES;
import static java.util.concurrent.TimeUnit.SECONDS;
import com.google.gerrit.acceptance.AbstractDaemonTest;
@@ -56,7 +57,7 @@ public class ChangeMessagesIT extends AbstractDaemonTest {
@Test
public void defaultMessage() throws Exception {
String changeId = createChange().getChangeId();
ChangeInfo c = get(changeId);
ChangeInfo c = get(changeId, MESSAGES);
assertThat(c.messages).isNotNull();
assertThat(c.messages).hasSize(1);
assertThat(c.messages.iterator().next().message).isEqualTo("Uploaded patch set 1.");
@@ -69,7 +70,7 @@ public class ChangeMessagesIT extends AbstractDaemonTest {
postMessage(changeId, firstMessage);
String secondMessage = "I like this feature.";
postMessage(changeId, secondMessage);
ChangeInfo c = get(changeId);
ChangeInfo c = get(changeId, MESSAGES);
assertThat(c.messages).isNotNull();
assertThat(c.messages).hasSize(3);
Iterator<ChangeMessageInfo> it = c.messages.iterator();
@@ -84,7 +85,7 @@ public class ChangeMessagesIT extends AbstractDaemonTest {
String tag = "jenkins";
String msg = "Message with tag.";
postMessage(changeId, msg, tag);
ChangeInfo c = get(changeId);
ChangeInfo c = get(changeId, MESSAGES);
assertThat(c.messages).isNotNull();
assertThat(c.messages).hasSize(2);
Iterator<ChangeMessageInfo> it = c.messages.iterator();

View File

@@ -16,6 +16,8 @@ package com.google.gerrit.acceptance.rest.change;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.TruthJUnit.assume;
import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_REVISION;
import static com.google.gerrit.extensions.client.ListChangesOption.MESSAGES;
import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.PushOneCommit;
@@ -111,7 +113,7 @@ public class SubmitByCherryPickIT extends AbstractSubmit {
handle.remove();
}
testRepo.git().fetch().setRemote("origin").call();
ChangeInfo info = get(change.getChangeId());
ChangeInfo info = get(change.getChangeId(), CURRENT_REVISION);
RevCommit c = testRepo.getRevWalk().parseCommit(ObjectId.fromString(info.currentRevision));
testRepo.getRevWalk().parseBody(c);
assertThat(c.getFooterLines("Custom")).containsExactly("refs/heads/master");
@@ -374,7 +376,7 @@ public class SubmitByCherryPickIT extends AbstractSubmit {
assertThat(getRemoteHead()).isEqualTo(headAfterFirstSubmit);
ChangeInfo info2 = get(change2.getChangeId());
ChangeInfo info2 = get(change2.getChangeId(), MESSAGES);
assertThat(info2.status).isEqualTo(ChangeStatus.MERGED);
assertThat(Iterables.getLast(info2.messages).message)
.isEqualTo(CommitMergeStatus.SKIPPED_IDENTICAL_TREE.getMessage());

View File

@@ -15,6 +15,7 @@
package com.google.gerrit.acceptance.rest.change;
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_REVISION;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.TestProjectInput;
@@ -126,7 +127,7 @@ public class SubmitByRebaseAlwaysIT extends AbstractSubmitByRebase {
private RevCommit getCurrentCommit(PushOneCommit.Result change) throws Exception {
testRepo.git().fetch().setRemote("origin").call();
ChangeInfo info = get(change.getChangeId());
ChangeInfo info = get(change.getChangeId(), CURRENT_REVISION);
RevCommit c = testRepo.getRevWalk().parseCommit(ObjectId.fromString(info.currentRevision));
testRepo.getRevWalk().parseBody(c);
return c;

View File

@@ -15,6 +15,7 @@
package com.google.gerrit.acceptance.server.event;
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.extensions.client.ListChangesOption.DETAILED_LABELS;
import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
import static com.google.gerrit.server.project.Util.category;
import static com.google.gerrit.server.project.Util.value;
@@ -124,7 +125,7 @@ public class CommentAddedEventIT extends AbstractDaemonTest {
revision(r).review(reviewInput);
// push a new revision with +1 vote
ChangeInfo c = get(r.getChangeId());
ChangeInfo c = info(r.getChangeId());
r = amendChange(c.changeId);
reviewInput = new ReviewInput().label(label.getName(), (short) 1);
revision(r).review(reviewInput);
@@ -206,7 +207,7 @@ public class CommentAddedEventIT extends AbstractDaemonTest {
reviewInput.message = label.getName();
revision(r).review(reviewInput);
ChangeInfo c = get(r.getChangeId());
ChangeInfo c = get(r.getChangeId(), DETAILED_LABELS);
LabelInfo q = c.labels.get(label.getName());
assertThat(q.all).hasSize(1);
ApprovalValues labelAttr = getApprovalValues(label);
@@ -233,7 +234,7 @@ public class CommentAddedEventIT extends AbstractDaemonTest {
reviewInput.message = pLabel.getName();
revision(r).review(reviewInput);
c = get(r.getChangeId());
c = get(r.getChangeId(), DETAILED_LABELS);
q = c.labels.get(label.getName());
assertThat(q.all).hasSize(1);
pLabelAttr = getApprovalValues(pLabel);

View File

@@ -16,6 +16,7 @@ package com.google.gerrit.acceptance.ssh;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
import static com.google.gerrit.extensions.client.ListChangesOption.MESSAGES;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.acceptance.AbstractDaemonTest;
@@ -67,7 +68,7 @@ public class AbandonRestoreIT extends AbstractDaemonTest {
}
private void assertChangeMessages(String changeId, List<String> expected) throws Exception {
ChangeInfo c = get(changeId);
ChangeInfo c = get(changeId, MESSAGES);
Iterable<ChangeMessageInfo> messages = c.messages;
assertThat(messages).isNotNull();
assertThat(messages).hasSize(expected.size());