AbstractRequiredApprovalConfigTest: Improve readability of 2 tests
1. testGetFromGlobalPluginConfig:
This method in the base class verified that "Code-Review+2" was
configured, but this configuration was done in the child classes via an
annotation. Due to this just looking at the
testGetFromGlobalPluginConfig() method it's unclear how the assertions
can work. To understand the assertions one must look at the calling
code. This is bad. Inline the method to improve the test readability,
even if it means that a small amount of code is being duplicated. After
doing this we can now use a more realistic label for the
getFromGlobalPluginConfig() test in OverrideApprovalConfigTest.
2. testCannotGetFromGlobalPluginConfigIfConfigIsInvalid:
Again the assertion only makes sense if you know the calling code.
Provide the invalid value as input, so that the assertion become more
readable.
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: I22a639cb878787fff8134ec4f3fcf397d21a1a20
diff --git a/javatests/com/google/gerrit/plugins/codeowners/config/AbstractRequiredApprovalConfigTest.java b/javatests/com/google/gerrit/plugins/codeowners/config/AbstractRequiredApprovalConfigTest.java
index acb5c9f..c9e6b36 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/config/AbstractRequiredApprovalConfigTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/config/AbstractRequiredApprovalConfigTest.java
@@ -34,16 +34,8 @@
/** Must return the {@link AbstractRequiredApprovalConfig} that should be tested. */
protected abstract AbstractRequiredApprovalConfig getRequiredApprovalConfig();
- protected void testGetFromGlobalPluginConfig() throws Exception {
- ProjectState projectState = projectCache.get(project).orElseThrow(illegalState(project));
- Optional<RequiredApproval> requiredApproval =
- getRequiredApprovalConfig().getFromGlobalPluginConfig(projectState);
- assertThat(requiredApproval).isPresent();
- assertThat(requiredApproval.get().labelType().getName()).isEqualTo("Code-Review");
- assertThat(requiredApproval.get().value()).isEqualTo(2);
- }
-
- protected void testCannotGetFromGlobalPluginConfigIfConfigIsInvalid() throws Exception {
+ protected void testCannotGetFromGlobalPluginConfigIfConfigIsInvalid(String invalidValue)
+ throws Exception {
ProjectState projectState = projectCache.get(project).orElseThrow(illegalState(project));
InvalidPluginConfigurationException exception =
assertThrows(
@@ -53,10 +45,10 @@
.hasMessageThat()
.isEqualTo(
String.format(
- "Invalid configuration of the code-owners plugin. Required approval 'INVALID' that is"
+ "Invalid configuration of the code-owners plugin. Required approval '%s' that is"
+ " configured in gerrit.config (parameter plugin.code-owners.%s) is"
+ " invalid: Invalid format, expected '<label-name>+<label-value>'.",
- getRequiredApprovalConfig().getConfigKey()));
+ invalidValue, getRequiredApprovalConfig().getConfigKey()));
}
@Test
diff --git a/javatests/com/google/gerrit/plugins/codeowners/config/OverrideApprovalConfigTest.java b/javatests/com/google/gerrit/plugins/codeowners/config/OverrideApprovalConfigTest.java
index 343ee30..3591eca 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/config/OverrideApprovalConfigTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/config/OverrideApprovalConfigTest.java
@@ -14,7 +14,13 @@
package com.google.gerrit.plugins.codeowners.config;
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.server.project.ProjectCache.illegalState;
+import static com.google.gerrit.truth.OptionalSubject.assertThat;
+
import com.google.gerrit.acceptance.config.GerritConfig;
+import com.google.gerrit.server.project.ProjectState;
+import java.util.Optional;
import org.junit.Before;
import org.junit.Test;
@@ -33,14 +39,21 @@
}
@Test
- @GerritConfig(name = "plugin.code-owners.overrideApproval", value = "Code-Review+2")
+ @GerritConfig(name = "plugin.code-owners.overrideApproval", value = "Owners-Override+1")
public void getFromGlobalPluginConfig() throws Exception {
- testGetFromGlobalPluginConfig();
+ createOwnersOverrideLabel();
+
+ ProjectState projectState = projectCache.get(project).orElseThrow(illegalState(project));
+ Optional<RequiredApproval> requiredApproval =
+ getRequiredApprovalConfig().getFromGlobalPluginConfig(projectState);
+ assertThat(requiredApproval).isPresent();
+ assertThat(requiredApproval.get().labelType().getName()).isEqualTo("Owners-Override");
+ assertThat(requiredApproval.get().value()).isEqualTo(1);
}
@Test
@GerritConfig(name = "plugin.code-owners.overrideApproval", value = "INVALID")
public void cannotGetFromGlobalPluginConfigIfConfigIsInvalid() throws Exception {
- testCannotGetFromGlobalPluginConfigIfConfigIsInvalid();
+ testCannotGetFromGlobalPluginConfigIfConfigIsInvalid("INVALID");
}
}
diff --git a/javatests/com/google/gerrit/plugins/codeowners/config/RequiredApprovalConfigTest.java b/javatests/com/google/gerrit/plugins/codeowners/config/RequiredApprovalConfigTest.java
index 53c0fa2..198ac0c 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/config/RequiredApprovalConfigTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/config/RequiredApprovalConfigTest.java
@@ -17,9 +17,11 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.server.project.ProjectCache.illegalState;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+import static com.google.gerrit.truth.OptionalSubject.assertThat;
import com.google.gerrit.acceptance.config.GerritConfig;
import com.google.gerrit.server.project.ProjectState;
+import java.util.Optional;
import org.junit.Before;
import org.junit.Test;
@@ -40,13 +42,18 @@
@Test
@GerritConfig(name = "plugin.code-owners.requiredApproval", value = "Code-Review+2")
public void getFromGlobalPluginConfig() throws Exception {
- testGetFromGlobalPluginConfig();
+ ProjectState projectState = projectCache.get(project).orElseThrow(illegalState(project));
+ Optional<RequiredApproval> requiredApproval =
+ getRequiredApprovalConfig().getFromGlobalPluginConfig(projectState);
+ assertThat(requiredApproval).isPresent();
+ assertThat(requiredApproval.get().labelType().getName()).isEqualTo("Code-Review");
+ assertThat(requiredApproval.get().value()).isEqualTo(2);
}
@Test
@GerritConfig(name = "plugin.code-owners.requiredApproval", value = "INVALID")
public void cannotGetFromGlobalPluginConfigIfConfigIsInvalid() throws Exception {
- testCannotGetFromGlobalPluginConfigIfConfigIsInvalid();
+ testCannotGetFromGlobalPluginConfigIfConfigIsInvalid("INVALID");
}
@Test
