Split getChecker/getCheck* tests into one test per field
Having seperate tests for each field in the info entities is cleaner:
- test failures can be directly attributed to a certain field
- for the creation of the test object we only need to care about the
field under test, just setting this one field improves readability
- some fields require multiple tests (e.g. with/without description),
the new structure makes these tests more consistent and it's more
obvious if they are missing
Change-Id: I9e653ddd2d243455fef4a4a3a7572cf700ce7260
Signed-off-by: Edwin Kempin <ekempin@google.com>
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckIT.java
index 97abf39..14ae012 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckIT.java
@@ -19,7 +19,6 @@
import static com.google.common.truth.Truth.assert_;
import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_REVISION;
-import com.google.common.collect.ImmutableSet;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
@@ -36,6 +35,7 @@
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Project;
+import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import java.sql.Timestamp;
import org.junit.Before;
@@ -52,51 +52,146 @@
}
@Test
- public void getCheck() throws Exception {
+ public void getCheckReturnsProject() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+
+ CheckKey checkKey = CheckKey.create(project, patchSetId, checkerUuid);
+ checkOperations.newCheck(checkKey).upsert();
+
+ assertThat(getCheckInfo(patchSetId, checkerUuid).project).isEqualTo(project.get());
+ assertThat(getCheckInfo(patchSetId, checkerUuid, ListChecksOption.CHECKER).project)
+ .isEqualTo(project.get());
+ }
+
+ @Test
+ public void getCheckReturnsChangeNumber() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+
+ CheckKey checkKey = CheckKey.create(project, patchSetId, checkerUuid);
+ checkOperations.newCheck(checkKey).upsert();
+
+ assertThat(getCheckInfo(patchSetId, checkerUuid).changeNumber)
+ .isEqualTo(patchSetId.getParentKey().get());
+ assertThat(getCheckInfo(patchSetId, checkerUuid, ListChecksOption.CHECKER).changeNumber)
+ .isEqualTo(patchSetId.getParentKey().get());
+ }
+
+ @Test
+ public void getCheckReturnsPatchSetId() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+
+ CheckKey checkKey = CheckKey.create(project, patchSetId, checkerUuid);
+ checkOperations.newCheck(checkKey).upsert();
+
+ assertThat(getCheckInfo(patchSetId, checkerUuid).patchSetId).isEqualTo(patchSetId.get());
+ assertThat(getCheckInfo(patchSetId, checkerUuid, ListChecksOption.CHECKER).patchSetId)
+ .isEqualTo(patchSetId.get());
+ }
+
+ @Test
+ public void getCheckReturnsCheckerUuid() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+
+ CheckKey checkKey = CheckKey.create(project, patchSetId, checkerUuid);
+ checkOperations.newCheck(checkKey).upsert();
+
+ assertThat(getCheckInfo(patchSetId, checkerUuid).checkerUuid).isEqualTo(checkerUuid.toString());
+ assertThat(getCheckInfo(patchSetId, checkerUuid, ListChecksOption.CHECKER).checkerUuid)
+ .isEqualTo(checkerUuid.toString());
+ }
+
+ @Test
+ public void getCheckReturnsState() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
CheckKey checkKey = CheckKey.create(project, patchSetId, checkerUuid);
checkOperations.newCheck(checkKey).setState(CheckState.RUNNING).upsert();
- CheckInfo checkInfo = checksApiFactory.revision(patchSetId).id(checkerUuid).get();
- CheckInfo expected = new CheckInfo();
- expected.project = checkKey.project().get();
- expected.changeNumber = checkKey.patchSet().getParentKey().get();
- expected.patchSetId = checkKey.patchSet().get();
- expected.checkerUuid = checkKey.checkerUuid().toString();
- expected.state = CheckState.RUNNING;
- expected.created = checkOperations.check(checkKey).get().created();
- expected.updated = expected.created;
- assertThat(checkInfo).isEqualTo(expected);
+ assertThat(getCheckInfo(patchSetId, checkerUuid).state).isEqualTo(CheckState.RUNNING);
+ assertThat(getCheckInfo(patchSetId, checkerUuid, ListChecksOption.CHECKER).state)
+ .isEqualTo(CheckState.RUNNING);
}
@Test
- public void getCheckWithOptions() throws Exception {
+ public void getCheckReturnsStateNotStartedIfStateNotSet() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+
+ CheckKey checkKey = CheckKey.create(project, patchSetId, checkerUuid);
+ checkOperations.newCheck(checkKey).upsert();
+
+ assertThat(getCheckInfo(patchSetId, checkerUuid).state).isEqualTo(CheckState.NOT_STARTED);
+ assertThat(getCheckInfo(patchSetId, checkerUuid, ListChecksOption.CHECKER).state)
+ .isEqualTo(CheckState.NOT_STARTED);
+ }
+
+ @Test
+ public void getCheckReturnsCreationTimestamp() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+
+ CheckKey checkKey = CheckKey.create(project, patchSetId, checkerUuid);
+ checkOperations.newCheck(checkKey).upsert();
+
+ Timestamp expectedCreationTimestamp = checkOperations.check(checkKey).get().created();
+
+ assertThat(getCheckInfo(patchSetId, checkerUuid).created).isEqualTo(expectedCreationTimestamp);
+ assertThat(getCheckInfo(patchSetId, checkerUuid, ListChecksOption.CHECKER).created)
+ .isEqualTo(expectedCreationTimestamp);
+ }
+
+ @Test
+ public void getCheckReturnsUpdatedTimestamp() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+
+ CheckKey checkKey = CheckKey.create(project, patchSetId, checkerUuid);
+ checkOperations.newCheck(checkKey).upsert();
+
+ Timestamp expectedUpdatedTimestamp = checkOperations.check(checkKey).get().updated();
+
+ assertThat(getCheckInfo(patchSetId, checkerUuid).created).isEqualTo(expectedUpdatedTimestamp);
+ assertThat(getCheckInfo(patchSetId, checkerUuid, ListChecksOption.CHECKER).created)
+ .isEqualTo(expectedUpdatedTimestamp);
+ }
+
+ @Test
+ public void getCheckReturnsCheckerNameOnlyForCheckerOption() throws Exception {
+ CheckerUuid checkerUuid =
+ checkerOperations.newChecker().repository(project).name("My Checker").create();
+
+ CheckKey checkKey = CheckKey.create(project, patchSetId, checkerUuid);
+ checkOperations.newCheck(checkKey).upsert();
+
+ assertThat(getCheckInfo(patchSetId, checkerUuid).checkerName).isNull();
+ assertThat(getCheckInfo(patchSetId, checkerUuid, ListChecksOption.CHECKER).checkerName)
+ .isEqualTo("My Checker");
+ }
+
+ @Test
+ public void getCheckReturnsCheckerStatusOnlyForCheckerOption() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+
+ CheckKey checkKey = CheckKey.create(project, patchSetId, checkerUuid);
+ checkOperations.newCheck(checkKey).upsert();
+
+ assertThat(getCheckInfo(patchSetId, checkerUuid).checkerStatus).isNull();
+ assertThat(getCheckInfo(patchSetId, checkerUuid, ListChecksOption.CHECKER).checkerStatus)
+ .isEqualTo(CheckerStatus.ENABLED);
+ }
+
+ @Test
+ public void getCheckReturnsBlockingConditionsOnlyForCheckerOption() throws Exception {
CheckerUuid checkerUuid =
checkerOperations
.newChecker()
- .name("My Checker")
- .blockingConditions(BlockingCondition.STATE_NOT_PASSING)
.repository(project)
+ .blockingConditions(BlockingCondition.STATE_NOT_PASSING)
.create();
CheckKey checkKey = CheckKey.create(project, patchSetId, checkerUuid);
- checkOperations.newCheck(checkKey).setState(CheckState.RUNNING).upsert();
+ checkOperations.newCheck(checkKey).upsert();
- CheckInfo checkInfo =
- checksApiFactory.revision(patchSetId).id(checkerUuid).get(ListChecksOption.CHECKER);
- CheckInfo expected = new CheckInfo();
- expected.project = checkKey.project().get();
- expected.changeNumber = checkKey.patchSet().getParentKey().get();
- expected.patchSetId = checkKey.patchSet().get();
- expected.checkerUuid = checkKey.checkerUuid().toString();
- expected.state = CheckState.RUNNING;
- expected.created = checkOperations.check(checkKey).get().created();
- expected.updated = expected.created;
- expected.checkerName = "My Checker";
- expected.blocking = ImmutableSet.of(BlockingCondition.STATE_NOT_PASSING);
- expected.checkerStatus = CheckerStatus.ENABLED;
- assertThat(checkInfo).isEqualTo(expected);
+ assertThat(getCheckInfo(patchSetId, checkerUuid).blocking).isNull();
+ assertThat(getCheckInfo(patchSetId, checkerUuid, ListChecksOption.CHECKER).blocking)
+ .containsExactly(BlockingCondition.STATE_NOT_PASSING);
}
@Test
@@ -107,8 +202,8 @@
CheckKey checkKey = CheckKey.create(project, patchSetId, checkerUuid);
checkOperations.newCheck(checkKey).setState(CheckState.RUNNING).upsert();
- CheckInfo checkInfo = checksApiFactory.revision(patchSetId).id(checkerUuid).get();
- assertThat(checkInfo).isEqualTo(checkOperations.check(checkKey).asInfo());
+ assertThat(getCheckInfo(patchSetId, checkerUuid))
+ .isEqualTo(checkOperations.check(checkKey).asInfo());
}
@Test
@@ -119,8 +214,8 @@
CheckKey checkKey = CheckKey.create(project, patchSetId, checkerUuid);
checkOperations.newCheck(checkKey).setState(CheckState.RUNNING).upsert();
- CheckInfo checkInfo = checksApiFactory.revision(patchSetId).id(checkerUuid).get();
- assertThat(checkInfo).isEqualTo(checkOperations.check(checkKey).asInfo());
+ assertThat(getCheckInfo(patchSetId, checkerUuid))
+ .isEqualTo(checkOperations.check(checkKey).asInfo());
}
@Test
@@ -164,8 +259,8 @@
checkerOperations.checker(checkerUuid).forUpdate().disable().update();
- CheckInfo checkInfo = checksApiFactory.revision(patchSetId).id(checkerUuid).get();
- assertThat(checkInfo).isEqualTo(checkOperations.check(checkKey).asInfo());
+ assertThat(getCheckInfo(patchSetId, checkerUuid))
+ .isEqualTo(checkOperations.check(checkKey).asInfo());
}
@Test
@@ -176,8 +271,8 @@
checkerOperations.checker(checkerUuid).forUpdate().forceInvalidConfig().update();
- CheckInfo checkInfo = checksApiFactory.revision(patchSetId).id(checkerUuid).get();
- assertThat(checkInfo).isEqualTo(checkOperations.check(checkKey).asInfo());
+ assertThat(getCheckInfo(patchSetId, checkerUuid))
+ .isEqualTo(checkOperations.check(checkKey).asInfo());
}
@Test
@@ -187,8 +282,8 @@
checkOperations.newCheck(checkKey).setState(CheckState.RUNNING).upsert();
checkerOperations.checker(checkerUuid).forUpdate().deleteRef().update();
- CheckInfo checkInfo = checksApiFactory.revision(patchSetId).id(checkerUuid).get();
- assertThat(checkInfo).isEqualTo(checkOperations.check(checkKey).asInfo());
+ assertThat(getCheckInfo(patchSetId, checkerUuid))
+ .isEqualTo(checkOperations.check(checkKey).asInfo());
}
@Test
@@ -212,7 +307,7 @@
expected.state = CheckState.NOT_STARTED;
expected.created = psCreated;
expected.updated = psCreated;
- assertThat(checksApiFactory.revision(patchSetId).id(checkerUuid).get()).isEqualTo(expected);
+ assertThat(getCheckInfo(patchSetId, checkerUuid)).isEqualTo(expected);
}
@Test
@@ -230,7 +325,7 @@
expected.state = CheckState.NOT_STARTED;
expected.created = psCreated;
expected.updated = psCreated;
- assertThat(checksApiFactory.revision(patchSetId).id(checkerUuid).get()).isEqualTo(expected);
+ assertThat(getCheckInfo(patchSetId, checkerUuid)).isEqualTo(expected);
checkerOperations.checker(checkerUuid).forUpdate().disable().update();
@@ -253,8 +348,8 @@
CheckKey checkKey = CheckKey.create(project, patchSetId, checkerUuid);
checkOperations.newCheck(checkKey).setState(CheckState.RUNNING).upsert();
- CheckInfo checkInfo = checksApiFactory.revision(patchSetId).id(checkerUuid).get();
- assertThat(checkInfo).isEqualTo(checkOperations.check(checkKey).asInfo());
+ assertThat(getCheckInfo(patchSetId, checkerUuid))
+ .isEqualTo(checkOperations.check(checkKey).asInfo());
}
@Test
@@ -266,7 +361,7 @@
gApi.changes().id(patchSetId.getParentKey().get()).delete();
try {
- checksApiFactory.revision(patchSetId).id(checkerUuid).get();
+ getCheckInfo(patchSetId, checkerUuid);
assert_().fail("expected ResourceNotFoundException");
} catch (ResourceNotFoundException e) {
assertThat(e)
@@ -276,6 +371,12 @@
}
}
+ private CheckInfo getCheckInfo(
+ PatchSet.Id patchSetId, CheckerUuid checkerUuid, ListChecksOption... options)
+ throws RestApiException, OrmException {
+ return checksApiFactory.revision(patchSetId).id(checkerUuid).get(options);
+ }
+
private Timestamp getPatchSetCreated(Change.Id changeId) throws RestApiException {
return getOnlyElement(
gApi.changes().id(changeId.get()).get(CURRENT_REVISION).revisions.values())
@@ -285,7 +386,7 @@
private void assertCheckNotFound(PatchSet.Id patchSetId, CheckerUuid checkerUuid)
throws Exception {
try {
- checksApiFactory.revision(patchSetId).id(checkerUuid);
+ getCheckInfo(patchSetId, checkerUuid);
assert_().fail("expected ResourceNotFoundException");
} catch (ResourceNotFoundException e) {
assertThat(e)
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckerIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckerIT.java
index 8c7d7b4..a450e41 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckerIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckerIT.java
@@ -16,6 +16,7 @@
import static com.google.common.truth.Truth.assertThat;
+import com.google.common.collect.ImmutableSortedSet;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
@@ -30,6 +31,7 @@
import com.google.gerrit.server.restapi.config.ListCapabilities;
import com.google.gerrit.server.restapi.config.ListCapabilities.CapabilityInfo;
import com.google.inject.Inject;
+import java.sql.Timestamp;
import java.util.Map;
import org.junit.Test;
@@ -38,79 +40,121 @@
@Inject private ListCapabilities listCapabilities;
@Test
- public void getChecker() throws Exception {
- String name = "my-checker";
- CheckerUuid checkerUuid =
- checkerOperations.newChecker().name(name).repository(project).query("").create();
-
- CheckerInfo info = checkersApi.id(checkerUuid).get();
- assertThat(info.uuid).isEqualTo(checkerUuid.toString());
- assertThat(info.name).isEqualTo(name);
- assertThat(info.description).isNull();
- assertThat(info.url).isNull();
- assertThat(info.repository).isEqualTo(project.get());
- assertThat(info.status).isEqualTo(CheckerStatus.ENABLED);
- assertThat(info.blocking).isEmpty();
- assertThat(info.query).isNull();
- assertThat(info.createdOn).isNotNull();
- assertThat(info.updatedOn).isEqualTo(info.createdOn);
+ public void getCheckerReturnsUuid() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().create();
+ assertThat(getCheckerInfo(checkerUuid).uuid).isEqualTo(checkerUuid.toString());
}
@Test
- public void getCheckerWithDescription() throws Exception {
+ public void getCheckerReturnsName() throws Exception {
+ String name = "My Checker";
+ CheckerUuid checkerUuid = checkerOperations.newChecker().name(name).create();
+
+ assertThat(getCheckerInfo(checkerUuid).name).isEqualTo(name);
+ }
+
+ @Test
+ public void getCheckerReturnsDescription() throws Exception {
String description = "some description";
CheckerUuid checkerUuid = checkerOperations.newChecker().description(description).create();
- CheckerInfo info = checkersApi.id(checkerUuid).get();
- assertThat(info.description).isEqualTo(description);
+ assertThat(getCheckerInfo(checkerUuid).description).isEqualTo(description);
}
@Test
- public void getCheckerWithUrl() throws Exception {
+ public void getCheckerWithoutDescription() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().clearDescription().create();
+ assertThat(getCheckerInfo(checkerUuid).description).isNull();
+ }
+
+ @Test
+ public void getCheckerReturnsUrl() throws Exception {
String url = "http://example.com/my-checker";
CheckerUuid checkerUuid = checkerOperations.newChecker().url(url).create();
- CheckerInfo info = checkersApi.id(checkerUuid).get();
- assertThat(info.url).isEqualTo(url);
+ assertThat(getCheckerInfo(checkerUuid).url).isEqualTo(url);
+ }
+
+ @Test
+ public void getCheckerWithoutUrl() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().clearUrl().create();
+ assertThat(getCheckerInfo(checkerUuid).url).isNull();
}
@Test
public void getCheckerWithInvalidUrl() throws Exception {
CheckerUuid checkerUuid =
checkerOperations.newChecker().url(CheckerTestData.INVALID_URL).create();
+ assertThat(getCheckerInfo(checkerUuid).url).isEqualTo(CheckerTestData.INVALID_URL);
+ }
- CheckerInfo info = checkersApi.id(checkerUuid).get();
- assertThat(info.url).isEqualTo(CheckerTestData.INVALID_URL);
+ @Test
+ public void getCheckerReturnsRepository() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+ assertThat(getCheckerInfo(checkerUuid).repository).isEqualTo(project.get());
+ }
+
+ @Test
+ public void getCheckerReturnsStatus() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().status(CheckerStatus.ENABLED).create();
+ assertThat(getCheckerInfo(checkerUuid).status).isEqualTo(CheckerStatus.ENABLED);
}
@Test
public void getDisabledChecker() throws Exception {
CheckerUuid checkerUuid =
checkerOperations.newChecker().status(CheckerStatus.DISABLED).create();
-
- CheckerInfo info = checkersApi.id(checkerUuid).get();
- assertThat(info.status).isEqualTo(CheckerStatus.DISABLED);
+ assertThat(getCheckerInfo(checkerUuid).status).isEqualTo(CheckerStatus.DISABLED);
}
@Test
- public void getCheckerWithBlockingCondition() throws Exception {
+ public void getCheckerReturnsBlockingCondition() throws Exception {
CheckerUuid checkerUuid =
checkerOperations
.newChecker()
.blockingConditions(BlockingCondition.STATE_NOT_PASSING)
.create();
-
- CheckerInfo info = checkersApi.id(checkerUuid).get();
- assertThat(info.blocking).containsExactly(BlockingCondition.STATE_NOT_PASSING);
+ assertThat(getCheckerInfo(checkerUuid).blocking)
+ .containsExactly(BlockingCondition.STATE_NOT_PASSING);
}
@Test
- public void getCheckerWithQuery() throws Exception {
+ public void getCheckerWithoutBlockingCondition() throws Exception {
+ CheckerUuid checkerUuid =
+ checkerOperations.newChecker().blockingConditions(ImmutableSortedSet.of()).create();
+ assertThat(getCheckerInfo(checkerUuid).blocking).isEmpty();
+ }
+
+ @Test
+ public void getCheckerReturnsQuery() throws Exception {
String query = "message:foo footer:bar";
CheckerUuid checkerUuid = checkerOperations.newChecker().query(query).create();
- CheckerInfo info = checkersApi.id(checkerUuid).get();
- assertThat(info.query).isEqualTo(query);
+ assertThat(getCheckerInfo(checkerUuid).query).isEqualTo(query);
+ }
+
+ @Test
+ public void getCheckerWithoutQuery() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().query("").create();
+ assertThat(getCheckerInfo(checkerUuid).query).isNull();
+ }
+
+ @Test
+ public void getCheckerReturnsCreationTimestamp() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().create();
+
+ Timestamp expectedCreationTimestamp =
+ checkerOperations.checker(checkerUuid).get().getCreatedOn();
+ assertThat(getCheckerInfo(checkerUuid).createdOn).isEqualTo(expectedCreationTimestamp);
+ }
+
+ @Test
+ public void getCheckerReturnsUpdatedTimestamp() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().create();
+
+ Timestamp expectedUpdatedTimestamp =
+ checkerOperations.checker(checkerUuid).get().getUpdatedOn();
+ assertThat(getCheckerInfo(checkerUuid).updatedOn).isEqualTo(expectedUpdatedTimestamp);
}
@Test
@@ -140,7 +184,7 @@
exception.expect(ResourceNotFoundException.class);
exception.expectMessage("Not found: " + checkerUuid);
- checkersApi.id(checkerUuid);
+ getCheckerInfo(checkerUuid);
}
@Test
@@ -150,7 +194,7 @@
exception.expect(RestApiException.class);
exception.expectMessage("Cannot retrieve checker " + checkerUuid);
- checkersApi.id(checkerUuid);
+ getCheckerInfo(checkerUuid);
}
@Test
@@ -172,7 +216,7 @@
exception.expect(AuthException.class);
exception.expectMessage("administrateCheckers for plugin checks not permitted");
- checkersApi.id(checkerUuid);
+ getCheckerInfo(checkerUuid);
}
@Test
@@ -184,4 +228,8 @@
assertThat(info.id).isEqualTo(capability);
assertThat(info.name).isEqualTo("Administrate Checkers");
}
+
+ private CheckerInfo getCheckerInfo(CheckerUuid checkerUuid) throws RestApiException {
+ return checkersApi.id(checkerUuid).get();
+ }
}
