Merge "Checks plugin table fixes"
diff --git a/java/com/google/gerrit/plugins/checks/acceptance/AbstractCheckersTest.java b/java/com/google/gerrit/plugins/checks/acceptance/AbstractCheckersTest.java
index de02574..e476537 100644
--- a/java/com/google/gerrit/plugins/checks/acceptance/AbstractCheckersTest.java
+++ b/java/com/google/gerrit/plugins/checks/acceptance/AbstractCheckersTest.java
@@ -14,15 +14,19 @@
package com.google.gerrit.plugins.checks.acceptance;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allowCapability;
+
import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
import com.google.gerrit.acceptance.ProjectResetter;
import com.google.gerrit.acceptance.TestPlugin;
+import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.plugins.checks.CheckerRef;
import com.google.gerrit.plugins.checks.acceptance.testsuite.CheckOperations;
import com.google.gerrit.plugins.checks.acceptance.testsuite.CheckerOperations;
import com.google.gerrit.plugins.checks.api.Checkers;
import com.google.gerrit.plugins.checks.api.ChecksFactory;
import com.google.gerrit.plugins.checks.api.PendingChecks;
+import com.google.inject.Inject;
import org.junit.Before;
// TODO(dborowitz): Improve the plugin test framework so we can avoid subclassing:
@@ -34,6 +38,8 @@
sysModule = "com.google.gerrit.plugins.checks.acceptance.TestModule",
httpModule = "com.google.gerrit.plugins.checks.HttpModule")
public class AbstractCheckersTest extends LightweightPluginDaemonTest {
+ @Inject protected ProjectOperations projectOperations;
+
protected CheckerOperations checkerOperations;
protected CheckOperations checkOperations;
protected Checkers checkersApi;
@@ -54,6 +60,12 @@
checksApiFactory = plugin.getHttpInjector().getInstance(ChecksFactory.class);
pendingChecksApi = plugin.getHttpInjector().getInstance(PendingChecks.class);
- allowGlobalCapabilities(group("Administrators").getGroupUUID(), "checks-administrateCheckers");
+ projectOperations
+ .project(allProjects)
+ .forUpdate()
+ .add(
+ allowCapability("checks-administrateCheckers")
+ .group(group("Administrators").getGroupUUID()))
+ .update();
}
}
diff --git a/java/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckOperationsImpl.java b/java/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckOperationsImpl.java
index c732548..310af7a 100644
--- a/java/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckOperationsImpl.java
+++ b/java/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckOperationsImpl.java
@@ -15,6 +15,7 @@
package com.google.gerrit.plugins.checks.acceptance.testsuite;
import static com.google.common.base.Preconditions.checkNotNull;
+import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
@@ -100,7 +101,8 @@
for (Note note : notes) {
raw.put(
- note.copy(), new String(notes.getCachedBytes(note.toObjectId(), Integer.MAX_VALUE)));
+ note.copy(),
+ new String(notes.getCachedBytes(note.toObjectId(), Integer.MAX_VALUE), UTF_8));
}
return raw.build();
}
diff --git a/java/com/google/gerrit/plugins/checks/api/ChecksCollection.java b/java/com/google/gerrit/plugins/checks/api/ChecksCollection.java
index 397d8b6..78d43e4 100644
--- a/java/com/google/gerrit/plugins/checks/api/ChecksCollection.java
+++ b/java/com/google/gerrit/plugins/checks/api/ChecksCollection.java
@@ -19,6 +19,7 @@
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ChildCollection;
import com.google.gerrit.extensions.restapi.IdString;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestReadView;
@@ -57,6 +58,10 @@
@Override
public CheckResource parse(RevisionResource parent, IdString id)
throws RestApiException, PermissionBackendException, IOException, StorageException {
+ if (parent.getEdit().isPresent()) {
+ throw new ResourceConflictException("checks are not supported on a change edit");
+ }
+
CheckerUuid checkerUuid =
CheckerUuid.tryParse(id.get())
.orElseThrow(
diff --git a/java/com/google/gerrit/plugins/checks/api/ListChecks.java b/java/com/google/gerrit/plugins/checks/api/ListChecks.java
index 6f1257d..39346e4 100644
--- a/java/com/google/gerrit/plugins/checks/api/ListChecks.java
+++ b/java/com/google/gerrit/plugins/checks/api/ListChecks.java
@@ -58,6 +58,10 @@
public ImmutableList<CheckInfo> apply(RevisionResource resource)
throws AuthException, BadRequestException, ResourceConflictException, StorageException,
IOException {
+ if (resource.getEdit().isPresent()) {
+ throw new ResourceConflictException("checks are not supported on a change edit");
+ }
+
ImmutableList.Builder<CheckInfo> result = ImmutableList.builder();
GetCheckOptions getCheckOptions = GetCheckOptions.withBackfilling();
diff --git a/java/com/google/gerrit/plugins/checks/api/PostCheck.java b/java/com/google/gerrit/plugins/checks/api/PostCheck.java
index 5de27ad..18572e9 100644
--- a/java/com/google/gerrit/plugins/checks/api/PostCheck.java
+++ b/java/com/google/gerrit/plugins/checks/api/PostCheck.java
@@ -17,6 +17,7 @@
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestCollectionModifyView;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
@@ -81,6 +82,10 @@
}
permissionBackend.currentUser().check(permission);
+ if (rsrc.getEdit().isPresent()) {
+ throw new ResourceConflictException("checks are not supported on a change edit");
+ }
+
if (input == null) {
input = new CheckInput();
}
diff --git a/java/com/google/gerrit/plugins/checks/db/CheckNotes.java b/java/com/google/gerrit/plugins/checks/db/CheckNotes.java
index 148fc82..ea58e9a 100644
--- a/java/com/google/gerrit/plugins/checks/db/CheckNotes.java
+++ b/java/com/google/gerrit/plugins/checks/db/CheckNotes.java
@@ -74,7 +74,7 @@
try (TraceTimer ignored =
TraceContext.newTimer(
- "Load check notes for change %s of project %s", getChangeId(), getProjectName())) {
+ "Load check notes", "changeId", getChangeId(), "projectName", getProjectName())) {
RevCommit tipCommit = handle.walk().parseCommit(metaId);
ObjectReader reader = handle.walk().getObjectReader();
revisionNoteMap =
diff --git a/java/com/google/gerrit/plugins/checks/db/NoteDbChecks.java b/java/com/google/gerrit/plugins/checks/db/NoteDbChecks.java
index bd3dd1e..02847e8 100644
--- a/java/com/google/gerrit/plugins/checks/db/NoteDbChecks.java
+++ b/java/com/google/gerrit/plugins/checks/db/NoteDbChecks.java
@@ -103,6 +103,10 @@
// TODO(gerrit-team): Instead of reading the complete notes map, read just one note.
ChangeData changeData = changeDataFactory.create(repositoryName, psId.changeId());
PatchSet patchSet = changeData.patchSet(psId);
+ if (patchSet == null) {
+ throw new StorageException("patch set not found: " + psId);
+ }
+
CheckNotes checkNotes = checkNotesFactory.create(changeData.change());
checkNotes.load();
diff --git a/java/com/google/gerrit/plugins/checks/testing/CheckerConfigSubject.java b/java/com/google/gerrit/plugins/checks/testing/CheckerConfigSubject.java
index 4c4dc20..e74f050 100644
--- a/java/com/google/gerrit/plugins/checks/testing/CheckerConfigSubject.java
+++ b/java/com/google/gerrit/plugins/checks/testing/CheckerConfigSubject.java
@@ -32,11 +32,10 @@
import com.google.gerrit.plugins.checks.db.CheckerConfig;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.truth.OptionalSubject;
-import java.sql.Timestamp;
import java.util.Optional;
import org.eclipse.jgit.lib.Config;
-public class CheckerConfigSubject extends Subject<CheckerConfigSubject, CheckerConfig> {
+public class CheckerConfigSubject extends Subject {
public static CheckerConfigSubject assertThat(CheckerConfig checkerConfig) {
return assertAbout(CheckerConfigSubject::new).that(checkerConfig);
}
@@ -88,7 +87,7 @@
check("query()").about(optionals()).that(checker().getQuery()).isEmpty();
}
- public ComparableSubject<?, Timestamp> hasCreatedThat() {
+ public ComparableSubject hasCreatedThat() {
return check("created()").that(checker().getCreated());
}
diff --git a/java/com/google/gerrit/plugins/checks/testing/PendingChecksInfoSubject.java b/java/com/google/gerrit/plugins/checks/testing/PendingChecksInfoSubject.java
index 6a0266d..14ada06 100644
--- a/java/com/google/gerrit/plugins/checks/testing/PendingChecksInfoSubject.java
+++ b/java/com/google/gerrit/plugins/checks/testing/PendingChecksInfoSubject.java
@@ -26,7 +26,7 @@
import com.google.gerrit.reviewdb.client.Project;
import java.util.Map;
-public class PendingChecksInfoSubject extends Subject<PendingChecksInfoSubject, PendingChecksInfo> {
+public class PendingChecksInfoSubject extends Subject {
public static PendingChecksInfoSubject assertThat(PendingChecksInfo pendingChecksInfo) {
return assertAbout(PendingChecksInfoSubject::new).that(pendingChecksInfo);
}
diff --git a/javatests/com/google/gerrit/plugins/checks/CheckerQueryTest.java b/javatests/com/google/gerrit/plugins/checks/CheckerQueryTest.java
index c3a93dd..750ef53 100644
--- a/javatests/com/google/gerrit/plugins/checks/CheckerQueryTest.java
+++ b/javatests/com/google/gerrit/plugins/checks/CheckerQueryTest.java
@@ -15,7 +15,7 @@
package com.google.gerrit.plugins.checks;
import static com.google.common.truth.Truth.assertThat;
-import static com.google.common.truth.Truth.assert_;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import com.google.gerrit.index.query.QueryParseException;
import com.google.gerrit.server.query.change.ChangeQueryBuilder;
@@ -54,12 +54,7 @@
@Test
public void disallowedUnknownOperator() throws Exception {
String query = "fhqwhgads:bar";
- try {
- newChangeQueryBuilder().parse(query);
- assert_().fail("expected QueryParseException");
- } catch (QueryParseException e) {
- // Expected.
- }
+ assertThrows(QueryParseException.class, () -> newChangeQueryBuilder().parse(query));
assertInvalidQuery(query, "Unsupported operator: fhqwhgads");
}
@@ -104,12 +99,9 @@
}
private static void assertInvalidQuery(String query, String expectedMessage) {
- try {
- CheckerQuery.clean(query);
- assert_().fail("expected ConfigInvalidException");
- } catch (ConfigInvalidException e) {
- assertThat(e).hasMessageThat().isEqualTo(expectedMessage);
- }
+ ConfigInvalidException thrown =
+ assertThrows(ConfigInvalidException.class, () -> CheckerQuery.clean(query));
+ assertThat(thrown).hasMessageThat().isEqualTo(expectedMessage);
}
private static void assertValidQuery(String query) {
diff --git a/javatests/com/google/gerrit/plugins/checks/CheckerUuidTest.java b/javatests/com/google/gerrit/plugins/checks/CheckerUuidTest.java
index 79c6394..ebbc611 100644
--- a/javatests/com/google/gerrit/plugins/checks/CheckerUuidTest.java
+++ b/javatests/com/google/gerrit/plugins/checks/CheckerUuidTest.java
@@ -17,7 +17,6 @@
import static com.google.common.truth.OptionalSubject.optionals;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
-import static com.google.common.truth.Truth8.assertThat;
import static com.google.gerrit.plugins.checks.CheckerUuid.MAX_SCHEME_LENGTH;
import static java.util.stream.Collectors.joining;
diff --git a/javatests/com/google/gerrit/plugins/checks/UrlValidatorTest.java b/javatests/com/google/gerrit/plugins/checks/UrlValidatorTest.java
index 5ada85a..63f36c2 100644
--- a/javatests/com/google/gerrit/plugins/checks/UrlValidatorTest.java
+++ b/javatests/com/google/gerrit/plugins/checks/UrlValidatorTest.java
@@ -15,7 +15,7 @@
package com.google.gerrit.plugins.checks;
import static com.google.common.truth.Truth.assertThat;
-import static com.google.common.truth.Truth.assert_;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import com.google.gerrit.extensions.restapi.BadRequestException;
import org.junit.Test;
@@ -59,11 +59,8 @@
}
private static void assertInvalidUrl(String url, String expectedMessage) {
- try {
- UrlValidator.clean(url);
- assert_().fail("expected BadRequestException");
- } catch (BadRequestException e) {
- assertThat(e).hasMessageThat().isEqualTo(expectedMessage);
- }
+ BadRequestException thrown =
+ assertThrows(BadRequestException.class, () -> UrlValidator.clean(url));
+ assertThat(thrown).hasMessageThat().isEqualTo(expectedMessage);
}
}
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/CheckerRefsIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/CheckerRefsIT.java
index 1fb36a4..e87dd42 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/CheckerRefsIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/CheckerRefsIT.java
@@ -17,6 +17,8 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.acceptance.GitUtil.deleteRef;
import static com.google.gerrit.acceptance.GitUtil.fetch;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allow;
+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.testing.GerritJUnit.assertThrows;
@@ -55,8 +57,12 @@
@Test
public void cannotCreateCheckerRef() throws Exception {
- grant(allProjects, CheckerRef.REFS_CHECKERS + "*", Permission.CREATE);
- grant(allProjects, CheckerRef.REFS_CHECKERS + "*", Permission.PUSH);
+ projectOperations
+ .project(allProjects)
+ .forUpdate()
+ .add(allow(Permission.CREATE).ref(CheckerRef.REFS_CHECKERS + "*").group(adminGroupUuid()))
+ .add(allow(Permission.PUSH).ref(CheckerRef.REFS_CHECKERS + "*").group(adminGroupUuid()))
+ .update();
String checkerRef = CheckerUuid.parse("test:my-checker").toRefName();
@@ -72,8 +78,12 @@
@Test
public void canCreateCheckerLikeRef() throws Exception {
- grant(project, CheckerRef.REFS_CHECKERS + "*", Permission.CREATE);
- grant(project, CheckerRef.REFS_CHECKERS + "*", Permission.PUSH);
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allow(Permission.CREATE).ref(CheckerRef.REFS_CHECKERS + "*").group(adminGroupUuid()))
+ .add(allow(Permission.PUSH).ref(CheckerRef.REFS_CHECKERS + "*").group(adminGroupUuid()))
+ .update();
String checkerRef = CheckerUuid.parse("test:my-checker").toRefName();
@@ -89,7 +99,15 @@
@Test
public void cannotDeleteCheckerRef() throws Exception {
- grant(allProjects, CheckerRef.REFS_CHECKERS + "*", Permission.DELETE, true, REGISTERED_USERS);
+ projectOperations
+ .project(allProjects)
+ .forUpdate()
+ .add(
+ allow(Permission.DELETE)
+ .ref(CheckerRef.REFS_CHECKERS + "*")
+ .group(REGISTERED_USERS)
+ .force(true))
+ .update();
CheckerUuid checkerUuid = checkerOperations.newChecker().create();
String checkerRef = checkerUuid.toRefName();
@@ -107,11 +125,17 @@
@Test
public void canDeleteCheckerLikeRef() throws Exception {
- grant(project, CheckerRef.REFS_CHECKERS + "*", Permission.DELETE, true, REGISTERED_USERS);
-
String checkerRef = CheckerUuid.parse("foo:bar").toRefName();
-
- allow(checkerRef, Permission.CREATE, adminGroupUuid());
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(
+ allow(Permission.DELETE)
+ .ref(CheckerRef.REFS_CHECKERS + "*")
+ .group(REGISTERED_USERS)
+ .force(true))
+ .add(allow(Permission.CREATE).ref(checkerRef).group(adminGroupUuid()))
+ .update();
createBranch(BranchNameKey.create(project, checkerRef));
// checker ref can be deleted in any project except All-Projects
@@ -134,7 +158,11 @@
fetch(repo, checkerRef + ":checkerRef");
repo.reset("checkerRef");
- grant(allProjects, CheckerRef.REFS_CHECKERS + "*", Permission.PUSH);
+ projectOperations
+ .project(allProjects)
+ .forUpdate()
+ .add(allow(Permission.PUSH).ref(CheckerRef.REFS_CHECKERS + "*").group(adminGroupUuid()))
+ .update();
PushOneCommit.Result r = pushFactory.create(admin.newIdent(), repo).to(checkerRef);
r.assertErrorStatus();
r.assertMessage("direct update of checker ref not allowed");
@@ -144,14 +172,22 @@
public void updateCheckerLikeRefByPush() throws Exception {
String checkerRef = CheckerUuid.parse("foo:bar").toRefName();
- allow(checkerRef, Permission.CREATE, adminGroupUuid());
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allow(Permission.CREATE).ref(checkerRef).group(adminGroupUuid()))
+ .update();
createBranch(BranchNameKey.create(project, checkerRef));
TestRepository<InMemoryRepository> repo = cloneProject(project, admin);
fetch(repo, checkerRef + ":checkerRef");
repo.reset("checkerRef");
- grant(project, CheckerRef.REFS_CHECKERS + "*", Permission.PUSH);
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allow(Permission.PUSH).ref(CheckerRef.REFS_CHECKERS + "*").group(adminGroupUuid()))
+ .update();
PushOneCommit.Result r = pushFactory.create(admin.newIdent(), repo).to(checkerRef);
r.assertOkStatus();
}
@@ -162,18 +198,22 @@
String checkerRef = checkerUuid.toRefName();
String changeId = createChangeWithoutCommitValidation(allProjects, checkerRef);
- grantLabel(
- "Code-Review",
- -2,
- 2,
- allProjects,
- CheckerRef.REFS_CHECKERS + "*",
- false,
- adminGroupUuid(),
- false);
+ projectOperations
+ .project(allProjects)
+ .forUpdate()
+ .add(
+ allowLabel("Code-Review")
+ .ref(CheckerRef.REFS_CHECKERS + "*")
+ .group(adminGroupUuid())
+ .range(-2, 2))
+ .update();
approve(changeId);
- grant(allProjects, CheckerRef.REFS_CHECKERS + "*", Permission.SUBMIT);
+ projectOperations
+ .project(allProjects)
+ .forUpdate()
+ .add(allow(Permission.SUBMIT).ref(CheckerRef.REFS_CHECKERS + "*").group(adminGroupUuid()))
+ .update();
ResourceConflictException thrown =
assertThrows(
@@ -185,23 +225,31 @@
public void submitToCheckerLikeRef() throws Exception {
String checkerRef = CheckerUuid.parse("foo:bar").toRefName();
- allow(checkerRef, Permission.CREATE, adminGroupUuid());
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allow(Permission.CREATE).ref(checkerRef).group(adminGroupUuid()))
+ .update();
createBranch(BranchNameKey.create(project, checkerRef));
String changeId = createChangeWithoutCommitValidation(project, checkerRef);
- grantLabel(
- "Code-Review",
- -2,
- 2,
- project,
- CheckerRef.REFS_CHECKERS + "*",
- false,
- adminGroupUuid(),
- false);
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(
+ allowLabel("Code-Review")
+ .ref(CheckerRef.REFS_CHECKERS + "*")
+ .group(adminGroupUuid())
+ .range(-2, 2))
+ .update();
approve(changeId);
- grant(project, CheckerRef.REFS_CHECKERS + "*", Permission.SUBMIT);
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allow(Permission.SUBMIT).ref(CheckerRef.REFS_CHECKERS + "*").group(adminGroupUuid()))
+ .update();
// submitting to a checker ref should work in any project except All-Projects
gApi.changes().id(changeId).current().submit();
@@ -218,7 +266,11 @@
fetch(repo, checkerRef + ":checkerRef");
repo.reset("checkerRef");
- grant(allProjects, CheckerRef.REFS_CHECKERS + "*", Permission.PUSH);
+ projectOperations
+ .project(allProjects)
+ .forUpdate()
+ .add(allow(Permission.PUSH).ref(CheckerRef.REFS_CHECKERS + "*").group(adminGroupUuid()))
+ .update();
PushOneCommit.Result r =
pushFactory.create(admin.newIdent(), repo).to("refs/for/" + checkerRef);
r.assertErrorStatus();
@@ -229,7 +281,11 @@
public void createChangeForCheckerLikeRefByPush() throws Exception {
String checkerRef = CheckerUuid.parse("foo:bar").toRefName();
- allow(checkerRef, Permission.CREATE, adminGroupUuid());
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allow(Permission.CREATE).ref(checkerRef).group(adminGroupUuid()))
+ .update();
createBranch(BranchNameKey.create(project, checkerRef));
TestRepository<InMemoryRepository> repo = cloneProject(project, admin);
@@ -237,7 +293,11 @@
repo.reset("checkerRef");
// creating a change on a checker ref by push should work in any project except All-Projects
- grant(project, CheckerRef.REFS_CHECKERS + "*", Permission.PUSH);
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allow(Permission.PUSH).ref(CheckerRef.REFS_CHECKERS + "*").group(adminGroupUuid()))
+ .update();
PushOneCommit.Result r =
pushFactory.create(admin.newIdent(), repo).to("refs/for/" + checkerRef);
r.assertOkStatus();
@@ -268,7 +328,11 @@
public void createChangeForCheckerLikeRefViaApi() throws Exception {
String checkerRef = CheckerUuid.parse("foo:bar").toRefName();
- allow(checkerRef, Permission.CREATE, adminGroupUuid());
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allow(Permission.CREATE).ref(checkerRef).group(adminGroupUuid()))
+ .update();
createBranch(BranchNameKey.create(project, checkerRef));
TestRepository<InMemoryRepository> repo = cloneProject(project, admin);
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckIT.java
index 808027b..a8a8d19 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckIT.java
@@ -18,6 +18,7 @@
import static com.google.common.truth.Truth8.assertThat;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
@@ -354,6 +355,23 @@
assertThat(thrown).hasMessageThat().contains("Authentication required");
}
+ @Test
+ public void createCheckOnChangeEditRejected() throws Exception {
+ int changeId = patchSetId.changeId().get();
+ gApi.changes().id(changeId).edit().modifyCommitMessage("new message");
+ assertThat(gApi.changes().id(changeId).edit().get()).isPresent();
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+
+ CheckInput input = new CheckInput();
+ input.checkerUuid = checkerUuid.get();
+ input.state = CheckState.RUNNING;
+ RestResponse response =
+ adminRestSession.post("/changes/" + changeId + "/revisions/edit/checks~checks", input);
+
+ response.assertConflict();
+ assertThat(response.getEntityContent()).isEqualTo("checks are not supported on a change edit");
+ }
+
// TODO(gerrit-team) More tests, especially for multiple checkers and PS and how commits behave
private Check getCheck(Project.NameKey project, PatchSet.Id patchSetId, CheckerUuid checkerUuid)
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckerIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckerIT.java
index 88859f1..7514878 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckerIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckerIT.java
@@ -15,7 +15,6 @@
package com.google.gerrit.plugins.checks.acceptance.api;
import static com.google.common.truth.Truth.assertThat;
-import static com.google.common.truth.Truth.assert_;
import static com.google.gerrit.git.testing.CommitSubject.assertCommit;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
@@ -385,14 +384,11 @@
input.repository = allProjects.get();
input.query = CheckerTestData.QUERY_WITH_UNSUPPORTED_OPERATOR;
- try {
- checkersApi.create(input).get();
- assert_().fail("expected BadRequestException");
- } catch (BadRequestException e) {
- assertThat(e)
- .hasMessageThat()
- .isEqualTo("Unsupported operator: " + CheckerTestData.UNSUPPORTED_OPERATOR);
- }
+ BadRequestException thrown =
+ assertThrows(BadRequestException.class, () -> checkersApi.create(input).get());
+ assertThat(thrown)
+ .hasMessageThat()
+ .isEqualTo("Unsupported operator: " + CheckerTestData.UNSUPPORTED_OPERATOR);
}
@Test
@@ -402,12 +398,9 @@
input.repository = allProjects.get();
input.query = CheckerTestData.INVALID_QUERY;
- try {
- checkersApi.create(input).get();
- assert_().fail("expected BadRequestException");
- } catch (BadRequestException e) {
- assertThat(e).hasMessageThat().contains("Invalid query: " + input.query);
- }
+ BadRequestException thrown =
+ assertThrows(BadRequestException.class, () -> checkersApi.create(input).get());
+ assertThat(thrown).hasMessageThat().contains("Invalid query: " + input.query);
}
@Test
@@ -417,19 +410,16 @@
input.repository = allProjects.get();
input.query = CheckerTestData.longQueryWithSupportedOperators(MAX_INDEX_TERMS * 2);
assertThat(CheckerQuery.clean(input.query)).isEqualTo(input.query);
- try {
- checkersApi.create(input).get();
- assert_().fail("expected BadRequestException");
- } catch (BadRequestException e) {
- assertThat(e)
- .hasMessageThat()
- .isEqualTo(
- "change query of checker "
- + input.uuid
- + " is invalid: "
- + input.query
- + " (too many terms in query)");
- }
+ BadRequestException thrown =
+ assertThrows(BadRequestException.class, () -> checkersApi.create(input).get());
+ assertThat(thrown)
+ .hasMessageThat()
+ .isEqualTo(
+ "change query of checker "
+ + input.uuid
+ + " is invalid: "
+ + input.query
+ + " (too many terms in query)");
}
@Test
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 a8239bb..9e2a066 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckIT.java
@@ -16,7 +16,7 @@
import static com.google.common.collect.Iterables.getOnlyElement;
import static com.google.common.truth.Truth.assertThat;
-import static com.google.common.truth.Truth.assert_;
+import static com.google.common.truth.Truth8.assertThat;
import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_REVISION;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
@@ -28,6 +28,7 @@
import com.google.gerrit.acceptance.rest.util.RestCall;
import com.google.gerrit.acceptance.rest.util.RestCall.Method;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
+import com.google.gerrit.extensions.common.EditInfo;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestApiException;
@@ -47,6 +48,7 @@
import com.google.inject.Inject;
import java.sql.Timestamp;
import java.time.Instant;
+import java.util.Optional;
import java.util.concurrent.TimeUnit;
import org.junit.After;
import org.junit.Before;
@@ -510,15 +512,28 @@
gApi.changes().id(patchSetId.changeId().get()).delete();
- try {
- getCheckInfo(patchSetId, checkerUuid);
- assert_().fail("expected ResourceNotFoundException");
- } catch (ResourceNotFoundException e) {
- assertThat(e)
- .hasMessageThat()
- .ignoringCase()
- .contains(String.format("change %d", patchSetId.changeId().get()));
- }
+ ResourceNotFoundException thrown =
+ assertThrows(ResourceNotFoundException.class, () -> getCheckInfo(patchSetId, checkerUuid));
+ assertThat(thrown)
+ .hasMessageThat()
+ .ignoringCase()
+ .contains(String.format("change %d", patchSetId.changeId().get()));
+ }
+
+ @Test
+ public void getCheckOnChangeEditRejected() throws Exception {
+ int changeId = patchSetId.changeId().get();
+ gApi.changes().id(changeId).edit().modifyCommitMessage("new message");
+ Optional<EditInfo> editInfo = gApi.changes().id(changeId).edit().get();
+ assertThat(editInfo).isPresent();
+
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+ RestResponse response =
+ adminRestSession.get(
+ "/changes/" + changeId + "/revisions/edit/checks~checks/" + checkerUuid.get());
+
+ response.assertConflict();
+ assertThat(response.getEntityContent()).isEqualTo("checks are not supported on a change edit");
}
private CheckInfo getCheckInfo(
@@ -535,17 +550,14 @@
private void assertCheckNotFound(PatchSet.Id patchSetId, CheckerUuid checkerUuid)
throws Exception {
- try {
- getCheckInfo(patchSetId, checkerUuid);
- assert_().fail("expected ResourceNotFoundException");
- } catch (ResourceNotFoundException e) {
- assertThat(e)
- .hasMessageThat()
- .isEqualTo(
- String.format(
- "Patch set %s in repository %s doesn't have check for checker %s.",
- patchSetId, project, checkerUuid));
- }
+ ResourceNotFoundException thrown =
+ assertThrows(ResourceNotFoundException.class, () -> getCheckInfo(patchSetId, checkerUuid));
+ assertThat(thrown)
+ .hasMessageThat()
+ .isEqualTo(
+ String.format(
+ "Patch set %s in repository %s doesn't have check for checker %s.",
+ patchSetId, project, checkerUuid));
}
private PatchSet.Id createPatchSet() throws Exception {
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/ListCheckersIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/ListCheckersIT.java
index 1fcdb91..590f82f 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/ListCheckersIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/ListCheckersIT.java
@@ -15,7 +15,7 @@
package com.google.gerrit.plugins.checks.acceptance.api;
import static com.google.common.truth.Truth.assertThat;
-import static com.google.common.truth.Truth.assert_;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import static java.util.stream.Collectors.toList;
import com.google.common.collect.ImmutableList;
@@ -63,12 +63,10 @@
requestScopeOperations.setApiUser(user.id());
- try {
- checkersApi.all();
- assert_().fail("expected AuthException");
- } catch (AuthException e) {
- assertThat(e.getMessage()).isEqualTo("administrateCheckers for plugin checks not permitted");
- }
+ AuthException thrown = assertThrows(AuthException.class, () -> checkersApi.all());
+ assertThat(thrown)
+ .hasMessageThat()
+ .isEqualTo("administrateCheckers for plugin checks not permitted");
}
@Test
@@ -77,12 +75,8 @@
requestScopeOperations.setApiUserAnonymous();
- try {
- checkersApi.all();
- assert_().fail("expected AuthException");
- } catch (AuthException e) {
- assertThat(e.getMessage()).isEqualTo("Authentication required");
- }
+ AuthException thrown = assertThrows(AuthException.class, () -> checkersApi.all());
+ assertThat(thrown).hasMessageThat().isEqualTo("Authentication required");
}
@Test
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/ListChecksIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/ListChecksIT.java
index bf12827..9650784 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/ListChecksIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/ListChecksIT.java
@@ -23,6 +23,7 @@
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
+import com.google.gerrit.extensions.common.EditInfo;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.plugins.checks.CheckKey;
import com.google.gerrit.plugins.checks.CheckerUuid;
@@ -327,6 +328,18 @@
assertThat(checksApiFactory.revision(patchSetId).list()).hasSize(1);
}
+ @Test
+ public void listAllOnChangeEditRejected() throws Exception {
+ gApi.changes().id(changeId).edit().modifyCommitMessage("new message");
+ Optional<EditInfo> editInfo = gApi.changes().id(changeId).edit().get();
+ assertThat(editInfo).isPresent();
+
+ RestResponse response =
+ adminRestSession.get("/changes/" + changeId + "/revisions/edit/checks?o=CHECKER");
+ response.assertConflict();
+ assertThat(response.getEntityContent()).isEqualTo("checks are not supported on a change edit");
+ }
+
private Timestamp getPatchSetCreated(Change.Id changeId) throws RestApiException {
return getOnlyElement(
gApi.changes().id(changeId.get()).get(CURRENT_REVISION).revisions.values())
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/QueryPendingChecksIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/QueryPendingChecksIT.java
index 697d183..5bfbee0 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/QueryPendingChecksIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/QueryPendingChecksIT.java
@@ -15,7 +15,8 @@
package com.google.gerrit.plugins.checks.acceptance.api;
import static com.google.common.truth.Truth.assertThat;
-import static com.google.common.truth.Truth.assert_;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allow;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.block;
import static com.google.gerrit.plugins.checks.testing.PendingChecksInfoSubject.assertThat;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
@@ -34,7 +35,6 @@
import com.google.gerrit.plugins.checks.api.PendingCheckInfo;
import com.google.gerrit.plugins.checks.api.PendingChecksInfo;
import com.google.gerrit.reviewdb.client.PatchSet;
-import com.google.gerrit.server.project.testing.Util;
import com.google.gerrit.testing.TestTimeUtil;
import com.google.gson.reflect.TypeToken;
import com.google.inject.Inject;
@@ -580,11 +580,12 @@
@Test
public void pendingChecksDontIncludeChecksForNonVisibleChanges() throws Exception {
// restrict project visibility so that it is only visible to administrators
- try (ProjectConfigUpdate u = updateProject(project)) {
- Util.allow(u.getConfig(), Permission.READ, adminGroupUuid(), "refs/*");
- Util.block(u.getConfig(), Permission.READ, REGISTERED_USERS, "refs/*");
- u.save();
- }
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allow(Permission.READ).ref("refs/*").group(adminGroupUuid()))
+ .add(block(Permission.READ).ref("refs/*").group(REGISTERED_USERS))
+ .update();
CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
checkOperations
@@ -643,13 +644,10 @@
assertThat(pendingChecksList).isNotEmpty();
}
- private void assertInvalidQuery(String query, String expectedMessage) throws RestApiException {
- try {
- pendingChecksApi.query(query).get();
- assert_().fail("expected BadRequestException");
- } catch (BadRequestException e) {
- assertThat(e).hasMessageThat().isEqualTo(expectedMessage);
- }
+ private void assertInvalidQuery(String query, String expectedMessage) {
+ BadRequestException thrown =
+ assertThrows(BadRequestException.class, () -> pendingChecksApi.query(query).get());
+ assertThat(thrown).hasMessageThat().isEqualTo(expectedMessage);
}
private List<PendingChecksInfo> queryPendingChecks(String queryString) throws RestApiException {
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/UpdateCheckerIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/UpdateCheckerIT.java
index 8b9be46..4089d63 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/UpdateCheckerIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/UpdateCheckerIT.java
@@ -15,7 +15,6 @@
package com.google.gerrit.plugins.checks.acceptance.api;
import static com.google.common.truth.Truth.assertThat;
-import static com.google.common.truth.Truth.assert_;
import static com.google.common.truth.Truth8.assertThat;
import static com.google.gerrit.git.testing.CommitSubject.assertCommit;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
@@ -498,14 +497,11 @@
CheckerInput input = new CheckerInput();
input.query = CheckerTestData.QUERY_WITH_UNSUPPORTED_OPERATOR;
- try {
- checkersApi.id(checkerUuid).update(input);
- assert_().fail("expected BadRequestException");
- } catch (BadRequestException e) {
- assertThat(e)
- .hasMessageThat()
- .isEqualTo("Unsupported operator: " + CheckerTestData.UNSUPPORTED_OPERATOR);
- }
+ BadRequestException thrown =
+ assertThrows(BadRequestException.class, () -> checkersApi.id(checkerUuid).update(input));
+ assertThat(thrown)
+ .hasMessageThat()
+ .isEqualTo("Unsupported operator: " + CheckerTestData.UNSUPPORTED_OPERATOR);
assertThat(checkerOperations.checker(checkerUuid).get().getQuery()).isEqualTo(oldQuery);
}
@@ -517,12 +513,9 @@
CheckerInput input = new CheckerInput();
input.query = CheckerTestData.INVALID_QUERY;
- try {
- checkersApi.id(checkerUuid).update(input);
- assert_().fail("expected BadRequestException");
- } catch (BadRequestException e) {
- assertThat(e).hasMessageThat().contains("Invalid query: " + input.query);
- }
+ BadRequestException thrown =
+ assertThrows(BadRequestException.class, () -> checkersApi.id(checkerUuid).update(input));
+ assertThat(thrown).hasMessageThat().contains("Invalid query: " + input.query);
assertThat(checkerOperations.checker(checkerUuid).get().getQuery()).isEqualTo(oldQuery);
}
@@ -535,19 +528,16 @@
CheckerInput input = new CheckerInput();
input.query = CheckerTestData.longQueryWithSupportedOperators(MAX_INDEX_TERMS * 2);
assertThat(CheckerQuery.clean(input.query)).isEqualTo(input.query);
- try {
- checkersApi.id(checkerUuid).update(input);
- assert_().fail("expected BadRequestException");
- } catch (BadRequestException e) {
- assertThat(e)
- .hasMessageThat()
- .isEqualTo(
- "change query of checker "
- + checkerUuid
- + " is invalid: "
- + input.query
- + " (too many terms in query)");
- }
+ BadRequestException thrown =
+ assertThrows(BadRequestException.class, () -> checkersApi.id(checkerUuid).update(input));
+ assertThat(thrown)
+ .hasMessageThat()
+ .isEqualTo(
+ "change query of checker "
+ + checkerUuid
+ + " is invalid: "
+ + input.query
+ + " (too many terms in query)");
assertThat(checkerOperations.checker(checkerUuid).get().getQuery()).isEqualTo(oldQuery);
}
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckTestDataTest.java b/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckTestDataTest.java
index e81c171..4c61a0c 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckTestDataTest.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckTestDataTest.java
@@ -15,7 +15,7 @@
package com.google.gerrit.plugins.checks.acceptance.testsuite;
import static com.google.common.truth.Truth.assertThat;
-import static com.google.common.truth.Truth.assert_;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.plugins.checks.UrlValidator;
@@ -25,12 +25,10 @@
public class CheckTestDataTest extends AbstractCheckersTest {
@Test
public void verifyTestUrls() throws Exception {
- try {
- UrlValidator.clean(CheckTestData.INVALID_URL);
- assert_().fail("expected BadRequestException");
- } catch (BadRequestException e) {
- assertMessage(e, "only http/https URLs supported", CheckTestData.INVALID_URL);
- }
+ BadRequestException thrown =
+ assertThrows(
+ BadRequestException.class, () -> UrlValidator.clean(CheckTestData.INVALID_URL));
+ assertMessage(thrown, "only http/https URLs supported", CheckTestData.INVALID_URL);
}
private static void assertMessage(Exception e, String... expectedMessageParts) {
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerOperationsImplTest.java b/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerOperationsImplTest.java
index ade6bad..403a3a7 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerOperationsImplTest.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerOperationsImplTest.java
@@ -16,7 +16,6 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
-import static com.google.common.truth.Truth.assert_;
import static com.google.common.truth.Truth8.assertThat;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import static java.nio.charset.StandardCharsets.UTF_8;
@@ -539,12 +538,7 @@
checkerOperations.checker(checkerUuid).forInvalidation().nonParseableConfig().invalidate();
- try {
- checkers.getChecker(checkerUuid);
- assert_().fail("expected ConfigInvalidException");
- } catch (ConfigInvalidException e) {
- // expected
- }
+ assertThrows(ConfigInvalidException.class, () -> checkers.getChecker(checkerUuid));
}
@Test
@@ -553,14 +547,10 @@
checkerOperations.checker(checkerUuid).forInvalidation().invalidUuid().invalidate();
- try {
- checkers.getChecker(checkerUuid);
- assert_().fail("expected ConfigInvalidException");
- } catch (ConfigInvalidException e) {
- // expected
- assertThat(e.getMessage()).contains("value of checker.uuid");
- assertThat(e.getMessage()).contains("does not match expected checker UUID");
- }
+ ConfigInvalidException thrown =
+ assertThrows(ConfigInvalidException.class, () -> checkers.getChecker(checkerUuid));
+ assertThat(thrown).hasMessageThat().contains("value of checker.uuid");
+ assertThat(thrown).hasMessageThat().contains("does not match expected checker UUID");
}
@Test
@@ -573,13 +563,9 @@
.invalidBlockingCondition()
.invalidate();
- try {
- checkers.getChecker(checkerUuid);
- assert_().fail("expected ConfigInvalidException");
- } catch (ConfigInvalidException e) {
- // expected
- assertThat(e.getMessage()).contains("Invalid value: checker.blocking");
- }
+ ConfigInvalidException thrown =
+ assertThrows(ConfigInvalidException.class, () -> checkers.getChecker(checkerUuid));
+ assertThat(thrown).hasMessageThat().contains("Invalid value: checker.blocking");
}
@Test
@@ -588,13 +574,9 @@
checkerOperations.checker(checkerUuid).forInvalidation().invalidStatus().invalidate();
- try {
- checkers.getChecker(checkerUuid);
- assert_().fail("expected ConfigInvalidException");
- } catch (ConfigInvalidException e) {
- // expected
- assertThat(e.getMessage()).contains("Invalid value: checker.status");
- }
+ ConfigInvalidException thrown =
+ assertThrows(ConfigInvalidException.class, () -> checkers.getChecker(checkerUuid));
+ assertThat(thrown).hasMessageThat().contains("Invalid value: checker.status");
}
@Test
@@ -603,13 +585,9 @@
checkerOperations.checker(checkerUuid).forInvalidation().unsetUuid().invalidate();
- try {
- checkers.getChecker(checkerUuid);
- assert_().fail("expected ConfigInvalidException");
- } catch (ConfigInvalidException e) {
- // expected
- assertThat(e.getMessage()).contains("checker.uuid is not set in config file");
- }
+ ConfigInvalidException thrown =
+ assertThrows(ConfigInvalidException.class, () -> checkers.getChecker(checkerUuid));
+ assertThat(thrown).hasMessageThat().contains("checker.uuid is not set in config file");
}
@Test
@@ -618,13 +596,9 @@
checkerOperations.checker(checkerUuid).forInvalidation().unsetRepository().invalidate();
- try {
- checkers.getChecker(checkerUuid);
- assert_().fail("expected ConfigInvalidException");
- } catch (ConfigInvalidException e) {
- // expected
- assertThat(e.getMessage()).contains("checker.repository is not set in config file");
- }
+ ConfigInvalidException thrown =
+ assertThrows(ConfigInvalidException.class, () -> checkers.getChecker(checkerUuid));
+ assertThat(thrown).hasMessageThat().contains("checker.repository is not set in config file");
}
@Test
@@ -633,13 +607,9 @@
checkerOperations.checker(checkerUuid).forInvalidation().unsetStatus().invalidate();
- try {
- checkers.getChecker(checkerUuid);
- assert_().fail("expected ConfigInvalidException");
- } catch (ConfigInvalidException e) {
- // expected
- assertThat(e.getMessage()).contains("checker.status is not set in config file");
- }
+ ConfigInvalidException thrown =
+ assertThrows(ConfigInvalidException.class, () -> checkers.getChecker(checkerUuid));
+ assertThat(thrown).hasMessageThat().contains("checker.status is not set in config file");
}
@Test
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerTestDataTest.java b/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerTestDataTest.java
index 37b22e0..cde0900 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerTestDataTest.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerTestDataTest.java
@@ -15,7 +15,7 @@
package com.google.gerrit.plugins.checks.acceptance.testsuite;
import static com.google.common.truth.Truth.assertThat;
-import static com.google.common.truth.Truth.assert_;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.plugins.checks.CheckerQuery;
@@ -33,12 +33,10 @@
@Test
public void verifyTestUrls() throws Exception {
- try {
- UrlValidator.clean(CheckerTestData.INVALID_URL);
- assert_().fail("expected BadRequestException");
- } catch (BadRequestException e) {
- assertMessage(e, "only http/https URLs supported", CheckerTestData.INVALID_URL);
- }
+ BadRequestException thrown =
+ assertThrows(
+ BadRequestException.class, () -> UrlValidator.clean(CheckerTestData.INVALID_URL));
+ assertMessage(thrown, "only http/https URLs supported", CheckerTestData.INVALID_URL);
}
@Test
@@ -58,12 +56,9 @@
}
private static void assertInvalidQuery(String query, String... expectedMessageParts) {
- try {
- CheckerQuery.clean(query);
- assert_().fail("expected ConfigInvalidException");
- } catch (ConfigInvalidException e) {
- assertMessage(e, expectedMessageParts);
- }
+ ConfigInvalidException thrown =
+ assertThrows(ConfigInvalidException.class, () -> CheckerQuery.clean(query));
+ assertMessage(thrown, expectedMessageParts);
}
private static void assertMessage(Exception e, String... expectedMessageParts) {
diff --git a/javatests/com/google/gerrit/plugins/checks/db/CheckerConfigTest.java b/javatests/com/google/gerrit/plugins/checks/db/CheckerConfigTest.java
index 706d4a0..15b85fe 100644
--- a/javatests/com/google/gerrit/plugins/checks/db/CheckerConfigTest.java
+++ b/javatests/com/google/gerrit/plugins/checks/db/CheckerConfigTest.java
@@ -16,7 +16,6 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
-import static com.google.common.truth.Truth.assert_;
import static com.google.gerrit.plugins.checks.testing.CheckerConfigSubject.assertThat;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
@@ -462,12 +461,7 @@
@Test
public void setQueryDoesNotValidateQuery() throws Exception {
String query = "foo:bar";
- try {
- CheckerQuery.clean(query);
- assert_().fail("expected ConfigInvalidException");
- } catch (ConfigInvalidException e) {
- // Expected.
- }
+ assertThrows(ConfigInvalidException.class, () -> CheckerQuery.clean(query));
createArbitraryChecker(checkerUuid);
