Always expose `has:approval_owners` submit requirement predicate
The `owners.enableSubmitRequirement = true` is a global switch that
allows for quick site switch from `prolog` submit rule to plugin's
provided submit rule. However in case when such migration is not
feasible (e.g. when `prolog` rules need to be slowly phased out or when
more control is needed over rule's applicability) one can now turn
submit requirement in `project.config` for a certain project (or
hierarchy of projects), without turning it on globally.
Changes:
* the `approval_owners` submit requirement operand is always registered
* operand propagates rule error through the
`SubmitRequirementEvaluationException` so that it can be properly
handled in the UI
* all existing integration tests were adjusted to verify that plugin
works correctly also in new mode
* the configuration was updated to highlight such possibility
Bug: Issue 345837787
Change-Id: Iaca45dd47e23d5f2d40a2732b52abdaeba170e8d
diff --git a/owners/src/main/java/com/googlesource/gerrit/owners/OwnersApprovalHasPredicate.java b/owners/src/main/java/com/googlesource/gerrit/owners/OwnersApprovalHasPredicate.java
index bb2d474..4e8f3b0 100644
--- a/owners/src/main/java/com/googlesource/gerrit/owners/OwnersApprovalHasPredicate.java
+++ b/owners/src/main/java/com/googlesource/gerrit/owners/OwnersApprovalHasPredicate.java
@@ -16,6 +16,7 @@
import com.google.gerrit.entities.SubmitRecord;
import com.google.gerrit.extensions.annotations.PluginName;
+import com.google.gerrit.server.project.SubmitRequirementEvaluationException;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.SubmitRequirementPredicate;
import com.google.gerrit.server.rules.SubmitRule;
@@ -43,7 +44,15 @@
@Override
public boolean match(ChangeData cd) {
Optional<SubmitRecord> submitRecord = ownersSubmitRequirement.evaluate(cd);
- return submitRecord.map(sr -> sr.status == SubmitRecord.Status.OK).orElse(true);
+ return submitRecord
+ .map(
+ sr -> {
+ if (sr.status == SubmitRecord.Status.RULE_ERROR) {
+ throw new SubmitRequirementEvaluationException(sr.errorMessage);
+ }
+ return sr.status == SubmitRecord.Status.OK;
+ })
+ .orElse(true);
}
/**
diff --git a/owners/src/main/java/com/googlesource/gerrit/owners/OwnersModule.java b/owners/src/main/java/com/googlesource/gerrit/owners/OwnersModule.java
index 7d2c4d5..be8a6ca 100644
--- a/owners/src/main/java/com/googlesource/gerrit/owners/OwnersModule.java
+++ b/owners/src/main/java/com/googlesource/gerrit/owners/OwnersModule.java
@@ -40,13 +40,12 @@
.to(OwnerPredicateProvider.class)
.asEagerSingleton();
install(new OwnersRestApiModule());
+ install(new OwnersApprovalHasOperand.OwnerApprovalHasOperandModule());
if (pluginSettings.enableSubmitRequirement()) {
install(new OwnersSubmitRequirement.OwnersSubmitRequirementModule());
- install(new OwnersApprovalHasOperand.OwnerApprovalHasOperandModule());
- } else {
- logger.atInfo().log(
- "OwnersSubmitRequirement is disabled therefore it will not be evaluated.");
}
+ logger.atInfo().log(
+ "Global `owners.enableSubmitRequirement = %b`.", pluginSettings.enableSubmitRequirement());
}
}
diff --git a/owners/src/main/resources/Documentation/config.md b/owners/src/main/resources/Documentation/config.md
index 6e62857..1d157c6 100644
--- a/owners/src/main/resources/Documentation/config.md
+++ b/owners/src/main/resources/Documentation/config.md
@@ -28,9 +28,11 @@
expandGroups = false
```
-owners.enableSubmitRequirement
-: If set to `true` the approvals are evaluated through the owners submit rule without a need of
- prolog predicate being added to a project. Defaults to `false`.
+<a name="owners.enableSubmitRequirement">owners.enableSubmitRequirement</a>
+: If set to `true` the approvals are evaluated through the owners plugin
+ provided submit rule without a need of prolog predicate being added to a
+ project or submit requirement configured in the `project.config` as it is
+ automatically applied to all projects. Defaults to `false`.
Example:
@@ -39,6 +41,31 @@
enableSubmitRequirement = true
```
+ > **Notes:**
+ >
+ > The `owners.enableSubmitRequirement = true` is a global
+ > setting and allows for quick site switch from `prolog` submit rule to
+ > plugin's provided submit rule. It is a drop-in replacement therefore,
+ > similarly to `prolog` rule, it cannot be overridden by Gerrit. In case
+ > when one-step migration is not feasible (e.g. when `prolog` rules need to
+ > be slowly phased out or when more control is needed over rule's
+ > applicability, submitability or ability to overide) one can configure
+ > submit requirement in `project.config` for a certain project (or
+ > hierarchy of projects), without turning it on globally, as
+ > `approval_owners` predicate is _always_ available.
+ >
+ > Please also note, that project's `rules.pl` should be removed in this
+ > case so that it doesn't interfere with a change evaluation.
+ >
+ > The minial configuration looks like below (see
+ > [submit requirements documentation](/Documentation/config-submit-requirements.html) for more details):
+ > ```
+ > [submit-requirement "Owner-Approval"]
+ > description = Files needs to be approved by owners
+ > submittableIf = has:approval_owners
+ > ```
+
+
cache."owners.path_owners_entries".memoryLimit
: The cache is used to hold the parsed version of `OWNERS` files in the
repository so that when submit rules are calculated (either through prolog
@@ -185,6 +212,9 @@
`Code-Review from owners` requirement is added making the change not
submittable.
+> See [notes](#owners.enableSubmitRequirement) for an example on how to enable
+> submit requirements for a specific project only.
+
### When `owners.enableSubmitRequirement = false` (default)
And sample rules.pl that uses this predicate to enable the submit rule if
@@ -258,6 +288,9 @@
As a result Gerrit would make the change Submittable only if 'John Doe' or
'Doug Smith' have provided at least a `Code-Review +1`.
+> See [notes](#owners.enableSubmitRequirement) for an example on how to enable
+> submit requirements for a specific project only.
+
### When `owners.enableSubmitRequirement = false` (default)
And a rule which makes submittable a change if at least one of the owners has
@@ -318,6 +351,9 @@
A change cannot be submitted until 'John Doe' or 'Doug Smith' add a label
`Owner-Approved`, independently from being able to provide any Code-Review.
+> See [notes](#owners.enableSubmitRequirement) for an example on how to enable
+> submit requirements for a specific project only.
+
### When `owners.enableSubmitRequirement = false` (default)
Finally, define prolog rules as shown in below (an amended version of
@@ -367,6 +403,9 @@
`Code-Review+2` for a change to be submittable as default submit rule is
still evaluated.
+> See [notes](#owners.enableSubmitRequirement) for an example on how to enable
+> submit requirements for a specific project only.
+
## Example 5 - Owners based on matchers
Often the ownership comes from the developer's skills and competencies and
@@ -398,6 +437,9 @@
have provided their `Code-Review +2` feedback (as `Code-Review` is default
label for owners submit requirement).
+> See [notes](#owners.enableSubmitRequirement) for an example on how to enable
+> submit requirements for a specific project only.
+
### When `owners.enableSubmitRequirement = false` (default)
And a rules.pl of:
diff --git a/owners/src/test/java/com/googlesource/gerrit/owners/OwnersApprovalHasOperandIT.java b/owners/src/test/java/com/googlesource/gerrit/owners/OwnersApprovalHasOperandIT.java
index 55bfa3c..7e057cd 100644
--- a/owners/src/test/java/com/googlesource/gerrit/owners/OwnersApprovalHasOperandIT.java
+++ b/owners/src/test/java/com/googlesource/gerrit/owners/OwnersApprovalHasOperandIT.java
@@ -16,6 +16,7 @@
package com.googlesource.gerrit.owners;
import static com.google.common.collect.ImmutableList.toImmutableList;
+import static com.google.gerrit.extensions.common.SubmitRequirementResultInfo.Status.ERROR;
import static com.google.gerrit.extensions.common.SubmitRequirementResultInfo.Status.SATISFIED;
import static com.google.gerrit.extensions.common.SubmitRequirementResultInfo.Status.UNSATISFIED;
@@ -23,7 +24,8 @@
import com.google.gerrit.acceptance.TestAccount;
import com.google.gerrit.acceptance.TestPlugin;
import com.google.gerrit.acceptance.UseLocalDisk;
-import com.google.gerrit.acceptance.config.GlobalPluginConfig;
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.entities.Project.NameKey;
import com.google.gerrit.entities.SubmitRequirement;
import com.google.gerrit.entities.SubmitRequirementExpression;
import com.google.gerrit.extensions.api.changes.ChangeApi;
@@ -44,20 +46,10 @@
@Before
public void setup() throws Exception {
- configSubmitRequirement(
- project,
- SubmitRequirement.builder()
- .setName(REQUIREMENT_NAME)
- .setSubmittabilityExpression(SubmitRequirementExpression.create("has:approval_owners"))
- .setAllowOverrideInChildProjects(false)
- .build());
+ enableSubmitRequirementsPredicateForProject(project);
}
@Test
- @GlobalPluginConfig(
- pluginName = "owners",
- name = "owners.enableSubmitRequirement",
- value = "true")
public void shouldOwnersRequirementBeSatisfied() throws Exception {
TestAccount admin2 = accountCreator.admin2();
addOwnerFileToRoot(true, LabelDefinition.parse("Code-Review,1").get(), admin2);
@@ -65,12 +57,34 @@
PushOneCommit.Result r = createChange("Add a file", "foo", "bar");
ChangeApi changeApi = forChange(r);
ChangeInfo changeNotReady = changeApi.get(ListChangesOption.SUBMIT_REQUIREMENTS);
- verifySubmitRequirements(changeNotReady.submitRequirements, REQUIREMENT_NAME, UNSATISFIED);
+ verifyChangeNotReady(changeNotReady);
requestScopeOperations.setApiUser(admin2.id());
forChange(r).current().review(ReviewInput.recommend());
ChangeInfo ownersVoteSufficient = forChange(r).get(ListChangesOption.SUBMIT_REQUIREMENTS);
- verifySubmitRequirements(ownersVoteSufficient.submitRequirements, REQUIREMENT_NAME, SATISFIED);
+ verifyChangeReady(ownersVoteSufficient);
+ }
+
+ @Override
+ protected void verifyChangeNotReady(ChangeInfo notReady) {
+ verifySubmitRequirements(notReady.submitRequirements, REQUIREMENT_NAME, UNSATISFIED);
+ }
+
+ @Override
+ protected void verifyChangeReady(ChangeInfo ready) {
+ verifySubmitRequirements(ready.submitRequirements, REQUIREMENT_NAME, SATISFIED);
+ }
+
+ @Override
+ protected void verifyRuleError(ChangeInfo change) {
+ verifySubmitRequirements(change.submitRequirements, REQUIREMENT_NAME, ERROR);
+ }
+
+ @Override
+ protected void updateChildProjectConfiguration(NameKey childProject) throws Exception {
+ // submit requirement predicate has to be configured either on a project level or somewhere in
+ // the project hierarchy
+ enableSubmitRequirementsPredicateForProject(childProject);
}
private void verifySubmitRequirements(
@@ -90,4 +104,15 @@
.map(r -> String.format("%s=%s", r.name, r.status))
.collect(toImmutableList())));
}
+
+ private void enableSubmitRequirementsPredicateForProject(Project.NameKey forProject)
+ throws Exception {
+ configSubmitRequirement(
+ forProject,
+ SubmitRequirement.builder()
+ .setName(REQUIREMENT_NAME)
+ .setSubmittabilityExpression(SubmitRequirementExpression.create("has:approval_owners"))
+ .setAllowOverrideInChildProjects(false)
+ .build());
+ }
}
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 0d5c2c5..0e7e40b 100644
--- a/owners/src/test/java/com/googlesource/gerrit/owners/OwnersSubmitRequirementIT.java
+++ b/owners/src/test/java/com/googlesource/gerrit/owners/OwnersSubmitRequirementIT.java
@@ -15,9 +15,64 @@
package com.googlesource.gerrit.owners;
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth8.assertThat;
+
import com.google.gerrit.acceptance.TestPlugin;
import com.google.gerrit.acceptance.UseLocalDisk;
+import com.google.gerrit.entities.Project.NameKey;
+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.server.config.SitePaths;
+import com.google.inject.Inject;
+import org.eclipse.jgit.storage.file.FileBasedConfig;
+import org.eclipse.jgit.util.FS;
@TestPlugin(name = "owners", sysModule = "com.googlesource.gerrit.owners.OwnersModule")
@UseLocalDisk
-public class OwnersSubmitRequirementIT extends OwnersSubmitRequirementITAbstract {}
+public class OwnersSubmitRequirementIT extends OwnersSubmitRequirementITAbstract {
+ private static final LegacySubmitRequirementInfo NOT_READY =
+ new LegacySubmitRequirementInfo("NOT_READY", "Owners", "owners");
+ private static final LegacySubmitRequirementInfo READY =
+ new LegacySubmitRequirementInfo("OK", "Owners", "owners");
+
+ @Inject private SitePaths sitePaths;
+
+ @Override
+ public void setUpTestPlugin() throws Exception {
+ // ensure that `owners.enableSubmitRequirements = true` for each integration test without
+ // relying on `GlobalPluginConfig` annotation
+ FileBasedConfig ownersConfig =
+ new FileBasedConfig(sitePaths.etc_dir.resolve("owners.config").toFile(), FS.DETECTED);
+ ownersConfig.setBoolean("owners", null, "enableSubmitRequirement", true);
+ ownersConfig.save();
+ super.setUpTestPlugin();
+ }
+
+ @Override
+ protected void verifyChangeNotReady(ChangeInfo notReady) {
+ assertThat(notReady.requirements).containsExactly(NOT_READY);
+ }
+
+ @Override
+ protected void verifyChangeReady(ChangeInfo ready) {
+ assertThat(ready.requirements).containsExactly(READY);
+ }
+
+ @Override
+ protected void verifyRuleError(ChangeInfo change) {
+ assertThat(
+ change.submitRecords.stream()
+ .filter(record -> SubmitRecordInfo.Status.RULE_ERROR == record.status)
+ .filter(record -> record.errorMessage.startsWith("Invalid owners file: OWNERS"))
+ .findAny())
+ .isPresent();
+ }
+
+ @Override
+ protected void updateChildProjectConfiguration(NameKey childProject) {
+ // there is no need to further customize project when `owners.enableSubmitRequirements = true`
+ // as it is globally enabled
+ }
+}
diff --git a/owners/src/test/java/com/googlesource/gerrit/owners/OwnersSubmitRequirementITAbstract.java b/owners/src/test/java/com/googlesource/gerrit/owners/OwnersSubmitRequirementITAbstract.java
index 3f7362d..2135b40 100644
--- a/owners/src/test/java/com/googlesource/gerrit/owners/OwnersSubmitRequirementITAbstract.java
+++ b/owners/src/test/java/com/googlesource/gerrit/owners/OwnersSubmitRequirementITAbstract.java
@@ -27,7 +27,6 @@
import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.TestAccount;
-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;
@@ -39,7 +38,6 @@
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.client.SubmitType;
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.project.testing.TestLabels;
@@ -52,19 +50,10 @@
import org.junit.Test;
abstract class OwnersSubmitRequirementITAbstract extends LightweightPluginDaemonTest {
- private static final LegacySubmitRequirementInfo NOT_READY =
- new LegacySubmitRequirementInfo("NOT_READY", "Owners", "owners");
- private static final LegacySubmitRequirementInfo READY =
- new LegacySubmitRequirementInfo("OK", "Owners", "owners");
-
@Inject protected RequestScopeOperations requestScopeOperations;
@Inject private ProjectOperations projectOperations;
@Test
- @GlobalPluginConfig(
- pluginName = "owners",
- name = "owners.enableSubmitRequirement",
- value = "true")
public void shouldRequireAtLeastOneApprovalForMatchingPathFromOwner() throws Exception {
TestAccount admin2 = accountCreator.admin2();
TestAccount user1 = accountCreator.user1();
@@ -74,25 +63,21 @@
ChangeApi changeApi = forChange(r);
ChangeInfo changeNotReady = changeApi.get();
assertThat(changeNotReady.submittable).isFalse();
- assertThat(changeNotReady.requirements).containsExactly(NOT_READY);
+ verifyChangeNotReady(changeNotReady);
changeApi.current().review(ReviewInput.approve());
ChangeInfo changeNotReadyAfterSelfApproval = changeApi.get();
assertThat(changeNotReadyAfterSelfApproval.submittable).isFalse();
- assertThat(changeNotReadyAfterSelfApproval.requirements).containsExactly(NOT_READY);
+ verifyChangeNotReady(changeNotReadyAfterSelfApproval);
requestScopeOperations.setApiUser(admin2.id());
forChange(r).current().review(ReviewInput.approve());
ChangeInfo changeReady = forChange(r).get();
assertThat(changeReady.submittable).isTrue();
- assertThat(changeReady.requirements).containsExactly(READY);
+ verifyChangeReady(changeReady);
}
@Test
- @GlobalPluginConfig(
- pluginName = "owners",
- name = "owners.enableSubmitRequirement",
- value = "true")
public void shouldNotRequireApprovalForNotMatchingPath() throws Exception {
TestAccount admin2 = accountCreator.admin2();
addOwnerFileWithMatchersToRoot(true, ".md", admin2);
@@ -110,10 +95,6 @@
}
@Test
- @GlobalPluginConfig(
- pluginName = "owners",
- name = "owners.enableSubmitRequirement",
- value = "true")
public void shouldRequireApprovalFromRootOwner() throws Exception {
TestAccount admin2 = accountCreator.admin2();
addOwnerFileToRoot(true, admin2);
@@ -122,25 +103,21 @@
ChangeApi changeApi = forChange(r);
ChangeInfo changeNotReady = changeApi.get();
assertThat(changeNotReady.submittable).isFalse();
- assertThat(changeNotReady.requirements).containsExactly(NOT_READY);
+ verifyChangeNotReady(changeNotReady);
changeApi.current().review(ReviewInput.approve());
ChangeInfo changeNotReadyAfterSelfApproval = changeApi.get();
assertThat(changeNotReadyAfterSelfApproval.submittable).isFalse();
- assertThat(changeNotReadyAfterSelfApproval.requirements).containsExactly(NOT_READY);
+ verifyChangeNotReady(changeNotReadyAfterSelfApproval);
requestScopeOperations.setApiUser(admin2.id());
forChange(r).current().review(ReviewInput.approve());
ChangeInfo changeReady = forChange(r).get();
assertThat(changeReady.submittable).isTrue();
- assertThat(changeReady.requirements).containsExactly(READY);
+ verifyChangeReady(changeReady);
}
@Test
- @GlobalPluginConfig(
- pluginName = "owners",
- name = "owners.enableSubmitRequirement",
- value = "true")
public void shouldBlockOwnersApprovalForMaxNegativeVote() throws Exception {
TestAccount admin2 = accountCreator.admin2();
addOwnerFileToRoot(true, admin2);
@@ -149,23 +126,19 @@
ChangeApi changeApi = forChange(r);
ChangeInfo changeNotReady = changeApi.get();
assertThat(changeNotReady.submittable).isFalse();
- assertThat(changeNotReady.requirements).containsExactly(NOT_READY);
+ verifyChangeNotReady(changeNotReady);
requestScopeOperations.setApiUser(admin2.id());
forChange(r).current().review(ReviewInput.approve());
ChangeInfo changeReady = forChange(r).get();
assertThat(changeReady.submittable).isTrue();
- assertThat(changeReady.requirements).containsExactly(READY);
+ verifyChangeReady(changeReady);
changeApi.current().review(ReviewInput.reject());
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);
@@ -175,12 +148,12 @@
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);
+ verifyChangeNotReady(changeApi.get());
requestScopeOperations.setApiUser(admin2.id());
forChange(r).current().review(ReviewInput.approve());
assertThat(forChange(r).get().submittable).isFalse();
- assertThat(forChange(r).get().requirements).containsExactly(READY);
+ verifyChangeReady(forChange(r).get());
verifyHasSubmitRecord(
forChange(r).get().submitRecords, LabelId.VERIFIED, SubmitRecordInfo.Label.Status.NEED);
@@ -191,10 +164,6 @@
}
@Test
- @GlobalPluginConfig(
- pluginName = "owners",
- name = "owners.enableSubmitRequirement",
- value = "true")
public void shouldRequireCodeOwnerApprovalEvenIfVerifiedWasApproved() throws Exception {
TestAccount admin2 = accountCreator.admin2();
addOwnerFileToRoot(true, admin2);
@@ -204,27 +173,23 @@
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);
+ verifyChangeNotReady(changeApi.get());
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);
+ verifyChangeNotReady(changeNotReady);
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);
+ verifyChangeReady(changeReady);
}
@Test
- @GlobalPluginConfig(
- pluginName = "owners",
- name = "owners.enableSubmitRequirement",
- value = "true")
public void shouldRequireConfiguredLabelByCodeOwner() throws Exception {
TestAccount admin2 = accountCreator.admin2();
String labelId = "Foo";
@@ -235,26 +200,22 @@
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);
+ verifyChangeNotReady(changeApi.get());
changeApi.current().review(ReviewInput.approve());
ChangeInfo changeStillNotReady = changeApi.get();
assertThat(changeStillNotReady.submittable).isFalse();
- assertThat(changeStillNotReady.requirements).containsExactly(NOT_READY);
+ verifyChangeNotReady(changeStillNotReady);
requestScopeOperations.setApiUser(admin2.id());
forChange(r).current().review(new ReviewInput().label(labelId, 1));
ChangeInfo changeReady = forChange(r).get();
assertThat(changeReady.submittable).isTrue();
- assertThat(changeReady.requirements).containsExactly(READY);
+ verifyChangeReady(changeReady);
verifyHasSubmitRecord(changeReady.submitRecords, labelId, SubmitRecordInfo.Label.Status.OK);
}
@Test
- @GlobalPluginConfig(
- pluginName = "owners",
- name = "owners.enableSubmitRequirement",
- value = "true")
public void shouldRequireConfiguredLabelByCodeOwnerEvenIfItIsNotConfiguredForProject()
throws Exception {
TestAccount admin2 = accountCreator.admin2();
@@ -264,19 +225,15 @@
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);
+ verifyChangeNotReady(changeApi.get());
changeApi.current().review(ReviewInput.approve());
ChangeInfo changeStillNotReady = changeApi.get();
assertThat(changeStillNotReady.submittable).isFalse();
- assertThat(changeStillNotReady.requirements).containsExactly(NOT_READY);
+ verifyChangeNotReady(changeStillNotReady);
}
@Test
- @GlobalPluginConfig(
- pluginName = "owners",
- name = "owners.enableSubmitRequirement",
- value = "true")
public void shouldRequireConfiguredLabelScoreByCodeOwner() throws Exception {
TestAccount admin2 = accountCreator.admin2();
addOwnerFileToRoot(true, LabelDefinition.parse("Code-Review,1").get(), admin2);
@@ -285,25 +242,21 @@
ChangeApi changeApi = forChange(r);
ChangeInfo changeNotReady = changeApi.get();
assertThat(changeNotReady.submittable).isFalse();
- assertThat(changeNotReady.requirements).containsExactly(NOT_READY);
+ verifyChangeNotReady(changeNotReady);
changeApi.current().review(ReviewInput.approve());
ChangeInfo changeNotReadyAfterSelfApproval = changeApi.get();
assertThat(changeNotReadyAfterSelfApproval.submittable).isFalse();
- assertThat(changeNotReadyAfterSelfApproval.requirements).containsExactly(NOT_READY);
+ verifyChangeNotReady(changeNotReadyAfterSelfApproval);
requestScopeOperations.setApiUser(admin2.id());
forChange(r).current().review(ReviewInput.recommend());
ChangeInfo changeReadyWithOwnerScore = forChange(r).get();
assertThat(changeReadyWithOwnerScore.submittable).isTrue();
- assertThat(changeReadyWithOwnerScore.requirements).containsExactly(READY);
+ verifyChangeReady(changeReadyWithOwnerScore);
}
@Test
- @GlobalPluginConfig(
- pluginName = "owners",
- name = "owners.enableSubmitRequirement",
- value = "true")
public void shouldConfiguredLabelScoreByCodeOwnerBeNotSufficientIfLabelRequiresMaxValue()
throws Exception {
TestAccount admin2 = accountCreator.admin2();
@@ -313,13 +266,13 @@
ChangeApi changeApi = forChange(r);
ChangeInfo changeNotReady = changeApi.get();
assertThat(changeNotReady.submittable).isFalse();
- assertThat(changeNotReady.requirements).containsExactly(NOT_READY);
+ verifyChangeNotReady(changeNotReady);
requestScopeOperations.setApiUser(admin2.id());
forChange(r).current().review(ReviewInput.recommend());
ChangeInfo ownersVoteNotSufficient = changeApi.get();
assertThat(ownersVoteNotSufficient.submittable).isFalse();
- assertThat(ownersVoteNotSufficient.requirements).containsExactly(READY);
+ verifyChangeReady(ownersVoteNotSufficient);
verifyHasSubmitRecord(
ownersVoteNotSufficient.submitRecords,
LabelId.CODE_REVIEW,
@@ -329,7 +282,7 @@
forChange(r).current().review(ReviewInput.approve());
ChangeInfo changeReadyWithMaxScore = forChange(r).get();
assertThat(changeReadyWithMaxScore.submittable).isTrue();
- assertThat(changeReadyWithMaxScore.requirements).containsExactly(READY);
+ verifyChangeReady(changeReadyWithMaxScore);
verifyHasSubmitRecord(
changeReadyWithMaxScore.submitRecords,
LabelId.CODE_REVIEW,
@@ -337,10 +290,6 @@
}
@Test
- @GlobalPluginConfig(
- pluginName = "owners",
- name = "owners.enableSubmitRequirement",
- value = "true")
public void shouldConfiguredLabelScoreByCodeOwnersOverwriteSubmitRequirement() throws Exception {
installLabel(TestLabels.codeReview().toBuilder().setFunction(LabelFunction.NO_OP).build());
@@ -351,25 +300,22 @@
ChangeApi changeApi = forChange(r);
ChangeInfo changeNotReady = changeApi.get();
assertThat(changeNotReady.submittable).isFalse();
- assertThat(changeNotReady.requirements).containsExactly(NOT_READY);
+ verifyChangeNotReady(changeNotReady);
requestScopeOperations.setApiUser(admin2.id());
forChange(r).current().review(ReviewInput.recommend());
ChangeInfo ownersVoteSufficient = forChange(r).get();
assertThat(ownersVoteSufficient.submittable).isTrue();
- assertThat(ownersVoteSufficient.requirements).containsExactly(READY);
+ verifyChangeReady(ownersVoteSufficient);
}
@Test
- @GlobalPluginConfig(
- pluginName = "owners",
- name = "owners.enableSubmitRequirement",
- value = "true")
public void shouldRequireApprovalFromGrandParentProjectOwner() throws Exception {
Project.NameKey parentProjectName =
createProjectOverAPI("parent", allProjects, true, SubmitType.FAST_FORWARD_ONLY);
Project.NameKey childProjectName =
createProjectOverAPI("child", parentProjectName, true, SubmitType.FAST_FORWARD_ONLY);
+ updateChildProjectConfiguration(childProjectName);
TestRepository<InMemoryRepository> childRepo = cloneProject(childProjectName);
TestAccount admin2 = accountCreator.admin2();
@@ -380,25 +326,21 @@
ChangeApi changeApi = forChange(r);
ChangeInfo changeNotReady = changeApi.get();
assertThat(changeNotReady.submittable).isFalse();
- assertThat(changeNotReady.requirements).containsExactly(NOT_READY);
+ verifyChangeNotReady(changeNotReady);
changeApi.current().review(ReviewInput.approve());
ChangeInfo changeNotReadyAfterSelfApproval = changeApi.get();
assertThat(changeNotReadyAfterSelfApproval.submittable).isFalse();
- assertThat(changeNotReadyAfterSelfApproval.requirements).containsExactly(NOT_READY);
+ verifyChangeNotReady(changeNotReadyAfterSelfApproval);
requestScopeOperations.setApiUser(admin2.id());
forChange(r).current().review(ReviewInput.approve());
ChangeInfo changeReady = forChange(r).get();
assertThat(changeReady.submittable).isTrue();
- assertThat(changeReady.requirements).containsExactly(READY);
+ verifyChangeReady(changeReady);
}
@Test
- @GlobalPluginConfig(
- pluginName = "owners",
- name = "owners.enableSubmitRequirement",
- value = "true")
public void shouldIndicateRuleErrorForBrokenOwnersFile() throws Exception {
addBrokenOwnersFileToRoot();
@@ -407,17 +349,17 @@
ChangeInfo changeNotReady = changeApi.get();
assertThat(changeNotReady.submittable).isFalse();
assertThat(changeNotReady.requirements).isEmpty();
- verifyHasSubmitRecordWithRuleError(changeNotReady.submitRecords);
+ verifyRuleError(changeNotReady);
}
- private void verifyHasSubmitRecordWithRuleError(Collection<SubmitRecordInfo> records) {
- assertThat(
- records.stream()
- .filter(record -> SubmitRecordInfo.Status.RULE_ERROR == record.status)
- .filter(record -> record.errorMessage.startsWith("Invalid owners file: OWNERS"))
- .findAny())
- .isPresent();
- }
+ protected abstract void verifyChangeNotReady(ChangeInfo notReady);
+
+ protected abstract void verifyChangeReady(ChangeInfo ready);
+
+ protected abstract void verifyRuleError(ChangeInfo change);
+
+ protected abstract void updateChildProjectConfiguration(Project.NameKey childProject)
+ throws Exception;
private void verifyHasSubmitRecord(
Collection<SubmitRecordInfo> records, String label, SubmitRecordInfo.Label.Status status) {
