opendev/gerrit
Code Issues Proposed changes

Merge "RemoveDrafts: disallow creation of new drafts"

Browse Source
This commit is contained in:
Patrick Hiesel
2017-09-15 08:39:57 +00:00
committed by Gerrit Code Review
parent 20f4c40e4a 4b44bdc702
commit 960ab1a3f9
23 changed files with 268 additions and 401 deletions
Download Patch File Download Diff File Expand all files Collapse all files

23
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
View File

@@ -834,7 +834,7 @@ public class ChangeIT extends AbstractDaemonTest {
@Test
public void publish() throws Exception {
PushOneCommit.Result r = createChange("refs/drafts/master");
PushOneCommit.Result r = createDraftChange();
assertThat(info(r.getChangeId()).status).isEqualTo(ChangeStatus.DRAFT);
gApi.changes().id(r.getChangeId()).publish();
assertThat(info(r.getChangeId()).status).isEqualTo(ChangeStatus.NEW);
@@ -842,7 +842,7 @@ public class ChangeIT extends AbstractDaemonTest {
@Test
public void deleteDraftChange() throws Exception {
PushOneCommit.Result r = createChange("refs/drafts/master");
PushOneCommit.Result r = createDraftChange();
assertThat(query(r.getChangeId())).hasSize(1);
assertThat(info(r.getChangeId()).status).isEqualTo(ChangeStatus.DRAFT);
gApi.changes().id(r.getChangeId()).delete();
@@ -2472,8 +2472,9 @@ public class ChangeIT extends AbstractDaemonTest {
// Amend draft as admin
PushOneCommit.Result r2 =
amendChange(r1.getChangeId(), "refs/drafts/master", admin, adminTestRepo);
amendChange(r1.getChangeId(), "refs/for/master", admin, adminTestRepo);
r2.assertOkStatus();
setCurrentPatchSetAsDraft(r2.getChange().getId());
// Add user as reviewer to make this patch set visible
AddReviewerInput in = new AddReviewerInput();
@@ -2485,9 +2486,9 @@ public class ChangeIT extends AbstractDaemonTest {
userTestRepo.reset("ps");
// Amend change as user
PushOneCommit.Result r3 =
amendChange(r2.getChangeId(), "refs/drafts/master", user, userTestRepo);
PushOneCommit.Result r3 = amendChange(r2.getChangeId(), "refs/for/master", user, userTestRepo);
r3.assertOkStatus();
setCurrentPatchSetAsDraft(r3.getChange().getId());
}
@Test
@@ -2503,8 +2504,9 @@ public class ChangeIT extends AbstractDaemonTest {
// Amend draft as admin
PushOneCommit.Result r2 =
amendChange(r1.getChangeId(), "refs/drafts/master", admin, adminTestRepo);
amendChange(r1.getChangeId(), "refs/for/master", admin, adminTestRepo);
r2.assertOkStatus();
setCurrentPatchSetAsDraft(r2.getChange().getId());
// Fetch change
GitUtil.fetch(userTestRepo, r1.getPatchSet().getRefName() + ":ps");
@@ -2594,8 +2596,9 @@ public class ChangeIT extends AbstractDaemonTest {
// Create change as admin
PushOneCommit push = pushFactory.create(db, admin.getIdent(), adminTestRepo);
PushOneCommit.Result r1 = push.to("refs/drafts/master");
PushOneCommit.Result r1 = push.to("refs/for/master");
r1.assertOkStatus();
markChangeAsDraft(r1.getChange().getId());
// Add user as reviewer
AddReviewerInput in = new AddReviewerInput();
@@ -2624,8 +2627,9 @@ public class ChangeIT extends AbstractDaemonTest {
// Create change as admin
PushOneCommit push = pushFactory.create(db, admin.getIdent(), adminTestRepo);
PushOneCommit.Result r1 = push.to("refs/drafts/master");
PushOneCommit.Result r1 = push.to("refs/for/master");
r1.assertOkStatus();
markChangeAsDraft(p, r1.getChange().getId());
// Add user as reviewer
AddReviewerInput in = new AddReviewerInput();
@@ -2637,8 +2641,7 @@ public class ChangeIT extends AbstractDaemonTest {
userTestRepo.reset("ps");
// Amend change as user
PushOneCommit.Result r2 =
amendChange(r1.getChangeId(), "refs/drafts/master", user, userTestRepo);
PushOneCommit.Result r2 = amendChange(r1.getChangeId(), "refs/for/master", user, userTestRepo);
r2.assertErrorStatus("cannot add patch set to " + r1.getChange().getId().id + ".");
}

7
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java
View File

@@ -303,7 +303,7 @@ public class RevisionIT extends AbstractDaemonTest {
@Test
public void deleteDraft() throws Exception {
PushOneCommit.Result r = createDraft();
PushOneCommit.Result r = createDraftChange();
gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).delete();
}
@@ -1305,11 +1305,6 @@ public class RevisionIT extends AbstractDaemonTest {
return push.to("refs/for/master");
}
private PushOneCommit.Result createDraft() throws Exception {
PushOneCommit push = pushFactory.create(db, admin.getIdent(), testRepo);
return push.to("refs/drafts/master");
}
private RevisionApi current(PushOneCommit.Result r) throws Exception {
return gApi.changes().id(r.getChangeId()).current();
}

14
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java
View File

@@ -622,21 +622,23 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
@Test
public void pushForMasterAsDraft() throws Exception {
// create draft by pushing to 'refs/drafts/'
// create draft by pushing to 'refs/drafts/' will get a private change.
PushOneCommit.Result r = pushTo("refs/drafts/master");
r.assertOkStatus();
r.assertChange(Change.Status.DRAFT, null);
r.assertChange(Change.Status.NEW, null);
// create draft by using 'draft' option
assertThat(gApi.changes().id(r.getChangeId()).get().isPrivate).isTrue();
// create draft by using 'draft' option will get a private change, too.
r = pushTo("refs/for/master%draft");
r.assertOkStatus();
r.assertChange(Change.Status.DRAFT, null);
r.assertChange(Change.Status.NEW, null);
assertThat(gApi.changes().id(r.getChangeId()).get().isPrivate).isTrue();
}
@Test
public void publishDraftChangeByPushingNonDraftPatchSet() throws Exception {
// create draft change
PushOneCommit.Result r = pushTo("refs/drafts/master");
PushOneCommit.Result r = createDraftChange();
r.assertOkStatus();
r.assertChange(Change.Status.DRAFT, null);

3
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
View File

@@ -323,8 +323,7 @@ public class RefAdvertisementIT extends AbstractDaemonTest {
public void uploadPackDraftRefs() throws Exception {
allow("refs/heads/*", Permission.READ, REGISTERED_USERS);
PushOneCommit.Result br =
pushFactory.create(db, admin.getIdent(), testRepo).to("refs/drafts/master");
PushOneCommit.Result br = createDraftChange();
br.assertOkStatus();
Change.Id c5 = br.getChange().getId();
String r5 = changeRefPrefix(c5);

2
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java
View File

@@ -172,7 +172,7 @@ public class SubmitOnPushIT extends AbstractDaemonTest {
@Test
public void submitOnPushingDraft_Error() throws Exception {
PushOneCommit.Result r = pushTo("refs/for/master%draft,submit");
r.assertErrorStatus("cannot submit draft");
r.assertErrorStatus();
}
@Test

2
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
View File

@@ -551,7 +551,7 @@ public abstract class AbstractSubmit extends AbstractDaemonTest {
@Test
public void submitDraftPatchSet() throws Exception {
PushOneCommit.Result change = createChange();
PushOneCommit.Result draft = amendChangeAsDraft(change.getChangeId());
PushOneCommit.Result draft = amendChangeAndMarkPatchSetAsDraft(change.getChangeId());
Change.Id num = draft.getChange().getId();
submitWithConflict(

38
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ActionsIT.java
View File

@@ -22,7 +22,6 @@ import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_REVI
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.TestProjectInput;
import com.google.gerrit.extensions.api.changes.ActionVisitor;
import com.google.gerrit.extensions.api.changes.ReviewInput;
@@ -43,8 +42,6 @@ import java.util.EnumSet;
import java.util.Map;
import java.util.Set;
import java.util.TreeSet;
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.Config;
import org.junit.After;
import org.junit.Before;
@@ -151,7 +148,7 @@ public class ActionsIT extends AbstractDaemonTest {
String etag1 = getETag(change);
setApiUser(admin);
String draft = createDraftWithTopic().getChangeId();
String draft = createDraftChange("topic").getChangeId();
approve(draft);
setApiUser(user);
@@ -225,7 +222,7 @@ public class ActionsIT extends AbstractDaemonTest {
// create another change with the same topic
String changeId2 =
createChangeWithTopic(testRepo, "foo2", "touching b", "b.txt", "real content")
createChangeWithTopic(testRepo, "topic", "touching b", "b.txt", "real content")
.getChangeId();
int changeNum2 = gApi.changes().id(changeId2).info()._number;
approve(changeId2);
@@ -477,35 +474,4 @@ public class ActionsIT extends AbstractDaemonTest {
assertThat(actions).containsKey("description");
assertThat(actions).containsKey("rebase");
}
private PushOneCommit.Result createCommitAndPush(
TestRepository<InMemoryRepository> repo,
String ref,
String commitMsg,
String fileName,
String content)
throws Exception {
return pushFactory.create(db, admin.getIdent(), repo, commitMsg, fileName, content).to(ref);
}
private PushOneCommit.Result createChangeWithTopic(
TestRepository<InMemoryRepository> repo,
String topic,
String commitMsg,
String fileName,
String content)
throws Exception {
assertThat(topic).isNotEmpty();
return createCommitAndPush(
repo, "refs/for/master/" + name(topic), commitMsg, fileName, content);
}
private PushOneCommit.Result createChangeWithTopic() throws Exception {
return createChangeWithTopic(testRepo, "foo2", "a message", "a.txt", "content\n");
}
private PushOneCommit.Result createDraftWithTopic() throws Exception {
return createCommitAndPush(
testRepo, "refs/drafts/master/" + name("foo2"), "a message", "a.txt", "content\n");
}
}

21
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
View File

@@ -26,6 +26,7 @@ import com.google.common.base.Strings;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.GerritConfig;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.PushOneCommit.Result;
import com.google.gerrit.acceptance.RestResponse;
@@ -43,7 +44,6 @@ import com.google.gerrit.extensions.common.CommitInfo;
import com.google.gerrit.extensions.common.MergeInput;
import com.google.gerrit.extensions.common.RevisionInfo;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
@@ -51,13 +51,11 @@ import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.config.AnonymousCowardNameProvider;
import com.google.gerrit.server.git.ChangeAlreadyMergedException;
import com.google.gerrit.testutil.ConfigSuite;
import com.google.gerrit.testutil.FakeEmailSender.Message;
import com.google.gerrit.testutil.TestTimeUtil;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Repository;
@@ -69,11 +67,6 @@ import org.junit.BeforeClass;
import org.junit.Test;
public class CreateChangeIT extends AbstractDaemonTest {
@ConfigSuite.Config
public static Config allowDraftsDisabled() {
return allowDraftsDisabledConfig();
}
@BeforeClass
public static void setTimeForTesting() {
TestTimeUtil.resetWithClockStep(1, SECONDS);
@@ -135,8 +128,8 @@ public class CreateChangeIT extends AbstractDaemonTest {
@Test
public void createNewChangeSignedOffByFooter() throws Exception {
assume().that(isAllowDrafts()).isTrue();
setSignedOffByFooter();
ChangeInfo info = assertCreateSucceeds(newChangeInput(ChangeStatus.NEW));
String message = info.revisions.get(info.currentRevision).commit.message;
assertThat(message)
@@ -146,16 +139,10 @@ public class CreateChangeIT extends AbstractDaemonTest {
}
@Test
public void createNewDraftChange() throws Exception {
assume().that(isAllowDrafts()).isTrue();
assertCreateSucceeds(newChangeInput(ChangeStatus.DRAFT));
}
@Test
@GerritConfig(name = "change.allowDrafts", value = "true")
public void createNewDraftChangeNotAllowed() throws Exception {
assume().that(isAllowDrafts()).isFalse();
ChangeInput ci = newChangeInput(ChangeStatus.DRAFT);
assertCreateFails(ci, MethodNotAllowedException.class, "draft workflow is disabled");
assertCreateFails(ci, BadRequestException.class, "unsupported change status");
}
@Test

45
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/DeleteDraftPatchSetIT.java
View File

@@ -21,7 +21,6 @@ import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.PushOneCommit.Result;
import com.google.gerrit.acceptance.TestAccount;
import com.google.gerrit.extensions.api.changes.DraftInput;
import com.google.gerrit.extensions.api.changes.ReviewInput;
@@ -40,8 +39,6 @@ import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.inject.Inject;
import java.util.HashMap;
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
import org.junit.Test;
@@ -189,14 +186,9 @@ public class DeleteDraftPatchSetIT extends AbstractDaemonTest {
@Test
public void deleteCurrentDraftPatchSetWhenPreviousPatchSetDoesNotExist() throws Exception {
PushOneCommit push = pushFactory.create(db, admin.getIdent(), testRepo);
String changeId = push.to("refs/for/master").getChangeId();
pushFactory
.create(db, admin.getIdent(), testRepo, PushOneCommit.SUBJECT, "b.txt", "foo", changeId)
.to("refs/drafts/master");
pushFactory
.create(db, admin.getIdent(), testRepo, PushOneCommit.SUBJECT, "b.txt", "bar", changeId)
.to("refs/drafts/master");
String changeId = createChange().getChangeId();
amendChangeAndMarkPatchSetAsDraft(changeId);
amendChangeAndMarkPatchSetAsDraft(changeId);
deletePatchSet(changeId, 2);
deletePatchSet(changeId, 3);
@@ -209,20 +201,13 @@ public class DeleteDraftPatchSetIT extends AbstractDaemonTest {
@Test
public void deleteDraftPatchSetAndPushNewDraftPatchSet() throws Exception {
String ref = "refs/drafts/master";
// Clone repository
TestRepository<InMemoryRepository> testRepo = cloneProject(project, admin);
// Create change
PushOneCommit push = pushFactory.create(db, admin.getIdent(), testRepo);
PushOneCommit.Result r1 = push.to(ref);
r1.assertOkStatus();
PushOneCommit.Result r1 = createDraftChange();
String changeId = r1.getChangeId();
String revPs1 = r1.getChange().currentPatchSet().getRevision().get();
// Push draft patch set
PushOneCommit.Result r2 = amendChange(r1.getChangeId(), ref, admin, testRepo);
r2.assertOkStatus();
PushOneCommit.Result r2 = amendChangeAndMarkPatchSetAsDraft(changeId);
String revPs2 = r2.getChange().currentPatchSet().getRevision().get();
assertThat(gApi.changes().id(r1.getChange().getId().get()).get().currentRevision)
@@ -235,8 +220,7 @@ public class DeleteDraftPatchSetIT extends AbstractDaemonTest {
.isEqualTo(revPs1);
// Push new draft patch set
PushOneCommit.Result r3 = amendChange(r1.getChangeId(), ref, admin, testRepo);
r3.assertOkStatus();
amendChangeAndMarkPatchSetAsDraft(changeId);
String revPs3 = r2.getChange().currentPatchSet().getRevision().get();
assertThat(gApi.changes().id(r1.getChange().getId().get()).get().currentRevision)
@@ -259,21 +243,6 @@ public class DeleteDraftPatchSetIT extends AbstractDaemonTest {
}
}
private String createDraftChangeWith2PS() throws Exception {
PushOneCommit push = pushFactory.create(db, admin.getIdent(), testRepo);
Result result = push.to("refs/drafts/master");
push =
pushFactory.create(
db,
admin.getIdent(),
testRepo,
PushOneCommit.SUBJECT,
"b.txt",
"4711",
result.getChangeId());
return push.to("refs/drafts/master").getChangeId();
}
private PatchSet getCurrentPatchSet(String changeId) throws Exception {
return getChange(changeId).currentPatchSet();
}

62
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/DraftChangeIT.java
View File

@@ -29,6 +29,8 @@ import com.google.gerrit.extensions.client.ChangeStatus;
import com.google.gerrit.extensions.client.ReviewerState;
import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.EditInfo;
import com.google.gerrit.extensions.common.EditInfoSubject;
import com.google.gerrit.extensions.common.LabelInfo;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
@@ -38,6 +40,7 @@ import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.testutil.ConfigSuite;
import java.util.Collection;
import java.util.Optional;
import org.eclipse.jgit.lib.Config;
import org.junit.Test;
@@ -49,7 +52,7 @@ public class DraftChangeIT extends AbstractDaemonTest {
@Test
public void forceCreateAndPublishDraftChangeWhenAllowDraftsDisabled() throws Exception {
PushOneCommit.Result result = forceDraftChange();
PushOneCommit.Result result = createDraftChange();
result.assertOkStatus();
String changeId = result.getChangeId();
String triplet = project.get() + "~master~" + changeId;
@@ -105,7 +108,7 @@ public class DraftChangeIT extends AbstractDaemonTest {
pushFactory.create(db, user.getIdent(), testRepo).to("refs/for/master");
Change.Id id = changeResult.getChange().getId();
markChangeAsDraft(id);
setDraftStatusOfPatchSetsOfChange(id, true);
setDraftStatusOfPatchSets(id, true);
String changeId = changeResult.getChangeId();
exception.expect(MethodNotAllowedException.class);
@@ -125,7 +128,7 @@ public class DraftChangeIT extends AbstractDaemonTest {
pushFactory.create(db, user.getIdent(), testRepo).to("refs/for/master");
Change.Id id = changeResult.getChange().getId();
markChangeAsDraft(id);
setDraftStatusOfPatchSetsOfChange(id, true);
setDraftStatusOfPatchSets(id, true);
String changeId = changeResult.getChangeId();
@@ -151,7 +154,7 @@ public class DraftChangeIT extends AbstractDaemonTest {
PushOneCommit.Result changeResult = createDraftChange();
Change.Id id = changeResult.getChange().getId();
setDraftStatusOfPatchSetsOfChange(id, false);
setDraftStatusOfPatchSets(id, false);
String changeId = changeResult.getChangeId();
exception.expect(ResourceConflictException.class);
@@ -194,8 +197,8 @@ public class DraftChangeIT extends AbstractDaemonTest {
@Test
public void createDraftChangeWhenDraftsNotAllowed() throws Exception {
assume().that(isAllowDrafts()).isFalse();
PushOneCommit.Result r = createDraftChange();
r.assertErrorStatus("draft workflow is disabled");
PushOneCommit.Result r = pushTo("refs/drafts/master");
r.assertErrorStatus();
}
@Test
@@ -228,6 +231,53 @@ public class DraftChangeIT extends AbstractDaemonTest {
assertThat(label.all.get(0).value).isEqualTo(1);
}
@Test
public void pushWithDraftOptionGetsPrivateChange() throws Exception {
assume().that(isAllowDrafts()).isTrue();
PushOneCommit.Result result = createChange("refs/drafts/master");
String changeId = result.getChangeId();
ChangeInfo changeInfo = gApi.changes().id(changeId).get();
assertThat(changeInfo.status).isEqualTo(ChangeStatus.NEW);
assertThat(changeInfo.isPrivate).isEqualTo(true);
assertThat(changeInfo.revisions).hasSize(1);
assertThat(Iterables.getOnlyElement(changeInfo.revisions.values()).draft).isNull();
}
@Test
public void pushWithDraftOptionToExistingNewChangeGetsChangeEdit() throws Exception {
assume().that(isAllowDrafts()).isTrue();
pushWithDraftOptionToExistingChangeGetsChangeEdit(createChange().getChangeId());
}
@Test
public void pushWithDraftOptionToExistingDraftChangeGetsChangeEdit() throws Exception {
assume().that(isAllowDrafts()).isTrue();
pushWithDraftOptionToExistingChangeGetsChangeEdit(createDraftChange().getChangeId());
}
private void pushWithDraftOptionToExistingChangeGetsChangeEdit(String changeId) throws Exception {
Optional<EditInfo> edit = getEdit(changeId);
EditInfoSubject.assertThat(edit).isAbsent();
ChangeInfo changeInfo = gApi.changes().id(changeId).get();
ChangeStatus originalChangeStatus = changeInfo.status;
Boolean originalPatchSetStatus = Iterables.getOnlyElement(changeInfo.revisions.values()).draft;
PushOneCommit.Result result = amendChange(changeId, "refs/drafts/master");
result.assertOkStatus();
changeInfo = gApi.changes().id(changeId).get();
assertThat(changeInfo.status).isEqualTo(originalChangeStatus);
assertThat(changeInfo.isPrivate).isNull();
assertThat(changeInfo.revisions).hasSize(1);
assertThat(Iterables.getOnlyElement(changeInfo.revisions.values()).draft)
.isEqualTo(originalPatchSetStatus);
edit = getEdit(changeId);
EditInfoSubject.assertThat(edit).isPresent();
}
private static RestResponse deleteChange(String changeId, RestSession s) throws Exception {
return s.delete("/changes/" + changeId);
}

41
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/SubmittedTogetherIT.java
View File

@@ -29,6 +29,7 @@ import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.FileInfo;
import com.google.gerrit.extensions.common.RevisionInfo;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.testutil.ConfigSuite;
import java.util.EnumSet;
@@ -156,14 +157,8 @@ public class SubmittedTogetherIT extends AbstractDaemonTest {
@Test
public void hiddenDraftInTopic() throws Exception {
RevCommit initialHead = getRemoteHead();
RevCommit a = commitBuilder().add("a", "1").message("change 1").create();
pushHead(testRepo, "refs/for/master/" + name("topic"), false);
String id1 = getChangeId(a);
testRepo.reset(initialHead);
commitBuilder().add("b", "2").message("invisible change").create();
pushHead(testRepo, "refs/drafts/master/" + name("topic"), false);
String id1 = createChange("subject", "a", "1", "topic").getChangeId();
createDraftChange("topic");
setApiUser(user);
SubmittedTogetherInfo result =
@@ -181,14 +176,8 @@ public class SubmittedTogetherIT extends AbstractDaemonTest {
@Test
public void hiddenDraftInTopicOldApi() throws Exception {
RevCommit initialHead = getRemoteHead();
RevCommit a = commitBuilder().add("a", "1").message("change 1").create();
pushHead(testRepo, "refs/for/master/" + name("topic"), false);
String id1 = getChangeId(a);
testRepo.reset(initialHead);
commitBuilder().add("b", "2").message("invisible change").create();
pushHead(testRepo, "refs/drafts/master/" + name("topic"), false);
String id1 = createChange("subject", "a", "1", "topic").getChangeId();
createDraftChange("topic");
setApiUser(user);
if (isSubmitWholeTopicEnabled()) {
@@ -209,18 +198,16 @@ public class SubmittedTogetherIT extends AbstractDaemonTest {
String id1 = getChangeId(a1);
testRepo.reset(initialHead);
RevCommit parent = commitBuilder().message("parent").create();
pushHead(testRepo, "refs/for/master", false);
String parentId = getChangeId(parent);
String parentId = createChange().getChangeId();
// TODO(jrn): use insertChangeId(id1) once jgit TestRepository accepts
// the leading "I".
// TODO(jrn): use insertChangeId(id1) once jgit TestRepository accepts the leading "I".
commitBuilder()
.insertChangeId(id1.substring(1))
.add("a", "2")
.message("draft patch set on change 1")
.create();
pushHead(testRepo, "refs/drafts/master/" + name("topic"), false);
pushHead(testRepo, "refs/for/master/" + name("topic"), false);
setCurrentPatchSetAsDraft(new Change.Id(gApi.changes().id(id1).get()._number));
testRepo.reset(initialHead);
RevCommit b = commitBuilder().message("change with same topic").create();
@@ -243,16 +230,12 @@ public class SubmittedTogetherIT extends AbstractDaemonTest {
@Test
public void doNotRevealVisibleAncestorOfHiddenDraft() throws Exception {
RevCommit initialHead = getRemoteHead();
commitBuilder().message("parent").create();
pushHead(testRepo, "refs/for/master", false);
createChange().getChangeId();
commitBuilder().message("draft").create();
pushHead(testRepo, "refs/drafts/master/" + name("topic"), false);
createDraftChange("topic");
testRepo.reset(initialHead);
RevCommit change = commitBuilder().message("same topic").create();
pushHead(testRepo, "refs/for/master/" + name("topic"), false);
String id = getChangeId(change);
String id = createChange("subject", "b", "1", "topic").getChangeId();
setApiUser(user);
SubmittedTogetherInfo result =

47
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java
View File

@@ -42,6 +42,7 @@ import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.api.changes.ReviewInput.CommentInput;
import com.google.gerrit.extensions.api.changes.ReviewInput.DraftHandling;
import com.google.gerrit.extensions.client.Side;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.CommentInfo;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestApiException;
@@ -485,23 +486,23 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
// TODO(dborowitz): Re-enable these assertions once we fix auto-rebuilding
// in the BatchUpdate path.
//// As an implementation detail, change wasn't actually rebuilt inside the
//// BatchUpdate transaction, but it was rebuilt during read for the
//// subsequent reindex. Thus it's impossible to actually observe an
//// out-of-date state in the caller.
//assertChangeUpToDate(true, id);
// As an implementation detail, change wasn't actually rebuilt inside the
// BatchUpdate transaction, but it was rebuilt during read for the
// subsequent reindex. Thus it's impossible to actually observe an
// out-of-date state in the caller.
// assertChangeUpToDate(true, id);
//// Check that the bundles are equal.
//ChangeNotes notes = notesFactory.create(dbProvider.get(), project, id);
//ChangeBundle actual = ChangeBundle.fromNotes(commentsUtil, notes);
//ChangeBundle expected = bundleReader.fromReviewDb(getUnwrappedDb(), id);
//assertThat(actual.differencesFrom(expected)).isEmpty();
//assertThat(
// Check that the bundles are equal.
// ChangeNotes notes = notesFactory.create(dbProvider.get(), project, id);
// ChangeBundle actual = ChangeBundle.fromNotes(commentsUtil, notes);
// ChangeBundle expected = bundleReader.fromReviewDb(getUnwrappedDb(), id);
// assertThat(actual.differencesFrom(expected)).isEmpty();
// assertThat(
// Iterables.transform(
// notes.getChangeMessages(),
// ChangeMessage::getMessage))
// .contains(msg);
//assertThat(actual.getChange().getTopic()).isEqualTo(topic);
// assertThat(actual.getChange().getTopic()).isEqualTo(topic);
}
@Test
@@ -817,28 +818,16 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
@Test
public void deleteDraftPS1WithNoOtherEntities() throws Exception {
PushOneCommit push = pushFactory.create(db, admin.getIdent(), testRepo);
PushOneCommit.Result r = push.to("refs/drafts/master");
push =
pushFactory.create(
db,
admin.getIdent(),
testRepo,
PushOneCommit.SUBJECT,
"b.txt",
"4711",
r.getChangeId());
r = push.to("refs/drafts/master");
PatchSet.Id psId = r.getPatchSetId();
Change.Id id = psId.getParentKey();
gApi.changes().id(r.getChangeId()).revision(1).delete();
String r = createDraftChangeWith2PS();
gApi.changes().id(r).revision(1).delete();
ChangeInfo changeInfo = get(r);
Change.Id id = new Change.Id(changeInfo._number);
checker.rebuildAndCheckChanges(id);
setNotesMigration(true, true);
ChangeNotes notes = notesFactory.create(db, project, id);
assertThat(notes.getPatchSets().keySet()).containsExactly(psId);
assertThat(notes.getPatchSets().keySet()).hasSize(1);
}
@Test

130
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/ProjectWatchIT.java
View File

@@ -93,72 +93,6 @@ public class ProjectWatchIT extends AbstractDaemonTest {
assertThat(m.body()).contains("Gerrit-PatchSet: 2\n");
}
@Test
public void noNotificationForDraftChangesForWatchersInNotifyConfig() throws Exception {
Address addr = new Address("Watcher", "watcher@example.com");
NotifyConfig nc = new NotifyConfig();
nc.addEmail(addr);
nc.setName("team");
nc.setHeader(NotifyConfig.Header.TO);
nc.setTypes(EnumSet.of(NotifyType.NEW_CHANGES, NotifyType.ALL_COMMENTS));
ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
cfg.putNotifyConfig("team", nc);
saveProjectConfig(project, cfg);
PushOneCommit.Result r =
pushFactory
.create(db, admin.getIdent(), testRepo, "draft change", "a", "a1")
.to("refs/for/master%draft");
r.assertOkStatus();
assertThat(sender.getMessages()).isEmpty();
setApiUser(admin);
ReviewInput in = new ReviewInput();
in.message = "comment";
gApi.changes().id(r.getChangeId()).current().review(in);
assertThat(sender.getMessages()).isEmpty();
}
@Test
public void noNotificationForDraftPatchSetsForWatchersInNotifyConfig() throws Exception {
Address addr = new Address("Watcher", "watcher@example.com");
NotifyConfig nc = new NotifyConfig();
nc.addEmail(addr);
nc.setName("team");
nc.setHeader(NotifyConfig.Header.TO);
nc.setTypes(EnumSet.of(NotifyType.NEW_PATCHSETS, NotifyType.ALL_COMMENTS));
ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
cfg.putNotifyConfig("team", nc);
saveProjectConfig(project, cfg);
PushOneCommit.Result r =
pushFactory
.create(db, admin.getIdent(), testRepo, "subject", "a", "a1")
.to("refs/for/master");
r.assertOkStatus();
sender.clear();
r =
pushFactory
.create(db, admin.getIdent(), testRepo, "subject", "a", "a2", r.getChangeId())
.to("refs/for/master%draft");
r.assertOkStatus();
assertThat(sender.getMessages()).isEmpty();
setApiUser(admin);
ReviewInput in = new ReviewInput();
in.message = "comment";
gApi.changes().id(r.getChangeId()).current().review(in);
assertThat(sender.getMessages()).isEmpty();
}
@Test
public void noNotificationForPrivateChangesForWatchersInNotifyConfig() throws Exception {
Address addr = new Address("Watcher", "watcher@example.com");
@@ -528,70 +462,6 @@ public class ProjectWatchIT extends AbstractDaemonTest {
assertThat(sender.getMessages()).isEmpty();
}
@Test
public void watchProjectNoNotificationForDraftChange() throws Exception {
// watch project
String watchedProject = createProject("watchedProject").get();
setApiUser(user);
watch(watchedProject);
// push a draft change to watched project -> should not trigger email notification
setApiUser(admin);
TestRepository<InMemoryRepository> watchedRepo =
cloneProject(new Project.NameKey(watchedProject), admin);
PushOneCommit.Result r =
pushFactory
.create(db, admin.getIdent(), watchedRepo, "draft change", "a", "a1")
.to("refs/for/master%draft");
r.assertOkStatus();
// assert email notification
assertThat(sender.getMessages()).isEmpty();
}
@Test
public void watchProjectNotifyOnDraftChange() throws Exception {
String watchedProject = createProject("watchedProject").get();
// create group that can view all drafts
GroupInfo groupThatCanViewDrafts = gApi.groups().create("groupThatCanViewDrafts").get();
grant(
new Project.NameKey(watchedProject),
"refs/*",
Permission.VIEW_DRAFTS,
false,
new AccountGroup.UUID(groupThatCanViewDrafts.id));
// watch project as user that can't view drafts
setApiUser(user);
watch(watchedProject);
// watch project as user that can view all drafts
TestAccount userThatCanViewDrafts =
accountCreator.create("user2", "user2@test.com", "User2", groupThatCanViewDrafts.name);
setApiUser(userThatCanViewDrafts);
watch(watchedProject);
// push a draft change to watched project -> should trigger email notification for
// userThatCanViewDrafts, but not for user
setApiUser(admin);
TestRepository<InMemoryRepository> watchedRepo =
cloneProject(new Project.NameKey(watchedProject), admin);
PushOneCommit.Result r =
pushFactory
.create(db, admin.getIdent(), watchedRepo, "TRIGGER", "a", "a1")
.to("refs/for/master%draft");
r.assertOkStatus();
// assert email notification
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
assertThat(m.rcpt()).containsExactly(userThatCanViewDrafts.emailAddress);
assertThat(m.body()).contains("Change subject: TRIGGER\n");
assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
}
@Test
public void watchProjectNoNotificationForIgnoredChange() throws Exception {
// watch project

2
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/QueryIT.java
View File

@@ -292,7 +292,7 @@ public class QueryIT extends AbstractDaemonTest {
@Test
public void queryWithNonVisibleCurrentPatchSet() throws Exception {
String changeId = createChange().getChangeId();
amendChangeAsDraft(changeId);
amendChangeAndMarkPatchSetAsDraft(changeId);
String query = "--current-patch-set --patch-sets " + changeId;
List<ChangeAttribute> changes = executeSuccessfulQuery(query);
assertThat(changes.size()).isEqualTo(1);