Ensure that 'Verified' label is handled by default
Add the integration tests that check handling of the 'Verified' label.
Assumptions:
* having 'CR+2' casted by the file owner is not sufficient if 'V+1' is
missing
* having 'V+1' casted by the file owner is not sufficient if 'CR+2' is
missing
Note:
In the setup where more than one label is votable (e.g. where 'Code
Review' and 'Verified' are defined) one needs to configure the owners
plugin which label is binding. This change introduce a method that will
be responsible for getting it from the configuration but for the time
being it is stubbed to always return 'Code Review' (or nothing in case
the 'Code Review' is not present in the setup).
Bug: Issue 15556
Change-Id: I85b989e74f7837365f3c26266bbcfac38e1d5381
diff --git a/owners/src/main/java/com/googlesource/gerrit/owners/OwnersSubmitRequirement.java b/owners/src/main/java/com/googlesource/gerrit/owners/OwnersSubmitRequirement.java
index 3a7334b..2fe4952 100644
--- a/owners/src/main/java/com/googlesource/gerrit/owners/OwnersSubmitRequirement.java
+++ b/owners/src/main/java/com/googlesource/gerrit/owners/OwnersSubmitRequirement.java
@@ -25,6 +25,7 @@
import com.google.gerrit.entities.Account.Id;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.LabelFunction;
+import com.google.gerrit.entities.LabelId;
import com.google.gerrit.entities.LabelType;
import com.google.gerrit.entities.LabelTypes;
import com.google.gerrit.entities.LegacySubmitRequirement;
@@ -140,7 +141,7 @@
ChangeNotes notes = cd.notes();
requireNonNull(notes, "notes");
- LabelTypes labelTypes = projectState.getLabelTypes(notes);
+ LabelTypes labelTypes = ownersLabel(projectState.getLabelTypes(notes));
Account.Id uploader = notes.getCurrentPatchSet().uploader();
Map<Account.Id, List<PatchSetApproval>> approvalsByAccount =
Streams.stream(approvalsUtil.byPatchSet(notes, cd.currentPatchSet().id()))
@@ -171,6 +172,19 @@
}
}
+ /** the idea is to select the label type that is configured for owner to cast the vote */
+ private LabelTypes ownersLabel(LabelTypes labelTypes) {
+
+ // TODO: there is no specific label configuration introduced to the OWNERS file therefore for
+ // the time being set it to Code Review explicitly or nothing if it doesn't exist for the
+ // project in question
+ return new LabelTypes(
+ labelTypes
+ .byLabel(LabelId.CODE_REVIEW)
+ .map(Collections::singletonList)
+ .orElseGet(Collections::emptyList));
+ }
+
static boolean isApprovalMissing(
Map.Entry<String, Set<Account.Id>> requiredApproval,
Account.Id uploader,
diff --git a/owners/src/test/java/com/googlesource/gerrit/owners/OwnersSubmitRequirementIT.java b/owners/src/test/java/com/googlesource/gerrit/owners/OwnersSubmitRequirementIT.java
index 9c507be..b611cde 100644
--- a/owners/src/test/java/com/googlesource/gerrit/owners/OwnersSubmitRequirementIT.java
+++ b/owners/src/test/java/com/googlesource/gerrit/owners/OwnersSubmitRequirementIT.java
@@ -16,6 +16,11 @@
package com.googlesource.gerrit.owners;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth8.assertThat;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allowLabel;
+import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
+import static com.google.gerrit.server.project.testing.TestLabels.labelBuilder;
+import static com.google.gerrit.server.project.testing.TestLabels.value;
import static java.util.stream.Collectors.joining;
import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
@@ -24,14 +29,26 @@
import com.google.gerrit.acceptance.TestPlugin;
import com.google.gerrit.acceptance.UseLocalDisk;
import com.google.gerrit.acceptance.config.GlobalPluginConfig;
+import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
+import com.google.gerrit.entities.LabelFunction;
+import com.google.gerrit.entities.LabelId;
+import com.google.gerrit.entities.LabelType;
+import com.google.gerrit.entities.RefNames;
import com.google.gerrit.extensions.api.changes.ChangeApi;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.LegacySubmitRequirementInfo;
+import com.google.gerrit.extensions.common.SubmitRecordInfo;
import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.server.git.meta.MetaDataUpdate;
+import com.google.gerrit.server.project.ProjectConfig;
import com.google.inject.Inject;
+import java.io.IOException;
+import java.util.Collection;
import java.util.stream.Stream;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.junit.Test;
@TestPlugin(name = "owners", sysModule = "com.googlesource.gerrit.owners.OwnersModule")
@@ -43,6 +60,7 @@
new LegacySubmitRequirementInfo("OK", "Owners", "owners");
@Inject private RequestScopeOperations requestScopeOperations;
+ @Inject private ProjectOperations projectOperations;
@Test
@GlobalPluginConfig(
@@ -145,6 +163,98 @@
assertThat(forChange(r).get().submittable).isFalse();
}
+ @Test
+ @GlobalPluginConfig(
+ pluginName = "owners",
+ name = "owners.enableSubmitRequirement",
+ value = "true")
+ public void shouldRequireVerifiedApprovalEvenIfCodeOwnerApproved() throws Exception {
+ TestAccount admin2 = accountCreator.admin2();
+ addOwnerFileToRoot(true, admin2);
+
+ installVerifiedLabel();
+
+ PushOneCommit.Result r = createChange("Add a file", "foo", "bar");
+ ChangeApi changeApi = forChange(r);
+ assertThat(changeApi.get().submittable).isFalse();
+ assertThat(changeApi.get().requirements).containsExactly(NOT_READY);
+
+ requestScopeOperations.setApiUser(admin2.id());
+ forChange(r).current().review(ReviewInput.approve());
+ assertThat(forChange(r).get().submittable).isFalse();
+ assertThat(forChange(r).get().requirements).containsExactly(READY);
+ verifyHasSubmitRecord(
+ forChange(r).get().submitRecords, LabelId.VERIFIED, SubmitRecordInfo.Label.Status.NEED);
+
+ changeApi.current().review(new ReviewInput().label(LabelId.VERIFIED, 1));
+ assertThat(changeApi.get().submittable).isTrue();
+ verifyHasSubmitRecord(
+ changeApi.get().submitRecords, LabelId.VERIFIED, SubmitRecordInfo.Label.Status.OK);
+ }
+
+ @Test
+ @GlobalPluginConfig(
+ pluginName = "owners",
+ name = "owners.enableSubmitRequirement",
+ value = "true")
+ public void shouldRequireCodeOwnerApprovalEvenIfVerifiedWasApproved() throws Exception {
+ TestAccount admin2 = accountCreator.admin2();
+ addOwnerFileToRoot(true, admin2);
+
+ installVerifiedLabel();
+
+ PushOneCommit.Result r = createChange("Add a file", "foo", "bar");
+ ChangeApi changeApi = forChange(r);
+ assertThat(changeApi.get().submittable).isFalse();
+ assertThat(changeApi.get().requirements).containsExactly(NOT_READY);
+
+ requestScopeOperations.setApiUser(admin2.id());
+ forChange(r).current().review(new ReviewInput().label(LabelId.VERIFIED, 1));
+ ChangeInfo changeNotReady = forChange(r).get();
+ assertThat(changeNotReady.submittable).isFalse();
+ assertThat(changeNotReady.requirements).containsExactly(NOT_READY);
+ verifyHasSubmitRecord(
+ changeNotReady.submitRecords, LabelId.VERIFIED, SubmitRecordInfo.Label.Status.OK);
+
+ forChange(r).current().review(ReviewInput.approve());
+ ChangeInfo changeReady = forChange(r).get();
+ assertThat(changeReady.submittable).isTrue();
+ assertThat(changeReady.requirements).containsExactly(READY);
+ }
+
+ private void verifyHasSubmitRecord(
+ Collection<SubmitRecordInfo> records, String label, SubmitRecordInfo.Label.Status status) {
+ assertThat(
+ records.stream()
+ .flatMap(record -> record.labels.stream())
+ .filter(l -> l.label.equals(label) && l.status == status)
+ .findAny())
+ .isPresent();
+ }
+
+ private void installVerifiedLabel()
+ throws IOException, ConfigInvalidException, RepositoryNotFoundException {
+ LabelType verified =
+ labelBuilder(
+ LabelId.VERIFIED, value(1, "Verified"), value(0, "No score"), value(-1, "Fails"))
+ .setFunction(LabelFunction.MAX_WITH_BLOCK)
+ .build();
+ try (MetaDataUpdate md = metaDataUpdateFactory.create(project)) {
+ ProjectConfig cfg = projectConfigFactory.create(project);
+ cfg.load(md);
+ cfg.upsertLabelType(verified);
+ cfg.commit(md);
+ }
+ projectCache.evictAndReindex(project);
+
+ String heads = RefNames.REFS_HEADS + "*";
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allowLabel(verified.getName()).ref(heads).group(REGISTERED_USERS).range(-1, 1))
+ .update();
+ }
+
private ChangeApi forChange(PushOneCommit.Result r) throws RestApiException {
return gApi.changes().id(r.getChangeId());
}
