ChangeIT: Assert that submitting a change doesn't remove non-voting reviewers

If a project is configured with two approval labels, for example Verified
and Code-Review, and a user has permission to approve on only one of them,
when the change is submitted without the user giving any review, the user
should not be removed from the review.

This commit is cherry-picked from stable-2.12 and adjusted to work on master
with noteDB.

Bug: Issue 4163
Change-Id: Iff87398d3fba2ca0b53f0040a1ebd122cc0e079b
This commit is contained in:
David Pursehouse
2016-06-23 13:11:15 +09:00
parent e7c0364756
commit 8f9efadeda
2 changed files with 89 additions and 0 deletions

View File

@@ -814,6 +814,91 @@ public class ChangeIT extends AbstractDaemonTest {
.deleteVote("Code-Review");
}
@Test
public void nonVotingReviewerStaysAfterSubmit() throws Exception {
LabelType verified = category("Verified",
value(1, "Passes"), value(0, "No score"), value(-1, "Failed"));
ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
cfg.getLabelSections().put(verified.getName(), verified);
String heads = "refs/heads/*";
AccountGroup.UUID owners =
SystemGroupBackend.getGroup(CHANGE_OWNER).getUUID();
AccountGroup.UUID registered =
SystemGroupBackend.getGroup(REGISTERED_USERS).getUUID();
Util.allow(cfg,
Permission.forLabel(verified.getName()), -1, 1, owners, heads);
Util.allow(cfg,
Permission.forLabel("Code-Review"), -2, +2, registered, heads);
saveProjectConfig(project, cfg);
// Set Code-Review+2 and Verified+1 as admin (change owner)
PushOneCommit.Result r = createChange();
String changeId = r.getChangeId();
String commit = r.getCommit().name();
ReviewInput input = ReviewInput.approve();
input.label(verified.getName(), 1);
gApi.changes()
.id(changeId)
.revision(commit)
.review(input);
// Reviewers should only be "admin"
ChangeInfo c = gApi.changes().id(changeId).get();
assertThat(getReviewers(c.reviewers.get(REVIEWER)))
.containsExactlyElementsIn(ImmutableSet.of(admin.getId()));
assertThat(c.reviewers.get(CC)).isNull();
// Add the user as reviewer
AddReviewerInput in = new AddReviewerInput();
in.reviewer = user.email;
gApi.changes()
.id(changeId)
.addReviewer(in);
c = gApi.changes().id(changeId).get();
if (NoteDbMode.readWrite()) {
assertThat(getReviewers(c.reviewers.get(REVIEWER)))
.containsExactlyElementsIn(ImmutableSet.of(
admin.getId(), user.getId()));
} else {
assertThat(getReviewers(c.reviewers.get(REVIEWER)))
.containsExactlyElementsIn(ImmutableSet.of(admin.getId()));
assertThat(getReviewers(c.reviewers.get(CC)))
.containsExactlyElementsIn(ImmutableSet.of(user.getId()));
}
// Approve the change as user, then remove the approval
// (only to confirm that the user does have Code-Review+2 permission)
setApiUser(user);
gApi.changes()
.id(changeId)
.revision(commit)
.review(ReviewInput.approve());
gApi.changes()
.id(changeId)
.revision(commit)
.review(ReviewInput.noScore());
// Submit the change
setApiUser(admin);
gApi.changes()
.id(changeId)
.revision(commit)
.submit();
// User should still be on the change
c = gApi.changes().id(changeId).get();
if (NoteDbMode.readWrite()) {
assertThat(getReviewers(c.reviewers.get(REVIEWER)))
.containsExactlyElementsIn(ImmutableSet.of(
admin.getId(), user.getId()));
} else {
assertThat(getReviewers(c.reviewers.get(REVIEWER)))
.containsExactlyElementsIn(ImmutableSet.of(admin.getId()));
assertThat(getReviewers(c.reviewers.get(CC)))
.containsExactlyElementsIn(ImmutableSet.of(user.getId()));
}
}
@Test
public void createEmptyChange() throws Exception {
ChangeInput in = new ChangeInput();

View File

@@ -124,6 +124,10 @@ public class ReviewInput {
return new ReviewInput().label("Code-Review", -1);
}
public static ReviewInput noScore() {
return new ReviewInput().label("Code-Review", 0);
}
public static ReviewInput approve() {
return new ReviewInput().label("Code-Review", 2);
}