From 20c7d695fb082213d9b301908ce4172cb0304baf Mon Sep 17 00:00:00 2001 From: dborowitz Date: Thu, 2 May 2019 08:53:58 -0700 Subject: [PATCH] Use assertThrows for testing exceptions Based on a change generated by an automated refactoring tool. Covers just the portions of Gerrit tests that are run internally at Google. The refactoring tool is not perfect, so some modifications were required to get tests to compile and pass, for example: * The tool didn't detect when variables referenced from newly-created lambdas were not effectively final in the enclosing scope. * The tool moved the entire body of methods with @Test(expected). This does actually match the semantics of the @Test annotation (and is the reason that construction is so strongly discouraged). However, our implementation of assertThrows does not propagate the exceptions thrown when an assume() call fails. This may or may not be desirable in our library, but for all the instances where it occurred in the code, it was clear that the right thing to do was to minimize the code in the lambda body anyway. For a full list of modifications from the tool's original output, diff this commit against earlier patch sets of this change. PiperOrigin-RevId: 246326737 Change-Id: I65a4c9635ed62ecae8097f9d284609add6b9929b --- .../gerrit/acceptance/AbstractDaemonTest.java | 4 +- .../gerrit/testing/GerritBaseTests.java | 4 +- .../acceptance/api/accounts/AccountIT.java | 252 +++++---- .../api/accounts/AccountManagerIT.java | 51 +- .../acceptance/api/accounts/AgreementsIT.java | 61 ++- .../api/accounts/GeneralPreferencesIT.java | 27 +- .../acceptance/api/change/AbandonIT.java | 39 +- .../acceptance/api/change/ChangeIT.java | 492 +++++++++++------- .../acceptance/api/change/ChangeIdIT.java | 70 ++- .../api/change/DisablePrivateChangesIT.java | 15 +- .../acceptance/api/change/MergeListIT.java | 21 +- .../api/change/PrivateChangeIT.java | 34 +- .../gerrit/acceptance/api/group/GroupsIT.java | 109 ++-- .../acceptance/api/group/GroupsUpdateIT.java | 10 +- .../acceptance/api/plugin/PluginIT.java | 12 +- .../acceptance/api/project/CheckAccessIT.java | 41 +- .../api/project/CheckProjectIT.java | 29 +- .../acceptance/api/project/DashboardIT.java | 16 +- .../acceptance/api/project/ProjectIT.java | 62 +-- .../acceptance/api/project/SetParentIT.java | 66 ++- .../acceptance/api/revision/RevisionIT.java | 165 ++++-- .../api/revision/RobotCommentsIT.java | 142 +++-- .../gerrit/acceptance/edit/ChangeEditIT.java | 52 +- .../gerrit/acceptance/git/GitmodulesIT.java | 16 +- ...bmoduleSubscriptionsWholeTopicMergeIT.java | 35 +- .../acceptance/rest/account/EmailIT.java | 29 +- .../acceptance/rest/account/ExternalIdIT.java | 50 +- .../acceptance/rest/account/GetAccountIT.java | 5 +- .../rest/account/ImpersonationIT.java | 97 ++-- .../rest/account/WatchedProjectsIT.java | 15 +- .../rest/change/AbstractSubmit.java | 33 +- .../acceptance/rest/change/AssigneeIT.java | 11 +- .../acceptance/rest/change/ChangeOwnerIT.java | 5 +- .../rest/change/ChangeReviewersIT.java | 25 +- .../rest/change/CreateChangeIT.java | 6 +- .../acceptance/rest/change/HashtagsIT.java | 12 +- .../acceptance/rest/change/MoveChangeIT.java | 95 ++-- .../rest/change/PrivateByDefaultIT.java | 7 +- .../acceptance/rest/project/AccessIT.java | 54 +- .../rest/project/CreateBranchIT.java | 6 +- .../rest/project/CreateProjectIT.java | 4 +- .../rest/project/DeleteBranchIT.java | 30 +- .../rest/project/DeleteBranchesIT.java | 19 +- .../acceptance/rest/project/DeleteTagIT.java | 9 +- .../acceptance/rest/project/FileBranchIT.java | 5 +- .../rest/project/GetChildProjectIT.java | 10 +- .../acceptance/rest/project/GetProjectIT.java | 6 +- .../rest/project/ListBranchesIT.java | 11 +- .../rest/project/ListChildProjectsIT.java | 10 +- .../acceptance/rest/project/TagsIT.java | 67 +-- .../acceptance/server/change/CommentsIT.java | 19 +- .../server/project/CustomLabelIT.java | 19 +- .../acceptance/server/project/ReflogIT.java | 5 +- .../server/quota/DefaultQuotaBackendIT.java | 12 +- .../group/GroupOperationsImplTest.java | 6 +- .../gerrit/common/data/AccessSectionTest.java | 44 +- .../common/data/GroupReferenceTest.java | 8 +- .../common/data/PermissionRuleTest.java | 4 +- .../httpd/restapi/ParameterParserTest.java | 29 +- .../google/gerrit/index/SchemaUtilTest.java | 7 +- .../gerrit/index/query/AndPredicateTest.java | 23 +- .../index/query/FieldPredicateTest.java | 9 +- .../gerrit/index/query/NotPredicateTest.java | 46 +- .../gerrit/index/query/OrPredicateTest.java | 23 +- .../gerrit/reviewdb/client/RefNamesTest.java | 12 +- .../server/account/HashedPasswordTest.java | 5 +- .../server/cache/PerThreadCacheTest.java | 7 +- .../gerrit/server/config/SitePathsTest.java | 5 +- .../fixes/FixReplacementInterpreterTest.java | 19 +- .../server/fixes/LineIdentifierTest.java | 15 +- .../server/fixes/StringModifierTest.java | 25 +- .../git/LocalDiskRepositoryManagerTest.java | 147 ++++-- ...ltiBaseLocalDiskRepositoryManagerTest.java | 17 +- .../server/group/db/GroupConfigTest.java | 68 +-- .../server/group/db/GroupNameNotesTest.java | 65 ++- .../index/change/ChangeIndexRewriterTest.java | 12 +- .../gerrit/server/notedb/ChangeNotesTest.java | 62 +-- .../server/notedb/RepoSequenceTest.java | 35 +- .../server/patch/IntraLineLoaderTest.java | 29 +- .../server/permissions/RefControlTest.java | 11 +- .../account/AbstractQueryAccountsTest.java | 7 +- .../change/AbstractQueryChangesTest.java | 7 +- .../query/change/LuceneQueryChangesTest.java | 11 +- .../query/group/AbstractQueryGroupsTest.java | 7 +- .../project/AbstractQueryProjectsTest.java | 22 +- .../gerrit/server/rules/GerritCommonTest.java | 17 +- .../gerrit/server/util/SocketUtilTest.java | 14 +- .../gerrit/testing/IndexVersionsTest.java | 7 +- 88 files changed, 1931 insertions(+), 1395 deletions(-) diff --git a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java index 56db9a91e0..6c141262d9 100644 --- a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java +++ b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java @@ -201,7 +201,9 @@ public abstract class AbstractDaemonTest { @ConfigSuite.Parameter public Config baseConfig; @ConfigSuite.Name private String configName; - @Rule public ExpectedException exception = ExpectedException.none(); + @Deprecated // Use GerritJUnit#assertThrows + @Rule + public ExpectedException exception = ExpectedException.none(); @Rule public TestRule testRunner = diff --git a/java/com/google/gerrit/testing/GerritBaseTests.java b/java/com/google/gerrit/testing/GerritBaseTests.java index 0e3ee4c173..5d4abe63fd 100644 --- a/java/com/google/gerrit/testing/GerritBaseTests.java +++ b/java/com/google/gerrit/testing/GerritBaseTests.java @@ -20,5 +20,7 @@ import org.junit.rules.ExpectedException; @Ignore public abstract class GerritBaseTests { - @Rule public ExpectedException exception = ExpectedException.none(); + @Deprecated // Use GerritJUnit#assertThrows + @Rule + public ExpectedException exception = ExpectedException.none(); } diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java index b50cbf287c..5377f334c0 100644 --- a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java +++ b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java @@ -32,6 +32,7 @@ import static com.google.gerrit.server.StarredChangesUtil.IGNORE_LABEL; import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_GPGKEY; import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS; import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.Objects.requireNonNull; import static java.util.concurrent.TimeUnit.SECONDS; @@ -354,9 +355,11 @@ public class AccountIT extends AbstractDaemonTest { AccountInput input = new AccountInput(); input.username = admin.username(); - exception.expect(ResourceConflictException.class); - exception.expectMessage("username '" + admin.username() + "' already exists"); - gApi.accounts().create(input); + ResourceConflictException thrown = + assertThrows(ResourceConflictException.class, () -> gApi.accounts().create(input)); + assertThat(thrown) + .hasMessageThat() + .contains("username '" + admin.username() + "' already exists"); } @Test @@ -365,9 +368,9 @@ public class AccountIT extends AbstractDaemonTest { input.username = "foo"; input.email = admin.email(); - exception.expect(UnprocessableEntityException.class); - exception.expectMessage("email '" + admin.email() + "' already exists"); - gApi.accounts().create(input); + UnprocessableEntityException thrown = + assertThrows(UnprocessableEntityException.class, () -> gApi.accounts().create(input)); + assertThat(thrown).hasMessageThat().contains("email '" + admin.email() + "' already exists"); } @Test @@ -628,9 +631,10 @@ public class AccountIT extends AbstractDaemonTest { @Test public void deactivateSelf() throws Exception { - exception.expect(ResourceConflictException.class); - exception.expectMessage("cannot deactivate own account"); - gApi.accounts().self().setActive(false); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, () -> gApi.accounts().self().setActive(false)); + assertThat(thrown).hasMessageThat().contains("cannot deactivate own account"); } @Test @@ -723,23 +727,31 @@ public class AccountIT extends AbstractDaemonTest { accountIndexedCounter.assertNoReindex(); requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - exception.expectMessage("not allowed to get stars of another account"); - gApi.accounts().id(Integer.toString((admin.id().get()))).getStars(triplet); + AuthException thrown = + assertThrows( + AuthException.class, + () -> gApi.accounts().id(Integer.toString((admin.id().get()))).getStars(triplet)); + assertThat(thrown).hasMessageThat().contains("not allowed to get stars of another account"); } @Test public void starWithInvalidLabels() throws Exception { PushOneCommit.Result r = createChange(); String triplet = project.get() + "~master~" + r.getChangeId(); - exception.expect(BadRequestException.class); - exception.expectMessage("invalid labels: another invalid label, invalid label"); - gApi.accounts() - .self() - .setStars( - triplet, - new StarsInput( - ImmutableSet.of(DEFAULT_LABEL, "invalid label", "blue", "another invalid label"))); + BadRequestException thrown = + assertThrows( + BadRequestException.class, + () -> + gApi.accounts() + .self() + .setStars( + triplet, + new StarsInput( + ImmutableSet.of( + DEFAULT_LABEL, "invalid label", "blue", "another invalid label")))); + assertThat(thrown) + .hasMessageThat() + .contains("invalid labels: another invalid label, invalid label"); } @Test @@ -757,17 +769,24 @@ public class AccountIT extends AbstractDaemonTest { public void starWithDefaultAndIgnoreLabel() throws Exception { PushOneCommit.Result r = createChange(); String triplet = project.get() + "~master~" + r.getChangeId(); - exception.expect(BadRequestException.class); - exception.expectMessage( - "The labels " - + DEFAULT_LABEL - + " and " - + IGNORE_LABEL - + " are mutually exclusive." - + " Only one of them can be set."); - gApi.accounts() - .self() - .setStars(triplet, new StarsInput(ImmutableSet.of(DEFAULT_LABEL, "blue", IGNORE_LABEL))); + BadRequestException thrown = + assertThrows( + BadRequestException.class, + () -> + gApi.accounts() + .self() + .setStars( + triplet, + new StarsInput(ImmutableSet.of(DEFAULT_LABEL, "blue", IGNORE_LABEL)))); + assertThat(thrown) + .hasMessageThat() + .contains( + "The labels " + + DEFAULT_LABEL + + " and " + + IGNORE_LABEL + + " are mutually exclusive." + + " Only one of them can be set."); } @Test @@ -908,9 +927,9 @@ public class AccountIT extends AbstractDaemonTest { TestAccount foo = accountCreator.create(name("foo"), email, "Foo"); requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - exception.expectMessage("modify account not permitted"); - gApi.accounts().id(foo.id().get()).getEmails(); + AuthException thrown = + assertThrows(AuthException.class, () -> gApi.accounts().id(foo.id().get()).getEmails()); + assertThat(thrown).hasMessageThat().contains("modify account not permitted"); } @Test @@ -975,8 +994,7 @@ public class AccountIT extends AbstractDaemonTest { TestAccount account = accountCreator.create(name("user")); EmailInput input = newEmailInput("test@test.com"); requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - gApi.accounts().id(account.username()).addEmail(input); + assertThrows(AuthException.class, () -> gApi.accounts().id(account.username()).addEmail(input)); } @Test @@ -984,9 +1002,13 @@ public class AccountIT extends AbstractDaemonTest { String email = "new.email@example.com"; EmailInput input = newEmailInput(email); gApi.accounts().self().addEmail(input); - exception.expect(ResourceConflictException.class); - exception.expectMessage("Identity 'mailto:" + email + "' in use by another account"); - gApi.accounts().id(user.username()).addEmail(input); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, + () -> gApi.accounts().id(user.username()).addEmail(input)); + assertThat(thrown) + .hasMessageThat() + .contains("Identity 'mailto:" + email + "' in use by another account"); } @Test @@ -1022,9 +1044,14 @@ public class AccountIT extends AbstractDaemonTest { TestAccount user = accountCreator.create(); requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - exception.expectMessage("modify account not permitted"); - gApi.accounts().id(admin.id().get()).addEmail(newEmailInput("foo@example.com", false)); + AuthException thrown = + assertThrows( + AuthException.class, + () -> + gApi.accounts() + .id(admin.id().get()) + .addEmail(newEmailInput("foo@example.com", false))); + assertThat(thrown).hasMessageThat().contains("modify account not permitted"); } @Test @@ -1109,9 +1136,11 @@ public class AccountIT extends AbstractDaemonTest { assertThat(getEmails()).doesNotContain(email); // user cannot delete email of admin - exception.expect(AuthException.class); - exception.expectMessage("modify account not permitted"); - gApi.accounts().id(admin.id().get()).deleteEmail(admin.email()); + AuthException thrown = + assertThrows( + AuthException.class, + () -> gApi.accounts().id(admin.id().get()).deleteEmail(admin.email())); + assertThat(thrown).hasMessageThat().contains("modify account not permitted"); } @Test @@ -1205,8 +1234,9 @@ public class AccountIT extends AbstractDaemonTest { @Test public void userCannotSetNameOfOtherUser() throws Exception { requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - gApi.accounts().id(admin.username()).setName("Admin McAdminface"); + assertThrows( + AuthException.class, + () -> gApi.accounts().id(admin.username()).setName("Admin McAdminface")); } @Test @@ -1266,9 +1296,13 @@ public class AccountIT extends AbstractDaemonTest { // fetching user branch of another user fails String otherUserRefName = RefNames.refsUsers(admin.id()); - exception.expect(TransportException.class); - exception.expectMessage("Remote does not have " + otherUserRefName + " available for fetch."); - fetch(allUsersRepo, otherUserRefName + ":otherUserRef"); + TransportException thrown = + assertThrows( + TransportException.class, + () -> fetch(allUsersRepo, otherUserRefName + ":otherUserRef")); + assertThat(thrown) + .hasMessageThat() + .contains("Remote does not have " + otherUserRefName + " available for fetch."); } @Test @@ -1404,17 +1438,21 @@ public class AccountIT extends AbstractDaemonTest { assertThat(r.getChange().change().getDest().branch()).isEqualTo(userRef); gApi.changes().id(r.getChangeId()).current().review(ReviewInput.approve()); - exception.expect(ResourceConflictException.class); - exception.expectMessage( - String.format( - "invalid account configuration: commit '%s' has an invalid '%s' file for account '%s':" - + " Invalid config file %s in commit %s", - r.getCommit().name(), - AccountProperties.ACCOUNT_CONFIG, - admin.id(), - AccountProperties.ACCOUNT_CONFIG, - r.getCommit().name())); - gApi.changes().id(r.getChangeId()).current().submit(); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, + () -> gApi.changes().id(r.getChangeId()).current().submit()); + assertThat(thrown) + .hasMessageThat() + .contains( + String.format( + "invalid account configuration: commit '%s' has an invalid '%s' file for account" + + " '%s': Invalid config file %s in commit %s", + r.getCommit().name(), + AccountProperties.ACCOUNT_CONFIG, + admin.id(), + AccountProperties.ACCOUNT_CONFIG, + r.getCommit().name())); } @Test @@ -1443,12 +1481,16 @@ public class AccountIT extends AbstractDaemonTest { assertThat(r.getChange().change().getDest().branch()).isEqualTo(userRef); gApi.changes().id(r.getChangeId()).current().review(ReviewInput.approve()); - exception.expect(ResourceConflictException.class); - exception.expectMessage( - String.format( - "invalid account configuration: invalid preferred email '%s' for account '%s'", - noEmail, admin.id())); - gApi.changes().id(r.getChangeId()).current().submit(); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, + () -> gApi.changes().id(r.getChangeId()).current().submit()); + assertThat(thrown) + .hasMessageThat() + .contains( + String.format( + "invalid account configuration: invalid preferred email '%s' for account '%s'", + noEmail, admin.id())); } @Test @@ -1476,9 +1518,13 @@ public class AccountIT extends AbstractDaemonTest { assertThat(r.getChange().change().getDest().branch()).isEqualTo(userRef); gApi.changes().id(r.getChangeId()).current().review(ReviewInput.approve()); - exception.expect(ResourceConflictException.class); - exception.expectMessage("invalid account configuration: cannot deactivate own account"); - gApi.changes().id(r.getChangeId()).current().submit(); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, + () -> gApi.changes().id(r.getChangeId()).current().submit()); + assertThat(thrown) + .hasMessageThat() + .contains("invalid account configuration: cannot deactivate own account"); } @Test @@ -1897,9 +1943,10 @@ public class AccountIT extends AbstractDaemonTest { assertThat(sender.getMessages().get(0).body()).contains("new GPG keys have been added"); requestScopeOperations.setApiUser(user.id()); - exception.expect(ResourceNotFoundException.class); - exception.expectMessage(id); - gApi.accounts().self().gpgKey(id).get(); + ResourceNotFoundException thrown = + assertThrows( + ResourceNotFoundException.class, () -> gApi.accounts().self().gpgKey(id).get()); + assertThat(thrown).hasMessageThat().contains(id); } @Test @@ -1942,9 +1989,9 @@ public class AccountIT extends AbstractDaemonTest { addGpgKey(key.getPublicKeyArmored()); requestScopeOperations.setApiUser(user.id()); - exception.expect(ResourceConflictException.class); - exception.expectMessage("GPG key already associated with another account"); - addGpgKey(key.getPublicKeyArmored()); + ResourceConflictException thrown = + assertThrows(ResourceConflictException.class, () -> addGpgKey(key.getPublicKeyArmored())); + assertThat(thrown).hasMessageThat().contains("GPG key already associated with another account"); } @Test @@ -1972,9 +2019,10 @@ public class AccountIT extends AbstractDaemonTest { accountIndexedCounter.assertReindexOf(admin); assertKeys(); - exception.expect(ResourceNotFoundException.class); - exception.expectMessage(id); - gApi.accounts().self().gpgKey(id).get(); + ResourceNotFoundException thrown = + assertThrows( + ResourceNotFoundException.class, () -> gApi.accounts().self().gpgKey(id).get()); + assertThat(thrown).hasMessageThat().contains(id); } @Test @@ -2008,20 +2056,25 @@ public class AccountIT extends AbstractDaemonTest { assertKeys(key2, key5); accountIndexedCounter.assertReindexOf(admin); - exception.expect(BadRequestException.class); - exception.expectMessage("Cannot both add and delete key: " + keyToString(key2.getPublicKey())); - gApi.accounts() - .self() - .putGpgKeys( - ImmutableList.of(key2.getPublicKeyArmored()), ImmutableList.of(key2.getKeyIdString())); + BadRequestException thrown = + assertThrows( + BadRequestException.class, + () -> + gApi.accounts() + .self() + .putGpgKeys( + ImmutableList.of(key2.getPublicKeyArmored()), + ImmutableList.of(key2.getKeyIdString()))); + assertThat(thrown) + .hasMessageThat() + .contains("Cannot both add and delete key: " + keyToString(key2.getPublicKey())); } @Test public void addMalformedGpgKey() throws Exception { String key = "-----BEGIN PGP PUBLIC KEY BLOCK-----\n\ntest\n-----END PGP PUBLIC KEY BLOCK-----"; - exception.expect(BadRequestException.class); - exception.expectMessage("Failed to parse GPG keys"); - addGpgKey(key); + BadRequestException thrown = assertThrows(BadRequestException.class, () -> addGpgKey(key)); + assertThat(thrown).hasMessageThat().contains("Failed to parse GPG keys"); } @Test @@ -2103,9 +2156,9 @@ public class AccountIT extends AbstractDaemonTest { accountIndexedCounter.assertReindexOf(user); // user cannot reindex any account - exception.expect(AuthException.class); - exception.expectMessage("modify account not permitted"); - gApi.accounts().id(admin.username()).index(); + AuthException thrown = + assertThrows(AuthException.class, () -> gApi.accounts().id(admin.username()).index()); + assertThat(thrown).hasMessageThat().contains("modify account not permitted"); } @Test @@ -2754,22 +2807,23 @@ public class AccountIT extends AbstractDaemonTest { @Test public void userCannotGenerateNewHttpPasswordForOtherUser() throws Exception { requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - gApi.accounts().id(admin.username()).generateHttpPassword(); + assertThrows( + AuthException.class, () -> gApi.accounts().id(admin.username()).generateHttpPassword()); } @Test public void userCannotExplicitlySetHttpPassword() throws Exception { requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - gApi.accounts().self().setHttpPassword("my-new-password"); + assertThrows( + AuthException.class, () -> gApi.accounts().self().setHttpPassword("my-new-password")); } @Test public void userCannotExplicitlySetHttpPasswordForOtherUser() throws Exception { requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - gApi.accounts().id(admin.username()).setHttpPassword("my-new-password"); + assertThrows( + AuthException.class, + () -> gApi.accounts().id(admin.username()).setHttpPassword("my-new-password")); } @Test @@ -2781,8 +2835,8 @@ public class AccountIT extends AbstractDaemonTest { @Test public void userCannotRemoveHttpPasswordForOtherUser() throws Exception { requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - gApi.accounts().id(admin.username()).setHttpPassword(null); + assertThrows( + AuthException.class, () -> gApi.accounts().id(admin.username()).setHttpPassword(null)); } @Test diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AccountManagerIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AccountManagerIT.java index ff6678914f..07bd7eedac 100644 --- a/javatests/com/google/gerrit/acceptance/api/accounts/AccountManagerIT.java +++ b/javatests/com/google/gerrit/acceptance/api/accounts/AccountManagerIT.java @@ -18,6 +18,7 @@ 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.testing.GerritJUnit.assertThrows; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.GerritConfig; @@ -236,9 +237,9 @@ public class AccountManagerIT extends AbstractDaemonTest { } AuthRequest who = AuthRequest.forUser(username); - exception.expect(AccountException.class); - exception.expectMessage("Authentication error, account not found"); - accountManager.authenticate(who); + AccountException thrown = + assertThrows(AccountException.class, () -> accountManager.authenticate(who)); + assertThat(thrown).hasMessageThat().contains("Authentication error, account not found"); } @Test @@ -252,9 +253,9 @@ public class AccountManagerIT extends AbstractDaemonTest { u -> u.setActive(false).addExternalId(ExternalId.create(gerritExtIdKey, accountId))); AuthRequest who = AuthRequest.forUser(username); - exception.expect(AccountException.class); - exception.expectMessage("Authentication error, account inactive"); - accountManager.authenticate(who); + AccountException thrown = + assertThrows(AccountException.class, () -> accountManager.authenticate(who)); + assertThat(thrown).hasMessageThat().contains("Authentication error, account inactive"); } @Test @@ -271,9 +272,9 @@ public class AccountManagerIT extends AbstractDaemonTest { AuthRequest who = AuthRequest.forUser(username); who.setActive(true); who.setAuthProvidesAccountActiveStatus(true); - exception.expect(AccountException.class); - exception.expectMessage("Authentication error, account inactive"); - accountManager.authenticate(who); + AccountException thrown = + assertThrows(AccountException.class, () -> accountManager.authenticate(who)); + assertThat(thrown).hasMessageThat().contains("Authentication error, account inactive"); } @Test @@ -362,9 +363,11 @@ public class AccountManagerIT extends AbstractDaemonTest { // Try to authenticate with this email to create a new account with a SCHEME_MAILTO external ID. // Expect that this fails because the email is already assigned to the other account. AuthRequest who = AuthRequest.forEmail(email); - exception.expect(AccountException.class); - exception.expectMessage("Email 'foo@example.com' in use by another account"); - accountManager.authenticate(who); + AccountException thrown = + assertThrows(AccountException.class, () -> accountManager.authenticate(who)); + assertThat(thrown) + .hasMessageThat() + .contains("Email 'foo@example.com' in use by another account"); } @Test @@ -384,9 +387,11 @@ public class AccountManagerIT extends AbstractDaemonTest { // Expect that this fails because the email is already assigned to the other account. AuthRequest who = AuthRequest.forUser("bar"); who.setEmailAddress(email); - exception.expect(AccountException.class); - exception.expectMessage("Email 'foo@example.com' in use by another account"); - accountManager.authenticate(who); + AccountException thrown = + assertThrows(AccountException.class, () -> accountManager.authenticate(who)); + assertThat(thrown) + .hasMessageThat() + .contains("Email 'foo@example.com' in use by another account"); } @Test @@ -505,9 +510,11 @@ public class AccountManagerIT extends AbstractDaemonTest { // Try to link external ID of the first account to the second account. // Expect that this fails because the external ID is already assigned to the first account. AuthRequest who = AuthRequest.forExternalUser(username1); - exception.expect(AccountException.class); - exception.expectMessage("Identity 'external:foo' in use by another account"); - accountManager.link(accountId2, who); + AccountException thrown = + assertThrows(AccountException.class, () -> accountManager.link(accountId2, who)); + assertThat(thrown) + .hasMessageThat() + .contains("Identity 'external:foo' in use by another account"); } @Test @@ -535,9 +542,11 @@ public class AccountManagerIT extends AbstractDaemonTest { // Try to link the email to the second account (via a new MAILTO external ID) and expect that // this fails because the email is already assigned to the first account. AuthRequest who = AuthRequest.forEmail(email); - exception.expect(AccountException.class); - exception.expectMessage("Email 'foo@example.com' in use by another account"); - accountManager.link(accountId, who); + AccountException thrown = + assertThrows(AccountException.class, () -> accountManager.link(accountId, who)); + assertThat(thrown) + .hasMessageThat() + .contains("Email 'foo@example.com' in use by another account"); } private void assertNoSuchExternalIds(ExternalId.Key... extIdKeys) throws Exception { diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java index 67f4151468..382b24ba4e 100644 --- a/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java +++ b/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java @@ -16,6 +16,7 @@ package com.google.gerrit.acceptance.api.accounts; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.TruthJUnit.assume; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.concurrent.TimeUnit.SECONDS; @@ -145,17 +146,21 @@ public class AgreementsIT extends AbstractDaemonTest { @Test public void signNonExistingAgreement() throws Exception { assume().that(isContributorAgreementsEnabled()).isTrue(); - exception.expect(UnprocessableEntityException.class); - exception.expectMessage("contributor agreement not found"); - gApi.accounts().self().signAgreement("does-not-exist"); + UnprocessableEntityException thrown = + assertThrows( + UnprocessableEntityException.class, + () -> gApi.accounts().self().signAgreement("does-not-exist")); + assertThat(thrown).hasMessageThat().contains("contributor agreement not found"); } @Test public void signAgreementNoAutoVerify() throws Exception { assume().that(isContributorAgreementsEnabled()).isTrue(); - exception.expect(BadRequestException.class); - exception.expectMessage("cannot enter a non-autoVerify agreement"); - gApi.accounts().self().signAgreement(caNoAutoVerify.getName()); + BadRequestException thrown = + assertThrows( + BadRequestException.class, + () -> gApi.accounts().self().signAgreement(caNoAutoVerify.getName())); + assertThat(thrown).hasMessageThat().contains("cannot enter a non-autoVerify agreement"); } @Test @@ -188,33 +193,40 @@ public class AgreementsIT extends AbstractDaemonTest { public void signAgreementAsOtherUser() throws Exception { assume().that(isContributorAgreementsEnabled()).isTrue(); assertThat(gApi.accounts().self().get().name).isNotEqualTo("admin"); - exception.expect(AuthException.class); - exception.expectMessage("not allowed to enter contributor agreement"); - gApi.accounts().id("admin").signAgreement(caAutoVerify.getName()); + AuthException thrown = + assertThrows( + AuthException.class, + () -> gApi.accounts().id("admin").signAgreement(caAutoVerify.getName())); + assertThat(thrown).hasMessageThat().contains("not allowed to enter contributor agreement"); } @Test public void signAgreementAnonymous() throws Exception { requestScopeOperations.setApiUserAnonymous(); - exception.expect(AuthException.class); - exception.expectMessage("Authentication required"); - gApi.accounts().self().signAgreement(caAutoVerify.getName()); + AuthException thrown = + assertThrows( + AuthException.class, + () -> gApi.accounts().self().signAgreement(caAutoVerify.getName())); + assertThat(thrown).hasMessageThat().contains("Authentication required"); } @Test public void agreementsDisabledSign() throws Exception { assume().that(isContributorAgreementsEnabled()).isFalse(); - exception.expect(MethodNotAllowedException.class); - exception.expectMessage("contributor agreements disabled"); - gApi.accounts().self().signAgreement(caAutoVerify.getName()); + MethodNotAllowedException thrown = + assertThrows( + MethodNotAllowedException.class, + () -> gApi.accounts().self().signAgreement(caAutoVerify.getName())); + assertThat(thrown).hasMessageThat().contains("contributor agreements disabled"); } @Test public void agreementsDisabledList() throws Exception { assume().that(isContributorAgreementsEnabled()).isFalse(); - exception.expect(MethodNotAllowedException.class); - exception.expectMessage("contributor agreements disabled"); - gApi.accounts().self().listAgreements(); + MethodNotAllowedException thrown = + assertThrows( + MethodNotAllowedException.class, () -> gApi.accounts().self().listAgreements()); + assertThat(thrown).hasMessageThat().contains("contributor agreements disabled"); } @Test @@ -233,9 +245,9 @@ public class AgreementsIT extends AbstractDaemonTest { // Revert is not allowed when CLA is required but not signed requestScopeOperations.setApiUser(user.id()); setUseContributorAgreements(InheritableBoolean.TRUE); - exception.expect(AuthException.class); - exception.expectMessage("Contributor Agreement"); - gApi.changes().id(change.changeId).revert(); + AuthException thrown = + assertThrows(AuthException.class, () -> gApi.changes().id(change.changeId).revert()); + assertThat(thrown).hasMessageThat().contains("Contributor Agreement"); } @Test @@ -287,9 +299,10 @@ public class AgreementsIT extends AbstractDaemonTest { CherryPickInput in = new CherryPickInput(); in.destination = dest.ref; in.message = change.subject; - exception.expect(AuthException.class); - exception.expectMessage("Contributor Agreement"); - gApi.changes().id(change.changeId).current().cherryPick(in); + AuthException thrown = + assertThrows( + AuthException.class, () -> gApi.changes().id(change.changeId).current().cherryPick(in)); + assertThat(thrown).hasMessageThat().contains("Contributor Agreement"); } @Test diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/GeneralPreferencesIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/GeneralPreferencesIT.java index 70e37ef5d3..12266c9ceb 100644 --- a/javatests/com/google/gerrit/acceptance/api/accounts/GeneralPreferencesIT.java +++ b/javatests/com/google/gerrit/acceptance/api/accounts/GeneralPreferencesIT.java @@ -16,6 +16,7 @@ package com.google.gerrit.acceptance.api.accounts; import static com.google.common.truth.Truth.assertThat; import static com.google.gerrit.acceptance.AssertUtil.assertPrefs; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.NoHttpd; @@ -160,9 +161,11 @@ public class GeneralPreferencesIT extends AbstractDaemonTest { i.my = new ArrayList<>(); i.my.add(new MenuItem(null, "url")); - exception.expect(BadRequestException.class); - exception.expectMessage("name for menu item is required"); - gApi.accounts().id(user42.id().toString()).setPreferences(i); + BadRequestException thrown = + assertThrows( + BadRequestException.class, + () -> gApi.accounts().id(user42.id().toString()).setPreferences(i)); + assertThat(thrown).hasMessageThat().contains("name for menu item is required"); } @Test @@ -171,9 +174,11 @@ public class GeneralPreferencesIT extends AbstractDaemonTest { i.my = new ArrayList<>(); i.my.add(new MenuItem("name", null)); - exception.expect(BadRequestException.class); - exception.expectMessage("URL for menu item is required"); - gApi.accounts().id(user42.id().toString()).setPreferences(i); + BadRequestException thrown = + assertThrows( + BadRequestException.class, + () -> gApi.accounts().id(user42.id().toString()).setPreferences(i)); + assertThat(thrown).hasMessageThat().contains("URL for menu item is required"); } @Test @@ -191,9 +196,13 @@ public class GeneralPreferencesIT extends AbstractDaemonTest { GeneralPreferencesInfo i = GeneralPreferencesInfo.defaults(); i.downloadScheme = "foo"; - exception.expect(BadRequestException.class); - exception.expectMessage("Unsupported download scheme: " + i.downloadScheme); - gApi.accounts().id(user42.id().toString()).setPreferences(i); + BadRequestException thrown = + assertThrows( + BadRequestException.class, + () -> gApi.accounts().id(user42.id().toString()).setPreferences(i)); + assertThat(thrown) + .hasMessageThat() + .contains("Unsupported download scheme: " + i.downloadScheme); } @Test diff --git a/javatests/com/google/gerrit/acceptance/api/change/AbandonIT.java b/javatests/com/google/gerrit/acceptance/api/change/AbandonIT.java index 27cb2e96fe..6a116d8592 100644 --- a/javatests/com/google/gerrit/acceptance/api/change/AbandonIT.java +++ b/javatests/com/google/gerrit/acceptance/api/change/AbandonIT.java @@ -17,6 +17,7 @@ package com.google.gerrit.acceptance.api.change; import static com.google.common.truth.Truth.assertThat; import static com.google.gerrit.extensions.client.ListChangesOption.MESSAGES; import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static java.util.concurrent.TimeUnit.HOURS; import static java.util.concurrent.TimeUnit.SECONDS; import static java.util.stream.Collectors.toList; @@ -59,9 +60,9 @@ public class AbandonIT extends AbstractDaemonTest { assertThat(info.status).isEqualTo(ChangeStatus.ABANDONED); assertThat(Iterables.getLast(info.messages).message.toLowerCase()).contains("abandoned"); - exception.expect(ResourceConflictException.class); - exception.expectMessage("change is abandoned"); - gApi.changes().id(changeId).abandon(); + ResourceConflictException thrown = + assertThrows(ResourceConflictException.class, () -> gApi.changes().id(changeId).abandon()); + assertThat(thrown).hasMessageThat().contains("change is abandoned"); } @Test @@ -96,10 +97,16 @@ public class AbandonIT extends AbstractDaemonTest { PushOneCommit.Result a = createChange(project1, "master", "x", "x", "x", ""); PushOneCommit.Result b = createChange(project2, "master", "x", "x", "x", ""); List list = ImmutableList.of(a.getChange(), b.getChange()); - exception.expect(ResourceConflictException.class); - exception.expectMessage( - String.format("Project name \"%s\" doesn't match \"%s\"", project2Name, project1Name)); - batchAbandon.batchAbandon(batchUpdateFactory, Project.nameKey(project1Name), user, list); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, + () -> + batchAbandon.batchAbandon( + batchUpdateFactory, Project.nameKey(project1Name), user, list)); + assertThat(thrown) + .hasMessageThat() + .contains( + String.format("Project name \"%s\" doesn't match \"%s\"", project2Name, project1Name)); } @Test @@ -157,9 +164,9 @@ public class AbandonIT extends AbstractDaemonTest { String changeId = r.getChangeId(); assertThat(info(changeId).status).isEqualTo(ChangeStatus.NEW); requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - exception.expectMessage("abandon not permitted"); - gApi.changes().id(changeId).abandon(); + AuthException thrown = + assertThrows(AuthException.class, () -> gApi.changes().id(changeId).abandon()); + assertThat(thrown).hasMessageThat().contains("abandon not permitted"); } @Test @@ -188,9 +195,9 @@ public class AbandonIT extends AbstractDaemonTest { assertThat(info.status).isEqualTo(ChangeStatus.NEW); assertThat(Iterables.getLast(info.messages).message.toLowerCase()).contains("restored"); - exception.expect(ResourceConflictException.class); - exception.expectMessage("change is new"); - gApi.changes().id(changeId).restore(); + ResourceConflictException thrown = + assertThrows(ResourceConflictException.class, () -> gApi.changes().id(changeId).restore()); + assertThat(thrown).hasMessageThat().contains("change is new"); } @Test @@ -201,9 +208,9 @@ public class AbandonIT extends AbstractDaemonTest { gApi.changes().id(changeId).abandon(); requestScopeOperations.setApiUser(user.id()); assertThat(info(changeId).status).isEqualTo(ChangeStatus.ABANDONED); - exception.expect(AuthException.class); - exception.expectMessage("restore not permitted"); - gApi.changes().id(changeId).restore(); + AuthException thrown = + assertThrows(AuthException.class, () -> gApi.changes().id(changeId).restore()); + assertThat(thrown).hasMessageThat().contains("restore not permitted"); } private List toChangeNumbers(List changes) { diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java index 18a7eb231d..0fed39b342 100644 --- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java +++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java @@ -47,6 +47,7 @@ import static com.google.gerrit.server.group.SystemGroupBackend.PROJECT_OWNERS; import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS; import static com.google.gerrit.server.project.testing.Util.category; import static com.google.gerrit.server.project.testing.Util.value; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.concurrent.TimeUnit.SECONDS; import static java.util.stream.Collectors.joining; @@ -276,9 +277,9 @@ public class ChangeIT extends AbstractDaemonTest { String changeId = rwip.getChangeId(); requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - exception.expectMessage("toggle work in progress state not permitted"); - gApi.changes().id(changeId).setWorkInProgress(); + AuthException thrown = + assertThrows(AuthException.class, () -> gApi.changes().id(changeId).setWorkInProgress()); + assertThat(thrown).hasMessageThat().contains("toggle work in progress state not permitted"); } @Test @@ -322,9 +323,9 @@ public class ChangeIT extends AbstractDaemonTest { gApi.changes().id(changeId).setWorkInProgress(); requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - exception.expectMessage("toggle work in progress state not permitted"); - gApi.changes().id(changeId).setReadyForReview(); + AuthException thrown = + assertThrows(AuthException.class, () -> gApi.changes().id(changeId).setReadyForReview()); + assertThat(thrown).hasMessageThat().contains("toggle work in progress state not permitted"); } @Test @@ -621,9 +622,10 @@ public class ChangeIT extends AbstractDaemonTest { PushOneCommit.Result r = createChange(); ReviewInput in = ReviewInput.noScore().setWorkInProgress(true); requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - exception.expectMessage("toggle work in progress state not permitted"); - gApi.changes().id(r.getChangeId()).current().review(in); + AuthException thrown = + assertThrows( + AuthException.class, () -> gApi.changes().id(r.getChangeId()).current().review(in)); + assertThat(thrown).hasMessageThat().contains("toggle work in progress state not permitted"); } @Test @@ -647,9 +649,10 @@ public class ChangeIT extends AbstractDaemonTest { PushOneCommit.Result r = createChange(); ReviewInput in = ReviewInput.noScore().setReady(true); requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - exception.expectMessage("toggle work in progress state not permitted"); - gApi.changes().id(r.getChangeId()).current().review(in); + AuthException thrown = + assertThrows( + AuthException.class, () -> gApi.changes().id(r.getChangeId()).current().review(in)); + assertThat(thrown).hasMessageThat().contains("toggle work in progress state not permitted"); } @Test @@ -673,9 +676,9 @@ public class ChangeIT extends AbstractDaemonTest { PushOneCommit.Result r2 = push2.to("refs/for/other"); assertThat(r2.getChangeId()).isEqualTo(changeId); - exception.expect(ResourceNotFoundException.class); - exception.expectMessage("Multiple changes found for " + changeId); - gApi.changes().id(changeId).get(); + ResourceNotFoundException thrown = + assertThrows(ResourceNotFoundException.class, () -> gApi.changes().id(changeId).get()); + assertThat(thrown).hasMessageThat().contains("Multiple changes found for " + changeId); } @Test @@ -769,9 +772,10 @@ public class ChangeIT extends AbstractDaemonTest { gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).review(ReviewInput.approve()); gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).submit(); - exception.expect(ResourceConflictException.class); - exception.expectMessage("Cannot revert initial commit"); - gApi.changes().id(r.getChangeId()).revert(); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, () -> gApi.changes().id(r.getChangeId()).revert()); + assertThat(thrown).hasMessageThat().contains("Cannot revert initial commit"); } @FunctionalInterface @@ -826,9 +830,10 @@ public class ChangeIT extends AbstractDaemonTest { assertThat(cr.all.get(0).value).isEqualTo(1); // Rebasing the second change again should fail - exception.expect(ResourceConflictException.class); - exception.expectMessage("Change is already up to date"); - gApi.changes().id(changeId).current().rebase(); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, () -> gApi.changes().id(changeId).current().rebase()); + assertThat(thrown).hasMessageThat().contains("Change is already up to date"); } @Test @@ -931,9 +936,9 @@ public class ChangeIT extends AbstractDaemonTest { // Rebase the second String changeId = r2.getChangeId(); requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - exception.expectMessage("rebase not permitted"); - gApi.changes().id(changeId).rebase(); + AuthException thrown = + assertThrows(AuthException.class, () -> gApi.changes().id(changeId).rebase()); + assertThat(thrown).hasMessageThat().contains("rebase not permitted"); } @Test @@ -974,9 +979,9 @@ public class ChangeIT extends AbstractDaemonTest { // Rebase the second String changeId = r2.getChangeId(); requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - exception.expectMessage("rebase not permitted"); - gApi.changes().id(changeId).rebase(); + AuthException thrown = + assertThrows(AuthException.class, () -> gApi.changes().id(changeId).rebase()); + assertThat(thrown).hasMessageThat().contains("rebase not permitted"); } @Test @@ -995,9 +1000,9 @@ public class ChangeIT extends AbstractDaemonTest { // Rebase the second String changeId = r2.getChangeId(); - exception.expect(AuthException.class); - exception.expectMessage("rebase not permitted"); - gApi.changes().id(changeId).rebase(); + AuthException thrown = + assertThrows(AuthException.class, () -> gApi.changes().id(changeId).rebase()); + assertThat(thrown).hasMessageThat().contains("rebase not permitted"); } @Test @@ -1013,9 +1018,9 @@ public class ChangeIT extends AbstractDaemonTest { String changeId = changeResult.getChangeId(); requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - exception.expectMessage("delete not permitted"); - gApi.changes().id(changeId).delete(); + AuthException thrown = + assertThrows(AuthException.class, () -> gApi.changes().id(changeId).delete()); + assertThat(thrown).hasMessageThat().contains("delete not permitted"); } @Test @@ -1111,9 +1116,9 @@ public class ChangeIT extends AbstractDaemonTest { String changeId = changeResult.getChangeId(); requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - exception.expectMessage("delete not permitted"); - gApi.changes().id(changeId).delete(); + AuthException thrown = + assertThrows(AuthException.class, () -> gApi.changes().id(changeId).delete()); + assertThat(thrown).hasMessageThat().contains("delete not permitted"); } finally { removePermission(project, "refs/*", Permission.DELETE_OWN_CHANGES); } @@ -1140,9 +1145,9 @@ public class ChangeIT extends AbstractDaemonTest { requestScopeOperations.setApiUser(user.id()); gApi.changes().id(changeId).abandon(); - exception.expect(AuthException.class); - exception.expectMessage("delete not permitted"); - gApi.changes().id(changeId).delete(); + AuthException thrown = + assertThrows(AuthException.class, () -> gApi.changes().id(changeId).delete()); + assertThat(thrown).hasMessageThat().contains("delete not permitted"); } @Test @@ -1166,9 +1171,9 @@ public class ChangeIT extends AbstractDaemonTest { merge(changeResult); - exception.expect(MethodNotAllowedException.class); - exception.expectMessage("delete not permitted"); - gApi.changes().id(changeId).delete(); + MethodNotAllowedException thrown = + assertThrows(MethodNotAllowedException.class, () -> gApi.changes().id(changeId).delete()); + assertThat(thrown).hasMessageThat().contains("delete not permitted"); } @Test @@ -1184,9 +1189,9 @@ public class ChangeIT extends AbstractDaemonTest { merge(changeResult); requestScopeOperations.setApiUser(user.id()); - exception.expect(MethodNotAllowedException.class); - exception.expectMessage("delete not permitted"); - gApi.changes().id(changeId).delete(); + MethodNotAllowedException thrown = + assertThrows(MethodNotAllowedException.class, () -> gApi.changes().id(changeId).delete()); + assertThat(thrown).hasMessageThat().contains("delete not permitted"); } finally { removePermission(project, "refs/*", Permission.DELETE_OWN_CHANGES); } @@ -1201,10 +1206,11 @@ public class ChangeIT extends AbstractDaemonTest { merge(changeResult); setChangeStatus(id, Change.Status.NEW); - exception.expect(ResourceConflictException.class); - exception.expectMessage( - String.format("Cannot delete change %s: patch set 1 is already merged", id)); - gApi.changes().id(changeId).delete(); + ResourceConflictException thrown = + assertThrows(ResourceConflictException.class, () -> gApi.changes().id(changeId).delete()); + assertThat(thrown) + .hasMessageThat() + .contains(String.format("Cannot delete change %s: patch set 1 is already merged", id)); } @Test @@ -1255,16 +1261,21 @@ public class ChangeIT extends AbstractDaemonTest { @Test public void rebaseUpToDateChange() throws Exception { PushOneCommit.Result r = createChange(); - exception.expect(ResourceConflictException.class); - exception.expectMessage("Change is already up to date"); - gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).rebase(); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, + () -> gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).rebase()); + assertThat(thrown).hasMessageThat().contains("Change is already up to date"); } @Test public void rebaseConflict() throws Exception { - PushOneCommit.Result r = createChange(); - gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).review(ReviewInput.approve()); - gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).submit(); + PushOneCommit.Result r1 = createChange(); + gApi.changes() + .id(r1.getChangeId()) + .revision(r1.getCommit().name()) + .review(ReviewInput.approve()); + gApi.changes().id(r1.getChangeId()).revision(r1.getCommit().name()).submit(); PushOneCommit push = pushFactory.create( @@ -1274,11 +1285,11 @@ public class ChangeIT extends AbstractDaemonTest { PushOneCommit.FILE_NAME, "other content", "If09d8782c1e59dd0b33de2b1ec3595d69cc10ad5"); - r = push.to("refs/for/master"); - r.assertOkStatus(); - - exception.expect(ResourceConflictException.class); - gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).rebase(); + PushOneCommit.Result r2 = push.to("refs/for/master"); + r2.assertOkStatus(); + assertThrows( + ResourceConflictException.class, + () -> gApi.changes().id(r2.getChangeId()).revision(r2.getCommit().name()).rebase()); } @Test @@ -1323,9 +1334,11 @@ public class ChangeIT extends AbstractDaemonTest { "base change " + r2.getChangeId() + " is a descendant of the current change - recursion not allowed"; - exception.expect(ResourceConflictException.class); - exception.expectMessage(expectedMessage); - gApi.changes().id(r1.getChangeId()).revision(r1.getCommit().name()).rebase(ri); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, + () -> gApi.changes().id(r1.getChangeId()).revision(r1.getCommit().name()).rebase(ri)); + assertThat(thrown).hasMessageThat().contains(expectedMessage); } @Test @@ -1337,9 +1350,11 @@ public class ChangeIT extends AbstractDaemonTest { ChangeInfo info = info(changeId); assertThat(info.status).isEqualTo(ChangeStatus.ABANDONED); - exception.expect(ResourceConflictException.class); - exception.expectMessage("change is abandoned"); - gApi.changes().id(changeId).revision(r.getCommit().name()).rebase(); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, + () -> gApi.changes().id(changeId).revision(r.getCommit().name()).rebase()); + assertThat(thrown).hasMessageThat().contains("change is abandoned"); } @Test @@ -1359,9 +1374,11 @@ public class ChangeIT extends AbstractDaemonTest { RebaseInput ri = new RebaseInput(); ri.base = r.getCommit().name(); - exception.expect(ResourceConflictException.class); - exception.expectMessage("base change is abandoned: " + changeId); - gApi.changes().id(r2.getChangeId()).revision(r2.getCommit().name()).rebase(ri); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, + () -> gApi.changes().id(r2.getChangeId()).revision(r2.getCommit().name()).rebase(ri)); + assertThat(thrown).hasMessageThat().contains("base change is abandoned: " + changeId); } @Test @@ -1371,9 +1388,11 @@ public class ChangeIT extends AbstractDaemonTest { String commit = r.getCommit().name(); RebaseInput ri = new RebaseInput(); ri.base = commit; - exception.expect(ResourceConflictException.class); - exception.expectMessage("cannot rebase change onto itself"); - gApi.changes().id(changeId).revision(commit).rebase(ri); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, + () -> gApi.changes().id(changeId).revision(commit).rebase(ri)); + assertThat(thrown).hasMessageThat().contains("cannot rebase change onto itself"); } @Test @@ -2056,8 +2075,8 @@ public class ChangeIT extends AbstractDaemonTest { comment.message = "comment 1"; review.comments = ImmutableMap.of(comment.path, Lists.newArrayList(comment)); - exception.expect(BadRequestException.class); - gApi.changes().id(changeId).current().review(review); + assertThrows( + BadRequestException.class, () -> gApi.changes().id(changeId).current().review(review)); } @Test @@ -2124,8 +2143,9 @@ public class ChangeIT extends AbstractDaemonTest { // Remove again, and then try to remove once more to verify 404 is // returned. gApi.changes().id(changeId).reviewer(user.id().toString()).remove(); - exception.expect(ResourceNotFoundException.class); - gApi.changes().id(changeId).reviewer(user.id().toString()).remove(); + assertThrows( + ResourceNotFoundException.class, + () -> gApi.changes().id(changeId).reviewer(user.id().toString()).remove()); } @Test @@ -2186,9 +2206,11 @@ public class ChangeIT extends AbstractDaemonTest { gApi.changes().id(changeId).revision(r.getCommit().name()).review(ReviewInput.approve()); requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - exception.expectMessage("remove reviewer not permitted"); - gApi.changes().id(r.getChangeId()).reviewer(admin.id().toString()).remove(); + AuthException thrown = + assertThrows( + AuthException.class, + () -> gApi.changes().id(r.getChangeId()).reviewer(admin.id().toString()).remove()); + assertThat(thrown).hasMessageThat().contains("remove reviewer not permitted"); } @Test @@ -2204,9 +2226,11 @@ public class ChangeIT extends AbstractDaemonTest { gApi.changes().id(changeId).revision(r.getCommit().name()).submit(); requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - exception.expectMessage("remove reviewer not permitted"); - gApi.changes().id(r.getChangeId()).reviewer("self").remove(); + AuthException thrown = + assertThrows( + AuthException.class, + () -> gApi.changes().id(r.getChangeId()).reviewer("self").remove()); + assertThat(thrown).hasMessageThat().contains("remove reviewer not permitted"); } @Test @@ -2238,9 +2262,11 @@ public class ChangeIT extends AbstractDaemonTest { gApi.changes().id(changeId).abandon(); requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - exception.expectMessage("remove reviewer not permitted"); - gApi.changes().id(r.getChangeId()).reviewer(admin.id().toString()).remove(); + AuthException thrown = + assertThrows( + AuthException.class, + () -> gApi.changes().id(r.getChangeId()).reviewer(admin.id().toString()).remove()); + assertThat(thrown).hasMessageThat().contains("remove reviewer not permitted"); } @Test @@ -2348,9 +2374,15 @@ public class ChangeIT extends AbstractDaemonTest { gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).review(ReviewInput.approve()); requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - exception.expectMessage("delete vote not permitted"); - gApi.changes().id(r.getChangeId()).reviewer(admin.id().toString()).deleteVote("Code-Review"); + AuthException thrown = + assertThrows( + AuthException.class, + () -> + gApi.changes() + .id(r.getChangeId()) + .reviewer(admin.id().toString()) + .deleteVote("Code-Review")); + assertThat(thrown).hasMessageThat().contains("delete vote not permitted"); } @Test @@ -2600,9 +2632,10 @@ public class ChangeIT extends AbstractDaemonTest { PushOneCommit.Result r = createChange(); assertThat(gApi.changes().id(r.getChangeId()).topic()).isEqualTo(""); requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - exception.expectMessage("edit topic name not permitted"); - gApi.changes().id(r.getChangeId()).topic("mytopic"); + AuthException thrown = + assertThrows( + AuthException.class, () -> gApi.changes().id(r.getChangeId()).topic("mytopic")); + assertThat(thrown).hasMessageThat().contains("edit topic name not permitted"); } @Test @@ -2655,9 +2688,11 @@ public class ChangeIT extends AbstractDaemonTest { PushOneCommit.Result r = createChange(); gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).review(ReviewInput.approve()); requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - exception.expectMessage("submit not permitted"); - gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).submit(); + AuthException thrown = + assertThrows( + AuthException.class, + () -> gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).submit()); + assertThat(thrown).hasMessageThat().contains("submit not permitted"); } @Test @@ -2867,9 +2902,9 @@ public class ChangeIT extends AbstractDaemonTest { info = gApi.changes().id(info._number).get(); assertThat(info.changeId).isEqualTo(r.getChangeId()); - - exception.expect(AuthException.class); - gApi.changes().id(triplet).current().review(ReviewInput.approve()); + assertThrows( + AuthException.class, + () -> gApi.changes().id(triplet).current().review(ReviewInput.approve())); } @Test @@ -2926,8 +2961,7 @@ public class ChangeIT extends AbstractDaemonTest { in.project = project.get(); in.newBranch = true; - exception.expect(ResourceConflictException.class); - gApi.changes().create(in).get(); + assertThrows(ResourceConflictException.class, () -> gApi.changes().create(in).get()); } @Test @@ -3095,9 +3129,14 @@ public class ChangeIT extends AbstractDaemonTest { testRepo.reset(initialHead); String changeId = createChange().getChangeId(); - exception.expect(UnprocessableEntityException.class); - exception.expectMessage("Read not permitted for " + baseChange); - gApi.changes().id(changeId).createMergePatchSet(createMergePatchSetInput(baseChange)); + UnprocessableEntityException thrown = + assertThrows( + UnprocessableEntityException.class, + () -> + gApi.changes() + .id(changeId) + .createMergePatchSet(createMergePatchSetInput(baseChange))); + assertThat(thrown).hasMessageThat().contains("Read not permitted for " + baseChange); } @Test @@ -3434,9 +3473,10 @@ public class ChangeIT extends AbstractDaemonTest { String changeId = createChange().getChangeId(); ReviewInput in = new ReviewInput().label("Code-Style", 1); - exception.expect(BadRequestException.class); - exception.expectMessage("label \"Code-Style\" is not a configured label"); - gApi.changes().id(changeId).current().review(in); + BadRequestException thrown = + assertThrows( + BadRequestException.class, () -> gApi.changes().id(changeId).current().review(in)); + assertThat(thrown).hasMessageThat().contains("label \"Code-Style\" is not a configured label"); } @Test @@ -3445,9 +3485,10 @@ public class ChangeIT extends AbstractDaemonTest { String changeId = createChange().getChangeId(); ReviewInput in = new ReviewInput().label("Code-Review", 3); - exception.expect(BadRequestException.class); - exception.expectMessage("label \"Code-Review\": 3 is not a valid value"); - gApi.changes().id(changeId).current().review(in); + BadRequestException thrown = + assertThrows( + BadRequestException.class, () -> gApi.changes().id(changeId).current().review(in)); + assertThat(thrown).hasMessageThat().contains("label \"Code-Review\": 3 is not a valid value"); } @Test @@ -3475,10 +3516,14 @@ public class ChangeIT extends AbstractDaemonTest { gApi.changes().id(result1.getChangeId()).current().submit(); - exception.expect(ResourceConflictException.class); - exception.expectMessage("Failed to submit 1 change due to the following problems"); - exception.expectMessage("needs All-Comments-Resolved"); - gApi.changes().id(result2.getChangeId()).current().submit(); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, + () -> gApi.changes().id(result2.getChangeId()).current().submit()); + assertThat(thrown) + .hasMessageThat() + .contains("Failed to submit 1 change due to the following problems"); + assertThat(thrown).hasMessageThat().contains("needs All-Comments-Resolved"); } @Test @@ -3489,10 +3534,14 @@ public class ChangeIT extends AbstractDaemonTest { PushOneCommit.Result r1 = pushFactory.create(user.newIdent(), testRepo).to("refs/for/master"); approve(r1.getChangeId()); - exception.expect(ResourceConflictException.class); - exception.expectMessage("Failed to submit 1 change due to the following problems"); - exception.expectMessage("needs Is-Pure-Revert"); - gApi.changes().id(r1.getChangeId()).current().submit(); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, + () -> gApi.changes().id(r1.getChangeId()).current().submit()); + assertThat(thrown) + .hasMessageThat() + .contains("Failed to submit 1 change due to the following problems"); + assertThat(thrown).hasMessageThat().contains("needs Is-Pure-Revert"); } @Test @@ -3507,10 +3556,13 @@ public class ChangeIT extends AbstractDaemonTest { amendChange(revertId); approve(revertId); - exception.expect(ResourceConflictException.class); - exception.expectMessage("Failed to submit 1 change due to the following problems"); - exception.expectMessage("needs Is-Pure-Revert"); - gApi.changes().id(revertId).current().submit(); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, () -> gApi.changes().id(revertId).current().submit()); + assertThat(thrown) + .hasMessageThat() + .contains("Failed to submit 1 change due to the following problems"); + assertThat(thrown).hasMessageThat().contains("needs Is-Pure-Revert"); } @Test @@ -3588,9 +3640,11 @@ public class ChangeIT extends AbstractDaemonTest { PushOneCommit.Result r = createChange(); assertThat(getCommitMessage(r.getChangeId())) .isEqualTo("test commit\n\nChange-Id: " + r.getChangeId() + "\n"); - exception.expect(ResourceConflictException.class); - exception.expectMessage("missing Change-Id footer"); - gApi.changes().id(r.getChangeId()).setMessage("modified commit\n"); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, + () -> gApi.changes().id(r.getChangeId()).setMessage("modified commit\n")); + assertThat(thrown).hasMessageThat().contains("missing Change-Id footer"); } @Test @@ -3598,11 +3652,14 @@ public class ChangeIT extends AbstractDaemonTest { PushOneCommit.Result r = createChange(); assertThat(getCommitMessage(r.getChangeId())) .isEqualTo("test commit\n\nChange-Id: " + r.getChangeId() + "\n"); - exception.expect(BadRequestException.class); - exception.expectMessage("NUL character"); - gApi.changes() - .id(r.getChangeId()) - .setMessage("test\0commit\n\nChange-Id: " + r.getChangeId() + "\n"); + BadRequestException thrown = + assertThrows( + BadRequestException.class, + () -> + gApi.changes() + .id(r.getChangeId()) + .setMessage("test\0commit\n\nChange-Id: " + r.getChangeId() + "\n")); + assertThat(thrown).hasMessageThat().contains("NUL character"); } @Test @@ -3611,11 +3668,15 @@ public class ChangeIT extends AbstractDaemonTest { PushOneCommit.Result r = createChange(); assertThat(getCommitMessage(r.getChangeId())) .isEqualTo("test commit\n\nChange-Id: " + r.getChangeId() + "\n"); - exception.expect(ResourceConflictException.class); - exception.expectMessage("wrong Change-Id footer"); - gApi.changes() - .id(r.getChangeId()) - .setMessage("modified commit\n\nChange-Id: " + otherChange.getChangeId() + "\n"); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, + () -> + gApi.changes() + .id(r.getChangeId()) + .setMessage( + "modified commit\n\nChange-Id: " + otherChange.getChangeId() + "\n")); + assertThat(thrown).hasMessageThat().contains("wrong Change-Id footer"); } @Test @@ -3630,9 +3691,10 @@ public class ChangeIT extends AbstractDaemonTest { PushOneCommit.Result r = push.to("refs/for/master"); r.assertOkStatus(); // Try to change the commit message - exception.expect(AuthException.class); - exception.expectMessage("modifying commit message not permitted"); - gApi.changes().id(r.getChangeId()).setMessage("foo"); + AuthException thrown = + assertThrows( + AuthException.class, () -> gApi.changes().id(r.getChangeId()).setMessage("foo")); + assertThat(thrown).hasMessageThat().contains("modifying commit message not permitted"); } @Test @@ -3640,9 +3702,11 @@ public class ChangeIT extends AbstractDaemonTest { PushOneCommit.Result r = createChange(); assertThat(getCommitMessage(r.getChangeId())) .isEqualTo("test commit\n\nChange-Id: " + r.getChangeId() + "\n"); - exception.expect(ResourceConflictException.class); - exception.expectMessage("new and existing commit message are the same"); - gApi.changes().id(r.getChangeId()).setMessage(getCommitMessage(r.getChangeId())); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, + () -> gApi.changes().id(r.getChangeId()).setMessage(getCommitMessage(r.getChangeId()))); + assertThat(thrown).hasMessageThat().contains("new and existing commit message are the same"); } @Test @@ -3730,9 +3794,11 @@ public class ChangeIT extends AbstractDaemonTest { PushOneCommit.Result r1 = createChange(); merge(r1); - exception.expect(BadRequestException.class); - exception.expectMessage("invalid object ID"); - gApi.changes().id(createChange().getChangeId()).pureRevert("invalid id"); + BadRequestException thrown = + assertThrows( + BadRequestException.class, + () -> gApi.changes().id(createChange().getChangeId()).pureRevert("invalid id")); + assertThat(thrown).hasMessageThat().contains("invalid object ID"); } @Test @@ -3777,9 +3843,11 @@ public class ChangeIT extends AbstractDaemonTest { @Test public void pureRevertThrowsExceptionWhenChangeIsNotARevertAndNoIdProvided() throws Exception { - exception.expect(BadRequestException.class); - exception.expectMessage("revertOf not set"); - gApi.changes().id(createChange().getChangeId()).pureRevert(); + BadRequestException thrown = + assertThrows( + BadRequestException.class, + () -> gApi.changes().id(createChange().getChangeId()).pureRevert()); + assertThat(thrown).hasMessageThat().contains("revertOf not set"); } @Test @@ -3787,9 +3855,9 @@ public class ChangeIT extends AbstractDaemonTest { String changeId = createChange().getChangeId(); String topic = Stream.generate(() -> "t").limit(2049).collect(joining()); - exception.expect(BadRequestException.class); - exception.expectMessage("topic length exceeds the limit"); - gApi.changes().id(changeId).topic(topic); + BadRequestException thrown = + assertThrows(BadRequestException.class, () -> gApi.changes().id(changeId).topic(topic)); + assertThat(thrown).hasMessageThat().contains("topic length exceeds the limit"); } @Test @@ -4065,9 +4133,9 @@ public class ChangeIT extends AbstractDaemonTest { public void cannotIgnoreOwnChange() throws Exception { String changeId = createChange().getChangeId(); - exception.expect(BadRequestException.class); - exception.expectMessage("cannot ignore own change"); - gApi.changes().id(changeId).ignore(true); + BadRequestException thrown = + assertThrows(BadRequestException.class, () -> gApi.changes().id(changeId).ignore(true)); + assertThat(thrown).hasMessageThat().contains("cannot ignore own change"); } @Test @@ -4078,14 +4146,17 @@ public class ChangeIT extends AbstractDaemonTest { gApi.accounts().self().starChange(changeId); assertThat(gApi.changes().id(changeId).get().starred).isTrue(); - exception.expect(ResourceConflictException.class); - exception.expectMessage( - "The labels " - + StarredChangesUtil.DEFAULT_LABEL - + " and " - + StarredChangesUtil.IGNORE_LABEL - + " are mutually exclusive. Only one of them can be set."); - gApi.changes().id(changeId).ignore(true); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, () -> gApi.changes().id(changeId).ignore(true)); + assertThat(thrown) + .hasMessageThat() + .contains( + "The labels " + + StarredChangesUtil.DEFAULT_LABEL + + " and " + + StarredChangesUtil.IGNORE_LABEL + + " are mutually exclusive. Only one of them can be set."); } @Test @@ -4096,14 +4167,17 @@ public class ChangeIT extends AbstractDaemonTest { gApi.changes().id(changeId).ignore(true); assertThat(gApi.changes().id(changeId).ignored()).isTrue(); - exception.expect(ResourceConflictException.class); - exception.expectMessage( - "The labels " - + StarredChangesUtil.DEFAULT_LABEL - + " and " - + StarredChangesUtil.IGNORE_LABEL - + " are mutually exclusive. Only one of them can be set."); - gApi.accounts().self().starChange(changeId); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, () -> gApi.accounts().self().starChange(changeId)); + assertThat(thrown) + .hasMessageThat() + .contains( + "The labels " + + StarredChangesUtil.DEFAULT_LABEL + + " and " + + StarredChangesUtil.IGNORE_LABEL + + " are mutually exclusive. Only one of them can be set."); } @Test @@ -4141,21 +4215,28 @@ public class ChangeIT extends AbstractDaemonTest { gApi.changes().id(changeId).markAsReviewed(true); assertThat(gApi.changes().id(changeId).get().reviewed).isTrue(); - exception.expect(BadRequestException.class); - exception.expectMessage( - "The labels " - + StarredChangesUtil.REVIEWED_LABEL - + "/" - + 1 - + " and " - + StarredChangesUtil.UNREVIEWED_LABEL - + "/" - + 1 - + " are mutually exclusive. Only one of them can be set."); - gApi.accounts() - .self() - .setStars( - changeId, new StarsInput(ImmutableSet.of(StarredChangesUtil.UNREVIEWED_LABEL + "/1"))); + BadRequestException thrown = + assertThrows( + BadRequestException.class, + () -> + gApi.accounts() + .self() + .setStars( + changeId, + new StarsInput( + ImmutableSet.of(StarredChangesUtil.UNREVIEWED_LABEL + "/1")))); + assertThat(thrown) + .hasMessageThat() + .contains( + "The labels " + + StarredChangesUtil.REVIEWED_LABEL + + "/" + + 1 + + " and " + + StarredChangesUtil.UNREVIEWED_LABEL + + "/" + + 1 + + " are mutually exclusive. Only one of them can be set."); } @Test @@ -4166,21 +4247,27 @@ public class ChangeIT extends AbstractDaemonTest { gApi.changes().id(changeId).markAsReviewed(false); assertThat(gApi.changes().id(changeId).get().reviewed).isNull(); - exception.expect(BadRequestException.class); - exception.expectMessage( - "The labels " - + StarredChangesUtil.REVIEWED_LABEL - + "/" - + 1 - + " and " - + StarredChangesUtil.UNREVIEWED_LABEL - + "/" - + 1 - + " are mutually exclusive. Only one of them can be set."); - gApi.accounts() - .self() - .setStars( - changeId, new StarsInput(ImmutableSet.of(StarredChangesUtil.REVIEWED_LABEL + "/1"))); + BadRequestException thrown = + assertThrows( + BadRequestException.class, + () -> + gApi.accounts() + .self() + .setStars( + changeId, + new StarsInput(ImmutableSet.of(StarredChangesUtil.REVIEWED_LABEL + "/1")))); + assertThat(thrown) + .hasMessageThat() + .contains( + "The labels " + + StarredChangesUtil.REVIEWED_LABEL + + "/" + + 1 + + " and " + + StarredChangesUtil.UNREVIEWED_LABEL + + "/" + + 1 + + " are mutually exclusive. Only one of them can be set."); } @Test @@ -4209,9 +4296,14 @@ public class ChangeIT extends AbstractDaemonTest { // label cannot contain whitespace String invalidLabel = "invalid label"; - exception.expect(BadRequestException.class); - exception.expectMessage("invalid labels: " + invalidLabel); - gApi.accounts().self().setStars(changeId, new StarsInput(ImmutableSet.of(invalidLabel))); + BadRequestException thrown = + assertThrows( + BadRequestException.class, + () -> + gApi.accounts() + .self() + .setStars(changeId, new StarsInput(ImmutableSet.of(invalidLabel)))); + assertThat(thrown).hasMessageThat().contains("invalid labels: " + invalidLabel); } @Test diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIdIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIdIT.java index 7899ecd13f..789a7c7944 100644 --- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIdIT.java +++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIdIT.java @@ -15,6 +15,7 @@ package com.google.gerrit.acceptance.api.change; import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.GerritConfig; @@ -56,16 +57,22 @@ public class ChangeIdIT extends AbstractDaemonTest { @Test public void wrongProjectInProjectChangeNumberReturnsNotFound() throws Exception { - exception.expect(ResourceNotFoundException.class); - exception.expectMessage("Not found: unknown~" + changeInfo._number); - gApi.changes().id("unknown", changeInfo._number); + ResourceNotFoundException thrown = + assertThrows( + ResourceNotFoundException.class, + () -> gApi.changes().id("unknown", changeInfo._number)); + assertThat(thrown).hasMessageThat().contains("Not found: unknown~" + changeInfo._number); } @Test public void wrongIdInProjectChangeNumberReturnsNotFound() throws Exception { - exception.expect(ResourceNotFoundException.class); - exception.expectMessage("Not found: " + project.get() + "~" + Integer.MAX_VALUE); - gApi.changes().id(project.get(), Integer.MAX_VALUE); + ResourceNotFoundException thrown = + assertThrows( + ResourceNotFoundException.class, + () -> gApi.changes().id(project.get(), Integer.MAX_VALUE)); + assertThat(thrown) + .hasMessageThat() + .contains("Not found: " + project.get() + "~" + Integer.MAX_VALUE); } @Test @@ -76,8 +83,7 @@ public class ChangeIdIT extends AbstractDaemonTest { @Test public void wrongChangeNumberReturnsNotFound() throws Exception { - exception.expect(ResourceNotFoundException.class); - gApi.changes().id(Integer.MAX_VALUE); + assertThrows(ResourceNotFoundException.class, () -> gApi.changes().id(Integer.MAX_VALUE)); } @Test @@ -88,25 +94,36 @@ public class ChangeIdIT extends AbstractDaemonTest { @Test public void wrongProjectInTripletChangeIdReturnsNotFound() throws Exception { - exception.expect(ResourceNotFoundException.class); - exception.expectMessage("Not found: unknown~" + changeInfo.branch + "~" + changeInfo.changeId); - gApi.changes().id("unknown", changeInfo.branch, changeInfo.changeId); + ResourceNotFoundException thrown = + assertThrows( + ResourceNotFoundException.class, + () -> gApi.changes().id("unknown", changeInfo.branch, changeInfo.changeId)); + assertThat(thrown) + .hasMessageThat() + .contains("Not found: unknown~" + changeInfo.branch + "~" + changeInfo.changeId); } @Test public void wrongBranchInTripletChangeIdReturnsNotFound() throws Exception { - exception.expect(ResourceNotFoundException.class); - exception.expectMessage("Not found: " + project.get() + "~unknown~" + changeInfo.changeId); - gApi.changes().id(project.get(), "unknown", changeInfo.changeId); + ResourceNotFoundException thrown = + assertThrows( + ResourceNotFoundException.class, + () -> gApi.changes().id(project.get(), "unknown", changeInfo.changeId)); + assertThat(thrown) + .hasMessageThat() + .contains("Not found: " + project.get() + "~unknown~" + changeInfo.changeId); } @Test public void wrongIdInTripletChangeIdReturnsNotFound() throws Exception { String unknownId = "I1234567890"; - exception.expect(ResourceNotFoundException.class); - exception.expectMessage( - "Not found: " + project.get() + "~" + changeInfo.branch + "~" + unknownId); - gApi.changes().id(project.get(), changeInfo.branch, unknownId); + ResourceNotFoundException thrown = + assertThrows( + ResourceNotFoundException.class, + () -> gApi.changes().id(project.get(), changeInfo.branch, unknownId)); + assertThat(thrown) + .hasMessageThat() + .contains("Not found: " + project.get() + "~" + changeInfo.branch + "~" + unknownId); } @Test @@ -121,8 +138,7 @@ public class ChangeIdIT extends AbstractDaemonTest { @Test public void wrongChangeIdReturnsNotFound() throws Exception { - exception.expect(ResourceNotFoundException.class); - gApi.changes().id("I1234567890"); + assertThrows(ResourceNotFoundException.class, () -> gApi.changes().id("I1234567890")); } @Test @@ -139,11 +155,13 @@ public class ChangeIdIT extends AbstractDaemonTest { // IHash throws ChangeInfo ci = gApi.changes().create(new ChangeInput(project.get(), "master", "different message")).get(); - exception.expect(DeprecatedIdentifierException.class); - exception.expectMessage( - "The provided change identifier " - + ci.changeId - + " is deprecated. Use 'project~changeNumber' instead."); - gApi.changes().id(ci.changeId); + DeprecatedIdentifierException thrown = + assertThrows(DeprecatedIdentifierException.class, () -> gApi.changes().id(ci.changeId)); + assertThat(thrown) + .hasMessageThat() + .contains( + "The provided change identifier " + + ci.changeId + + " is deprecated. Use 'project~changeNumber' instead."); } } diff --git a/javatests/com/google/gerrit/acceptance/api/change/DisablePrivateChangesIT.java b/javatests/com/google/gerrit/acceptance/api/change/DisablePrivateChangesIT.java index ae88afdab2..eab6c3ca6c 100644 --- a/javatests/com/google/gerrit/acceptance/api/change/DisablePrivateChangesIT.java +++ b/javatests/com/google/gerrit/acceptance/api/change/DisablePrivateChangesIT.java @@ -15,6 +15,7 @@ package com.google.gerrit.acceptance.api.change; import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.GerritConfig; @@ -31,9 +32,9 @@ public class DisablePrivateChangesIT extends AbstractDaemonTest { public void createPrivateChangeWithDisablePrivateChangesTrue() throws Exception { ChangeInput input = new ChangeInput(project.get(), "master", "empty change"); input.isPrivate = true; - exception.expect(MethodNotAllowedException.class); - exception.expectMessage("private changes are disabled"); - gApi.changes().create(input); + MethodNotAllowedException thrown = + assertThrows(MethodNotAllowedException.class, () -> gApi.changes().create(input)); + assertThat(thrown).hasMessageThat().contains("private changes are disabled"); } @Test @@ -107,9 +108,11 @@ public class DisablePrivateChangesIT extends AbstractDaemonTest { public void setPrivateWithDisablePrivateChangesTrue() throws Exception { PushOneCommit.Result result = createChange(); - exception.expect(MethodNotAllowedException.class); - exception.expectMessage("private changes are disabled"); - gApi.changes().id(result.getChangeId()).setPrivate(true, "set private"); + MethodNotAllowedException thrown = + assertThrows( + MethodNotAllowedException.class, + () -> gApi.changes().id(result.getChangeId()).setPrivate(true, "set private")); + assertThat(thrown).hasMessageThat().contains("private changes are disabled"); } @Test diff --git a/javatests/com/google/gerrit/acceptance/api/change/MergeListIT.java b/javatests/com/google/gerrit/acceptance/api/change/MergeListIT.java index 1b65e92468..c5765da427 100644 --- a/javatests/com/google/gerrit/acceptance/api/change/MergeListIT.java +++ b/javatests/com/google/gerrit/acceptance/api/change/MergeListIT.java @@ -17,6 +17,7 @@ package com.google.gerrit.acceptance.api.change; import static com.google.common.truth.Truth.assertThat; import static com.google.gerrit.git.ObjectIds.abbreviateName; import static com.google.gerrit.reviewdb.client.Patch.MERGE_LIST; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static java.nio.charset.StandardCharsets.UTF_8; import static org.eclipse.jgit.lib.Constants.HEAD; @@ -153,18 +154,26 @@ public class MergeListIT extends AbstractDaemonTest { public void editMergeList() throws Exception { gApi.changes().id(changeId).edit().create(); - exception.expect(ResourceConflictException.class); - exception.expectMessage("Invalid path: " + MERGE_LIST); - gApi.changes().id(changeId).edit().modifyFile(MERGE_LIST, RawInputUtil.create("new content")); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, + () -> + gApi.changes() + .id(changeId) + .edit() + .modifyFile(MERGE_LIST, RawInputUtil.create("new content"))); + assertThat(thrown).hasMessageThat().contains("Invalid path: " + MERGE_LIST); } @Test public void deleteMergeList() throws Exception { gApi.changes().id(changeId).edit().create(); - exception.expect(ResourceConflictException.class); - exception.expectMessage("no changes were made"); - gApi.changes().id(changeId).edit().deleteFile(MERGE_LIST); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, + () -> gApi.changes().id(changeId).edit().deleteFile(MERGE_LIST)); + assertThat(thrown).hasMessageThat().contains("no changes were made"); } private String getMergeListContent(RevCommit... commits) { diff --git a/javatests/com/google/gerrit/acceptance/api/change/PrivateChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/PrivateChangeIT.java index a4d944b1f1..ab72491c63 100644 --- a/javatests/com/google/gerrit/acceptance/api/change/PrivateChangeIT.java +++ b/javatests/com/google/gerrit/acceptance/api/change/PrivateChangeIT.java @@ -16,6 +16,7 @@ package com.google.gerrit.acceptance.api.change; import static com.google.common.truth.Truth.assertThat; import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import com.google.common.collect.Iterables; import com.google.gerrit.acceptance.AbstractDaemonTest; @@ -89,9 +90,9 @@ public class PrivateChangeIT extends AbstractDaemonTest { String changeId = result.getChangeId(); assertThat(gApi.changes().id(changeId).get().isPrivate).isNull(); - exception.expect(BadRequestException.class); - exception.expectMessage("cannot set merged change to private"); - gApi.changes().id(changeId).setPrivate(true); + BadRequestException thrown = + assertThrows(BadRequestException.class, () -> gApi.changes().id(changeId).setPrivate(true)); + assertThat(thrown).hasMessageThat().contains("cannot set merged change to private"); } @Test @@ -102,9 +103,9 @@ public class PrivateChangeIT extends AbstractDaemonTest { gApi.changes().id(changeId).abandon(); assertThat(gApi.changes().id(changeId).get().isPrivate).isNull(); - exception.expect(BadRequestException.class); - exception.expectMessage("cannot set abandoned change to private"); - gApi.changes().id(changeId).setPrivate(true); + BadRequestException thrown = + assertThrows(BadRequestException.class, () -> gApi.changes().id(changeId).setPrivate(true)); + assertThat(thrown).hasMessageThat().contains("cannot set abandoned change to private"); } @Test @@ -126,9 +127,11 @@ public class PrivateChangeIT extends AbstractDaemonTest { public void cannotSetOtherUsersChangePrivate() throws Exception { PushOneCommit.Result result = createChange(); requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - exception.expectMessage("not allowed to mark private"); - gApi.changes().id(result.getChangeId()).setPrivate(true, null); + AuthException thrown = + assertThrows( + AuthException.class, + () -> gApi.changes().id(result.getChangeId()).setPrivate(true, null)); + assertThat(thrown).hasMessageThat().contains("not allowed to mark private"); } @Test @@ -153,9 +156,10 @@ public class PrivateChangeIT extends AbstractDaemonTest { gApi.changes().id(result.getChangeId()).reviewer(admin.id().toString()).remove(); // This change should not be visible for admin anymore. - exception.expect(ResourceNotFoundException.class); - exception.expectMessage("Not found: " + result.getChangeId()); - gApi.changes().id(result.getChangeId()); + ResourceNotFoundException thrown = + assertThrows( + ResourceNotFoundException.class, () -> gApi.changes().id(result.getChangeId())); + assertThat(thrown).hasMessageThat().contains("Not found: " + result.getChangeId()); } @Test @@ -191,9 +195,9 @@ public class PrivateChangeIT extends AbstractDaemonTest { merge(result); requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - exception.expectMessage("not allowed to mark private"); - gApi.changes().id(changeId).setPrivate(true, null); + AuthException thrown = + assertThrows(AuthException.class, () -> gApi.changes().id(changeId).setPrivate(true, null)); + assertThat(thrown).hasMessageThat().contains("not allowed to mark private"); } @Test diff --git a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java index 9c3dc8df42..09c0631432 100644 --- a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java +++ b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java @@ -24,6 +24,7 @@ import static com.google.gerrit.acceptance.api.group.GroupAssert.assertGroupInfo import static com.google.gerrit.acceptance.rest.account.AccountAssert.assertAccountInfos; import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS; import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static java.lang.annotation.ElementType.METHOD; import static java.lang.annotation.RetentionPolicy.RUNTIME; import static java.util.stream.Collectors.toList; @@ -89,6 +90,7 @@ import com.google.gerrit.server.project.ProjectConfig; import com.google.gerrit.server.project.testing.Util; import com.google.gerrit.server.util.MagicBranch; import com.google.gerrit.server.util.time.TimeUtil; +import com.google.gerrit.testing.GerritJUnit.ThrowingRunnable; import com.google.gerrit.testing.TestTimeUtil; import com.google.inject.Inject; import java.io.IOException; @@ -168,14 +170,16 @@ public class GroupsIT extends AbstractDaemonTest { @Test public void addToNonExistingGroup_NotFound() throws Exception { - exception.expect(ResourceNotFoundException.class); - gApi.groups().id("non-existing").addMembers("admin"); + assertThrows( + ResourceNotFoundException.class, + () -> gApi.groups().id("non-existing").addMembers("admin")); } @Test public void removeFromNonExistingGroup_NotFound() throws Exception { - exception.expect(ResourceNotFoundException.class); - gApi.groups().id("non-existing").removeMembers("admin"); + assertThrows( + ResourceNotFoundException.class, + () -> gApi.groups().id("non-existing").removeMembers("admin")); } @Test @@ -231,8 +235,9 @@ public class GroupsIT extends AbstractDaemonTest { @Test public void addNonExistingMember_UnprocessableEntity() throws Exception { - exception.expect(UnprocessableEntityException.class); - gApi.groups().id("Administrators").addMembers("non-existing"); + assertThrows( + UnprocessableEntityException.class, + () -> gApi.groups().id("Administrators").addMembers("non-existing")); } @Test @@ -351,9 +356,9 @@ public class GroupsIT extends AbstractDaemonTest { public void createDuplicateInternalGroupCaseSensitiveName_Conflict() throws Exception { String dupGroupName = name("dupGroup"); gApi.groups().create(dupGroupName); - exception.expect(ResourceConflictException.class); - exception.expectMessage("group '" + dupGroupName + "' already exists"); - gApi.groups().create(dupGroupName); + ResourceConflictException thrown = + assertThrows(ResourceConflictException.class, () -> gApi.groups().create(dupGroupName)); + assertThat(thrown).hasMessageThat().contains("group '" + dupGroupName + "' already exists"); } @Test @@ -369,33 +374,34 @@ public class GroupsIT extends AbstractDaemonTest { @Test public void createDuplicateSystemGroupCaseSensitiveName_Conflict() throws Exception { String newGroupName = "Registered Users"; - exception.expect(ResourceConflictException.class); - exception.expectMessage("group 'Registered Users' already exists"); - gApi.groups().create(newGroupName); + ResourceConflictException thrown = + assertThrows(ResourceConflictException.class, () -> gApi.groups().create(newGroupName)); + assertThat(thrown).hasMessageThat().contains("group 'Registered Users' already exists"); } @Test public void createDuplicateSystemGroupCaseInsensitiveName_Conflict() throws Exception { String newGroupName = "registered users"; - exception.expect(ResourceConflictException.class); - exception.expectMessage("group 'Registered Users' already exists"); - gApi.groups().create(newGroupName); + ResourceConflictException thrown = + assertThrows(ResourceConflictException.class, () -> gApi.groups().create(newGroupName)); + assertThat(thrown).hasMessageThat().contains("group 'Registered Users' already exists"); } @Test @GerritConfig(name = "groups.global:Anonymous-Users.name", value = "All Users") public void createGroupWithConfiguredNameOfSystemGroup_Conflict() throws Exception { - exception.expect(ResourceConflictException.class); - exception.expectMessage("group 'All Users' already exists"); - gApi.groups().create("all users"); + ResourceConflictException thrown = + assertThrows(ResourceConflictException.class, () -> gApi.groups().create("all users")); + assertThat(thrown).hasMessageThat().contains("group 'All Users' already exists"); } @Test @GerritConfig(name = "groups.global:Anonymous-Users.name", value = "All Users") public void createGroupWithDefaultNameOfSystemGroup_Conflict() throws Exception { - exception.expect(ResourceConflictException.class); - exception.expectMessage("group name 'Anonymous Users' is reserved"); - gApi.groups().create("anonymous users"); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, () -> gApi.groups().create("anonymous users")); + assertThat(thrown).hasMessageThat().contains("group name 'Anonymous Users' is reserved"); } @Test @@ -414,8 +420,7 @@ public class GroupsIT extends AbstractDaemonTest { @Test public void createGroupWithoutCapability_Forbidden() throws Exception { requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - gApi.groups().create(name("newGroup")); + assertThrows(AuthException.class, () -> gApi.groups().create(name("newGroup"))); } @Test @@ -481,8 +486,7 @@ public class GroupsIT extends AbstractDaemonTest { @Test @GerritConfig(name = "groups.global:Anonymous-Users.name", value = "All Users") public void getSystemGroupByDefaultName_NotFound() throws Exception { - exception.expect(ResourceNotFoundException.class); - gApi.groups().id("Anonymous-Users").get(); + assertThrows(ResourceNotFoundException.class, () -> gApi.groups().id("Anonymous-Users").get()); } @Test @@ -498,8 +502,7 @@ public class GroupsIT extends AbstractDaemonTest { String name = name("Users"); gApi.groups().create(name).get(); - exception.expect(ResourceConflictException.class); - gApi.groups().create(name); + assertThrows(ResourceConflictException.class, () -> gApi.groups().create(name)); } @Test @@ -528,9 +531,7 @@ public class GroupsIT extends AbstractDaemonTest { String name2 = name("Name2"); gApi.groups().create(name2); - - exception.expect(ResourceConflictException.class); - gApi.groups().id(group1.id).name(name2); + assertThrows(ResourceConflictException.class, () -> gApi.groups().id(group1.id).name(name2)); } @Test @@ -554,8 +555,7 @@ public class GroupsIT extends AbstractDaemonTest { gApi.groups().id(group.id).name(newName); assertGroupDoesNotExist(name); - exception.expect(ResourceNotFoundException.class); - gApi.groups().id(name).get(); + assertThrows(ResourceNotFoundException.class, () -> gApi.groups().id(name).get()); } @Test @@ -626,14 +626,15 @@ public class GroupsIT extends AbstractDaemonTest { assertThat(Url.decode(gApi.groups().id(name).owner().id)).isEqualTo(adminUUID); // set non existing owner - exception.expect(UnprocessableEntityException.class); - gApi.groups().id(name).owner("Non-Existing Group"); + assertThrows( + UnprocessableEntityException.class, + () -> gApi.groups().id(name).owner("Non-Existing Group")); } @Test public void listNonExistingGroupIncludes_NotFound() throws Exception { - exception.expect(ResourceNotFoundException.class); - gApi.groups().id("non-existing").includedGroups(); + assertThrows( + ResourceNotFoundException.class, () -> gApi.groups().id("non-existing").includedGroups()); } @Test @@ -645,8 +646,9 @@ public class GroupsIT extends AbstractDaemonTest { @Test public void includeNonExistingGroup() throws Exception { AccountGroup.UUID gx = groupOperations.newGroup().create(); - exception.expect(UnprocessableEntityException.class); - gApi.groups().id(gx.get()).addGroups("non-existing"); + assertThrows( + UnprocessableEntityException.class, + () -> gApi.groups().id(gx.get()).addGroups("non-existing")); } @Test @@ -675,8 +677,7 @@ public class GroupsIT extends AbstractDaemonTest { @Test public void listNonExistingGroupMembers_NotFound() throws Exception { - exception.expect(ResourceNotFoundException.class); - gApi.groups().id("non-existing").members(); + assertThrows(ResourceNotFoundException.class, () -> gApi.groups().id("non-existing").members()); } @Test @@ -820,9 +821,11 @@ public class GroupsIT extends AbstractDaemonTest { assertThat(owned).isEmpty(); // By non-existing group - exception.expect(UnprocessableEntityException.class); - exception.expectMessage("Group Not Found: does-not-exist"); - gApi.groups().list().withOwnedBy("does-not-exist").get(); + UnprocessableEntityException thrown = + assertThrows( + UnprocessableEntityException.class, + () -> gApi.groups().list().withOwnedBy("does-not-exist").get()); + assertThat(thrown).hasMessageThat().contains("Group Not Found: does-not-exist"); } @Test @@ -1023,9 +1026,9 @@ public class GroupsIT extends AbstractDaemonTest { // user cannot reindex any group requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - exception.expectMessage("not allowed to index group"); - gApi.groups().id(group.id).index(); + AuthException thrown = + assertThrows(AuthException.class, () -> gApi.groups().id(group.id).index()); + assertThat(thrown).hasMessageThat().contains("not allowed to index group"); } @Test @@ -1118,13 +1121,13 @@ public class GroupsIT extends AbstractDaemonTest { } @Test - public void pushToGroupBranchForReviewForAllUsersRepoIsRejectedOnSubmit() throws Exception { + public void pushToGroupBranchForReviewForAllUsersRepoIsRejectedOnSubmit() throws Throwable { pushToGroupBranchForReviewAndSubmit( allUsers, RefNames.refsGroups(adminGroupUuid()), "group update not allowed"); } @Test - public void pushToGroupBranchForReviewForNonAllUsersRepoAndSubmit() throws Exception { + public void pushToGroupBranchForReviewForNonAllUsersRepoAndSubmit() throws Throwable { String groupRef = RefNames.refsGroups(adminGroupUuid()); createBranch(project, groupRef); pushToGroupBranchForReviewAndSubmit(project, groupRef, null); @@ -1411,7 +1414,7 @@ public class GroupsIT extends AbstractDaemonTest { } private void pushToGroupBranchForReviewAndSubmit( - Project.NameKey project, String groupRef, String expectedError) throws Exception { + Project.NameKey project, String groupRef, String expectedError) throws Throwable { grantLabel( "Code-Review", -2, 2, project, RefNames.REFS_GROUPS + "*", false, REGISTERED_USERS, false); grant(project, RefNames.REFS_GROUPS + "*", Permission.SUBMIT, false, REGISTERED_USERS); @@ -1428,11 +1431,13 @@ public class GroupsIT extends AbstractDaemonTest { assertThat(r.getChange().change().getDest().branch()).isEqualTo(groupRef); gApi.changes().id(r.getChangeId()).current().review(ReviewInput.approve()); + ThrowingRunnable submit = () -> gApi.changes().id(r.getChangeId()).current().submit(); if (expectedError != null) { - exception.expect(ResourceConflictException.class); - exception.expectMessage("group update not allowed"); + Throwable thrown = assertThrows(ResourceConflictException.class, submit); + assertThat(thrown).hasMessageThat().contains("group update not allowed"); + } else { + submit.run(); } - gApi.changes().id(r.getChangeId()).current().submit(); } private void createBranch(Project.NameKey project, String ref) throws IOException { diff --git a/javatests/com/google/gerrit/acceptance/api/group/GroupsUpdateIT.java b/javatests/com/google/gerrit/acceptance/api/group/GroupsUpdateIT.java index 8f0dcee72f..73731e5f60 100644 --- a/javatests/com/google/gerrit/acceptance/api/group/GroupsUpdateIT.java +++ b/javatests/com/google/gerrit/acceptance/api/group/GroupsUpdateIT.java @@ -15,6 +15,7 @@ package com.google.gerrit.acceptance.api.group; import static com.google.common.truth.Truth8.assertThat; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import com.google.common.collect.ImmutableSet; import com.google.gerrit.common.data.GroupReference; @@ -36,12 +37,9 @@ import java.util.stream.Stream; import org.eclipse.jgit.errors.ConfigInvalidException; import org.junit.Rule; import org.junit.Test; -import org.junit.rules.ExpectedException; public class GroupsUpdateIT { @Rule public InMemoryTestEnvironment testEnvironment = new InMemoryTestEnvironment(); - @Rule public ExpectedException expectedException = ExpectedException.none(); - @Inject @ServerInitiated private Provider groupsUpdateProvider; @Inject private Groups groups; @@ -79,9 +77,9 @@ public class GroupsUpdateIT { public void groupUpdateFailsWithExceptionForNotExistingGroup() throws Exception { InternalGroupUpdate groupUpdate = InternalGroupUpdate.builder().setDescription("A description for the group").build(); - - expectedException.expect(NoSuchGroupException.class); - updateGroup(AccountGroup.uuid("nonexistent-group-UUID"), groupUpdate); + assertThrows( + NoSuchGroupException.class, + () -> updateGroup(AccountGroup.uuid("nonexistent-group-UUID"), groupUpdate)); } private void createGroup(String groupName, String groupUuid) throws Exception { diff --git a/javatests/com/google/gerrit/acceptance/api/plugin/PluginIT.java b/javatests/com/google/gerrit/acceptance/api/plugin/PluginIT.java index 1d3eb176b4..2943445a8d 100644 --- a/javatests/com/google/gerrit/acceptance/api/plugin/PluginIT.java +++ b/javatests/com/google/gerrit/acceptance/api/plugin/PluginIT.java @@ -15,6 +15,7 @@ package com.google.gerrit.acceptance.api.plugin; import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.stream.Collectors.toList; @@ -136,15 +137,16 @@ public class PluginIT extends AbstractDaemonTest { @Test public void installNotAllowed() throws Exception { - exception.expect(MethodNotAllowedException.class); - exception.expectMessage("remote plugin administration is disabled"); - gApi.plugins().install("test.js", new InstallPluginInput()); + MethodNotAllowedException thrown = + assertThrows( + MethodNotAllowedException.class, + () -> gApi.plugins().install("test.js", new InstallPluginInput())); + assertThat(thrown).hasMessageThat().contains("remote plugin administration is disabled"); } @Test public void getNonExistingThrowsNotFound() throws Exception { - exception.expect(ResourceNotFoundException.class); - gApi.plugins().name("does-not-exist"); + assertThrows(ResourceNotFoundException.class, () -> gApi.plugins().name("does-not-exist")); } private ListRequest list() throws RestApiException { diff --git a/javatests/com/google/gerrit/acceptance/api/project/CheckAccessIT.java b/javatests/com/google/gerrit/acceptance/api/project/CheckAccessIT.java index 8cdd2f66e2..4bc4037164 100644 --- a/javatests/com/google/gerrit/acceptance/api/project/CheckAccessIT.java +++ b/javatests/com/google/gerrit/acceptance/api/project/CheckAccessIT.java @@ -15,6 +15,7 @@ package com.google.gerrit.acceptance.api.project; import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import com.google.common.collect.ImmutableList; import com.google.gerrit.acceptance.AbstractDaemonTest; @@ -89,9 +90,11 @@ public class CheckAccessIT extends AbstractDaemonTest { @Test public void emptyInput() throws Exception { - exception.expect(BadRequestException.class); - exception.expectMessage("input requires 'account'"); - gApi.projects().name(normalProject.get()).checkAccess(new AccessCheckInput()); + BadRequestException thrown = + assertThrows( + BadRequestException.class, + () -> gApi.projects().name(normalProject.get()).checkAccess(new AccessCheckInput())); + assertThat(thrown).hasMessageThat().contains("input requires 'account'"); } @Test @@ -101,9 +104,11 @@ public class CheckAccessIT extends AbstractDaemonTest { in.permission = "notapermission"; in.ref = "refs/heads/master"; - exception.expect(BadRequestException.class); - exception.expectMessage("not recognized"); - gApi.projects().name(normalProject.get()).checkAccess(in); + BadRequestException thrown = + assertThrows( + BadRequestException.class, + () -> gApi.projects().name(normalProject.get()).checkAccess(in)); + assertThat(thrown).hasMessageThat().contains("not recognized"); } @Test @@ -112,9 +117,11 @@ public class CheckAccessIT extends AbstractDaemonTest { in.account = user.email(); in.permission = "forge_author"; - exception.expect(BadRequestException.class); - exception.expectMessage("must set 'ref'"); - gApi.projects().name(normalProject.get()).checkAccess(in); + BadRequestException thrown = + assertThrows( + BadRequestException.class, + () -> gApi.projects().name(normalProject.get()).checkAccess(in)); + assertThat(thrown).hasMessageThat().contains("must set 'ref'"); } @Test @@ -124,9 +131,11 @@ public class CheckAccessIT extends AbstractDaemonTest { in.permission = "rebase"; in.ref = "refs/heads/master"; - exception.expect(BadRequestException.class); - exception.expectMessage("recognized as ref permission"); - gApi.projects().name(normalProject.get()).checkAccess(in); + BadRequestException thrown = + assertThrows( + BadRequestException.class, + () -> gApi.projects().name(normalProject.get()).checkAccess(in)); + assertThat(thrown).hasMessageThat().contains("recognized as ref permission"); } @Test @@ -136,9 +145,11 @@ public class CheckAccessIT extends AbstractDaemonTest { in.permission = "rebase"; in.ref = "refs/heads/master"; - exception.expect(UnprocessableEntityException.class); - exception.expectMessage("Account 'doesnotexist@invalid.com' not found"); - gApi.projects().name(normalProject.get()).checkAccess(in); + UnprocessableEntityException thrown = + assertThrows( + UnprocessableEntityException.class, + () -> gApi.projects().name(normalProject.get()).checkAccess(in)); + assertThat(thrown).hasMessageThat().contains("Account 'doesnotexist@invalid.com' not found"); } private static class TestCase { diff --git a/javatests/com/google/gerrit/acceptance/api/project/CheckProjectIT.java b/javatests/com/google/gerrit/acceptance/api/project/CheckProjectIT.java index bf82603c0e..96ba722ac5 100644 --- a/javatests/com/google/gerrit/acceptance/api/project/CheckProjectIT.java +++ b/javatests/com/google/gerrit/acceptance/api/project/CheckProjectIT.java @@ -16,6 +16,7 @@ package com.google.gerrit.acceptance.api.project; import static com.google.common.truth.Truth.assertThat; import static com.google.gerrit.acceptance.GitUtil.pushHead; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static java.util.stream.Collectors.toList; import static java.util.stream.Collectors.toSet; @@ -232,18 +233,21 @@ public class CheckProjectIT extends AbstractDaemonTest { CheckProjectInput input = new CheckProjectInput(); input.autoCloseableChangesCheck = new AutoCloseableChangesCheckInput(); - exception.expect(BadRequestException.class); - exception.expectMessage("branch is required"); - gApi.projects().name(project.get()).check(input); + BadRequestException thrown = + assertThrows( + BadRequestException.class, () -> gApi.projects().name(project.get()).check(input)); + assertThat(thrown).hasMessageThat().contains("branch is required"); } @Test public void nonExistingBranch() throws Exception { CheckProjectInput input = checkProjectInputForAutoCloseableCheck("non-existing"); - exception.expect(UnprocessableEntityException.class); - exception.expectMessage("branch 'non-existing' not found"); - gApi.projects().name(project.get()).check(input); + UnprocessableEntityException thrown = + assertThrows( + UnprocessableEntityException.class, + () -> gApi.projects().name(project.get()).check(input)); + assertThat(thrown).hasMessageThat().contains("branch 'non-existing' not found"); } @Test @@ -266,11 +270,14 @@ public class CheckProjectIT extends AbstractDaemonTest { input.autoCloseableChangesCheck.maxCommits = ProjectsConsistencyChecker.AUTO_CLOSE_MAX_COMMITS_LIMIT + 1; - exception.expect(BadRequestException.class); - exception.expectMessage( - "max commits can at most be set to " - + ProjectsConsistencyChecker.AUTO_CLOSE_MAX_COMMITS_LIMIT); - gApi.projects().name(project.get()).check(input); + BadRequestException thrown = + assertThrows( + BadRequestException.class, () -> gApi.projects().name(project.get()).check(input)); + assertThat(thrown) + .hasMessageThat() + .contains( + "max commits can at most be set to " + + ProjectsConsistencyChecker.AUTO_CLOSE_MAX_COMMITS_LIMIT); } private RevCommit pushCommitWithoutChangeIdForReview() throws Exception { diff --git a/javatests/com/google/gerrit/acceptance/api/project/DashboardIT.java b/javatests/com/google/gerrit/acceptance/api/project/DashboardIT.java index e51a06967a..4a5ad6a8fe 100644 --- a/javatests/com/google/gerrit/acceptance/api/project/DashboardIT.java +++ b/javatests/com/google/gerrit/acceptance/api/project/DashboardIT.java @@ -16,6 +16,7 @@ package com.google.gerrit.acceptance.api.project; import static com.google.common.truth.Truth.assertThat; import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static java.util.stream.Collectors.toList; import com.google.common.collect.ImmutableList; @@ -47,14 +48,12 @@ public class DashboardIT extends AbstractDaemonTest { @Test public void defaultDashboardDoesNotExist() throws Exception { - exception.expect(ResourceNotFoundException.class); - project().defaultDashboard().get(); + assertThrows(ResourceNotFoundException.class, () -> project().defaultDashboard().get()); } @Test public void dashboardDoesNotExist() throws Exception { - exception.expect(ResourceNotFoundException.class); - project().dashboard("my:dashboard").get(); + assertThrows(ResourceNotFoundException.class, () -> project().dashboard("my:dashboard").get()); } @Test @@ -110,8 +109,7 @@ public class DashboardIT extends AbstractDaemonTest { project().removeDefaultDashboard(); assertThat(project().dashboard(info.id).get().isDefault).isNull(); - exception.expect(ResourceNotFoundException.class); - project().defaultDashboard().get(); + assertThrows(ResourceNotFoundException.class, () -> project().defaultDashboard().get()); } @Test @@ -133,9 +131,9 @@ public class DashboardIT extends AbstractDaemonTest { @Test public void cannotGetDashboardWithInheritedForNonDefault() throws Exception { DashboardInfo info = createTestDashboard(); - exception.expect(BadRequestException.class); - exception.expectMessage("inherited flag can only be used with default"); - project().dashboard(info.id).get(true); + BadRequestException thrown = + assertThrows(BadRequestException.class, () -> project().dashboard(info.id).get(true)); + assertThat(thrown).hasMessageThat().contains("inherited flag can only be used with default"); } private void assertDashboardInfo(DashboardInfo actual, DashboardInfo expected) throws Exception { diff --git a/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java b/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java index 1c38cddeeb..a5164689b8 100644 --- a/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java +++ b/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java @@ -20,6 +20,7 @@ import static com.google.gerrit.server.project.ProjectState.INHERITED_FROM_GLOBA import static com.google.gerrit.server.project.ProjectState.INHERITED_FROM_PARENT; import static com.google.gerrit.server.project.ProjectState.OVERRIDDEN_BY_GLOBAL; import static com.google.gerrit.server.project.ProjectState.OVERRIDDEN_BY_PARENT; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static java.util.stream.Collectors.toSet; import com.google.common.collect.ImmutableList; @@ -165,17 +166,17 @@ public class ProjectIT extends AbstractDaemonTest { public void createProjectWithMismatchedInput() throws Exception { ProjectInput in = new ProjectInput(); in.name = name("foo"); - exception.expect(BadRequestException.class); - exception.expectMessage("name must match input.name"); - gApi.projects().name("bar").create(in); + BadRequestException thrown = + assertThrows(BadRequestException.class, () -> gApi.projects().name("bar").create(in)); + assertThat(thrown).hasMessageThat().contains("name must match input.name"); } @Test public void createProjectNoNameInInput() throws Exception { ProjectInput in = new ProjectInput(); - exception.expect(BadRequestException.class); - exception.expectMessage("input.name is required"); - gApi.projects().create(in); + BadRequestException thrown = + assertThrows(BadRequestException.class, () -> gApi.projects().create(in)); + assertThat(thrown).hasMessageThat().contains("input.name is required"); } @Test @@ -183,9 +184,9 @@ public class ProjectIT extends AbstractDaemonTest { ProjectInput in = new ProjectInput(); in.name = name("baz"); gApi.projects().create(in); - exception.expect(ResourceConflictException.class); - exception.expectMessage("Project already exists"); - gApi.projects().create(in); + ResourceConflictException thrown = + assertThrows(ResourceConflictException.class, () -> gApi.projects().create(in)); + assertThat(thrown).hasMessageThat().contains("Project already exists"); } @Test @@ -194,9 +195,9 @@ public class ProjectIT extends AbstractDaemonTest { in.name = name("baz"); in.parent = "non-existing"; - exception.expect(UnprocessableEntityException.class); - exception.expectMessage("Project Not Found: " + in.parent); - gApi.projects().create(in); + UnprocessableEntityException thrown = + assertThrows(UnprocessableEntityException.class, () -> gApi.projects().create(in)); + assertThat(thrown).hasMessageThat().contains("Project Not Found: " + in.parent); } @Test @@ -205,9 +206,9 @@ public class ProjectIT extends AbstractDaemonTest { in.name = name("baz"); in.parent = in.name; - exception.expect(UnprocessableEntityException.class); - exception.expectMessage("Project Not Found: " + in.parent); - gApi.projects().create(in); + UnprocessableEntityException thrown = + assertThrows(UnprocessableEntityException.class, () -> gApi.projects().create(in)); + assertThat(thrown).hasMessageThat().contains("Project Not Found: " + in.parent); } @Test @@ -215,9 +216,11 @@ public class ProjectIT extends AbstractDaemonTest { ProjectInput in = new ProjectInput(); in.name = name("foo"); in.parent = allUsers.get(); - exception.expect(ResourceConflictException.class); - exception.expectMessage(String.format("Cannot inherit from '%s' project", allUsers.get())); - gApi.projects().create(in); + ResourceConflictException thrown = + assertThrows(ResourceConflictException.class, () -> gApi.projects().create(in)); + assertThat(thrown) + .hasMessageThat() + .contains(String.format("Cannot inherit from '%s' project", allUsers.get())); } @Test @@ -354,9 +357,9 @@ public class ProjectIT extends AbstractDaemonTest { public void nonOwnerCannotSetConfig() throws Exception { ConfigInput input = createTestConfigInput(); requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - exception.expectMessage("write refs/meta/config not permitted"); - gApi.projects().name(project.get()).config(input); + AuthException thrown = + assertThrows(AuthException.class, () -> gApi.projects().name(project.get()).config(input)); + assertThat(thrown).hasMessageThat().contains("write refs/meta/config not permitted"); } @Test @@ -372,8 +375,9 @@ public class ProjectIT extends AbstractDaemonTest { @Test public void setHeadToNonexistentBranch() throws Exception { - exception.expect(UnprocessableEntityException.class); - gApi.projects().name(project.get()).head("does-not-exist"); + assertThrows( + UnprocessableEntityException.class, + () -> gApi.projects().name(project.get()).head("does-not-exist")); } @Test @@ -389,9 +393,9 @@ public class ProjectIT extends AbstractDaemonTest { public void setHeadNotAllowed() throws Exception { gApi.projects().name(project.get()).branch("test").create(new BranchInput()); requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - exception.expectMessage("not permitted: set HEAD on refs/heads/test"); - gApi.projects().name(project.get()).head("test"); + AuthException thrown = + assertThrows(AuthException.class, () -> gApi.projects().name(project.get()).head("test")); + assertThat(thrown).hasMessageThat().contains("not permitted: set HEAD on refs/heads/test"); } @Test @@ -614,9 +618,9 @@ public class ProjectIT extends AbstractDaemonTest { @Test public void invalidMaxObjectSizeIsRejected() throws Exception { - exception.expect(ResourceConflictException.class); - exception.expectMessage("100 foo"); - setMaxObjectSize("100 foo"); + ResourceConflictException thrown = + assertThrows(ResourceConflictException.class, () -> setMaxObjectSize("100 foo")); + assertThat(thrown).hasMessageThat().contains("100 foo"); } @Test diff --git a/javatests/com/google/gerrit/acceptance/api/project/SetParentIT.java b/javatests/com/google/gerrit/acceptance/api/project/SetParentIT.java index 3c1428d3ae..b2da40283c 100644 --- a/javatests/com/google/gerrit/acceptance/api/project/SetParentIT.java +++ b/javatests/com/google/gerrit/acceptance/api/project/SetParentIT.java @@ -15,6 +15,7 @@ package com.google.gerrit.acceptance.api.project; import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.GerritConfig; @@ -42,8 +43,7 @@ public class SetParentIT extends AbstractDaemonTest { public void setParentNotAllowed() throws Exception { String parent = projectOperations.newProject().create().get(); requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - gApi.projects().name(project.get()).parent(parent); + assertThrows(AuthException.class, () -> gApi.projects().name(project.get()).parent(parent)); } @Test @@ -51,8 +51,7 @@ public class SetParentIT extends AbstractDaemonTest { public void setParentNotAllowedForNonOwners() throws Exception { String parent = projectOperations.newProject().create().get(); requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - gApi.projects().name(project.get()).parent(parent); + assertThrows(AuthException.class, () -> gApi.projects().name(project.get()).parent(parent)); } @Test @@ -96,47 +95,63 @@ public class SetParentIT extends AbstractDaemonTest { @Test public void setParentForAllProjectsNotAllowed() throws Exception { - exception.expect(ResourceConflictException.class); - exception.expectMessage("cannot set parent of " + AllProjectsNameProvider.DEFAULT); - gApi.projects().name(allProjects.get()).parent(project.get()); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, + () -> gApi.projects().name(allProjects.get()).parent(project.get())); + assertThat(thrown) + .hasMessageThat() + .contains("cannot set parent of " + AllProjectsNameProvider.DEFAULT); } @Test public void setParentToSelfNotAllowed() throws Exception { - exception.expect(ResourceConflictException.class); - exception.expectMessage("cannot set parent to self"); - gApi.projects().name(project.get()).parent(project.get()); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, + () -> gApi.projects().name(project.get()).parent(project.get())); + assertThat(thrown).hasMessageThat().contains("cannot set parent to self"); } @Test public void setParentToOwnChildNotAllowed() throws Exception { String child = projectOperations.newProject().parent(project).create().get(); - exception.expect(ResourceConflictException.class); - exception.expectMessage("cycle exists between"); - gApi.projects().name(project.get()).parent(child); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, + () -> gApi.projects().name(project.get()).parent(child)); + assertThat(thrown).hasMessageThat().contains("cycle exists between"); } @Test public void setParentToGrandchildNotAllowed() throws Exception { Project.NameKey child = projectOperations.newProject().parent(project).create(); String grandchild = projectOperations.newProject().parent(child).create().get(); - exception.expect(ResourceConflictException.class); - exception.expectMessage("cycle exists between"); - gApi.projects().name(project.get()).parent(grandchild); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, + () -> gApi.projects().name(project.get()).parent(grandchild)); + assertThat(thrown).hasMessageThat().contains("cycle exists between"); } @Test public void setParentToNonexistentProject() throws Exception { - exception.expect(UnprocessableEntityException.class); - exception.expectMessage("not found"); - gApi.projects().name(project.get()).parent("non-existing"); + UnprocessableEntityException thrown = + assertThrows( + UnprocessableEntityException.class, + () -> gApi.projects().name(project.get()).parent("non-existing")); + assertThat(thrown).hasMessageThat().contains("not found"); } @Test public void setParentToAllUsersNotAllowed() throws Exception { - exception.expect(ResourceConflictException.class); - exception.expectMessage(String.format("Cannot inherit from '%s' project", allUsers.get())); - gApi.projects().name(project.get()).parent(allUsers.get()); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, + () -> gApi.projects().name(project.get()).parent(allUsers.get())); + assertThat(thrown) + .hasMessageThat() + .contains(String.format("Cannot inherit from '%s' project", allUsers.get())); } @Test @@ -145,8 +160,9 @@ public class SetParentIT extends AbstractDaemonTest { String parent = projectOperations.newProject().create().get(); - exception.expect(BadRequestException.class); - exception.expectMessage("All-Users must inherit from All-Projects"); - gApi.projects().name(allUsers.get()).parent(parent); + BadRequestException thrown = + assertThrows( + BadRequestException.class, () -> gApi.projects().name(allUsers.get()).parent(parent)); + assertThat(thrown).hasMessageThat().contains("All-Users must inherit from All-Projects"); } } diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java index a569cfa02c..80466808e3 100644 --- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java +++ b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java @@ -27,6 +27,7 @@ import static com.google.gerrit.git.ObjectIds.abbreviateName; import static com.google.gerrit.reviewdb.client.Patch.COMMIT_MSG; import static com.google.gerrit.reviewdb.client.Patch.MERGE_LIST; import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.stream.Collectors.toList; import static org.eclipse.jgit.lib.Constants.HEAD; @@ -260,9 +261,11 @@ public class RevisionIT extends AbstractDaemonTest { ReviewInput in = new ReviewInput(); in.label("Code-Review", 0); - exception.expect(ResourceConflictException.class); - exception.expectMessage("Cannot reduce vote on labels for closed change: Code-Review"); - revision(r).review(in); + ResourceConflictException thrown = + assertThrows(ResourceConflictException.class, () -> revision(r).review(in)); + assertThat(thrown) + .hasMessageThat() + .contains("Cannot reduce vote on labels for closed change: Code-Review"); } @TestProjectInput(submitType = SubmitType.CHERRY_PICK) @@ -288,18 +291,26 @@ public class RevisionIT extends AbstractDaemonTest { public void voteOnAbandonedChange() throws Exception { PushOneCommit.Result r = createChange(); gApi.changes().id(r.getChangeId()).abandon(); - exception.expect(ResourceConflictException.class); - exception.expectMessage("change is closed"); - gApi.changes().id(r.getChangeId()).current().review(ReviewInput.reject()); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, + () -> gApi.changes().id(r.getChangeId()).current().review(ReviewInput.reject())); + assertThat(thrown).hasMessageThat().contains("change is closed"); } @Test public void voteNotAllowedWithoutPermission() throws Exception { PushOneCommit.Result r = createChange(); requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - exception.expectMessage("is restricted"); - gApi.changes().id(r.getChange().getId().get()).current().review(ReviewInput.approve()); + AuthException thrown = + assertThrows( + AuthException.class, + () -> + gApi.changes() + .id(r.getChange().getId().get()) + .current() + .review(ReviewInput.approve())); + assertThat(thrown).hasMessageThat().contains("is restricted"); } @Test @@ -455,9 +466,11 @@ public class RevisionIT extends AbstractDaemonTest { cherry.current().review(ReviewInput.approve()); cherry.current().submit(); - exception.expect(ResourceConflictException.class); - exception.expectMessage("Cherry pick failed: identical tree"); - orig.revision(r.getCommit().name()).cherryPick(in); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, + () -> orig.revision(r.getCommit().name()).cherryPick(in)); + assertThat(thrown).hasMessageThat().contains("Cherry pick failed: identical tree"); } @Test @@ -481,9 +494,11 @@ public class RevisionIT extends AbstractDaemonTest { ChangeApi orig = gApi.changes().id(triplet); assertThat(orig.get().messages).hasSize(1); - exception.expect(ResourceConflictException.class); - exception.expectMessage("Cherry pick failed: merge conflict"); - orig.revision(r.getCommit().name()).cherryPick(in); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, + () -> orig.revision(r.getCommit().name()).cherryPick(in)); + assertThat(thrown).hasMessageThat().contains("Cherry pick failed: merge conflict"); } @Test @@ -690,10 +705,17 @@ public class RevisionIT extends AbstractDaemonTest { cherryPickInput.message = "Cherry-pick a merge commit to another branch"; cherryPickInput.parent = 0; - exception.expect(BadRequestException.class); - exception.expectMessage( - "Cherry Pick: Parent 0 does not exist. Please specify a parent in range [1, 2]."); - gApi.changes().id(mergeChangeResult.getChangeId()).current().cherryPick(cherryPickInput); + BadRequestException thrown = + assertThrows( + BadRequestException.class, + () -> + gApi.changes() + .id(mergeChangeResult.getChangeId()) + .current() + .cherryPick(cherryPickInput)); + assertThat(thrown) + .hasMessageThat() + .contains("Cherry Pick: Parent 0 does not exist. Please specify a parent in range [1, 2]."); } @Test @@ -711,10 +733,17 @@ public class RevisionIT extends AbstractDaemonTest { cherryPickInput.message = "Cherry-pick a merge commit to another branch"; cherryPickInput.parent = 3; - exception.expect(BadRequestException.class); - exception.expectMessage( - "Cherry Pick: Parent 3 does not exist. Please specify a parent in range [1, 2]."); - gApi.changes().id(mergeChangeResult.getChangeId()).current().cherryPick(cherryPickInput); + BadRequestException thrown = + assertThrows( + BadRequestException.class, + () -> + gApi.changes() + .id(mergeChangeResult.getChangeId()) + .current() + .cherryPick(cherryPickInput)); + assertThat(thrown) + .hasMessageThat() + .contains("Cherry Pick: Parent 3 does not exist. Please specify a parent in range [1, 2]."); } @Test @@ -851,10 +880,13 @@ public class RevisionIT extends AbstractDaemonTest { input.message = srcChange.getCommit().getFullMessage(); requestScopeOperations.setApiUser(user.id()); - exception.expect(UnprocessableEntityException.class); - exception.expectMessage( - String.format("Commit %s does not exist on branch refs/heads/foo", input.base)); - gApi.changes().id(srcChange.getChangeId()).current().cherryPick(input).get(); + UnprocessableEntityException thrown = + assertThrows( + UnprocessableEntityException.class, + () -> gApi.changes().id(srcChange.getChangeId()).current().cherryPick(input).get()); + assertThat(thrown) + .hasMessageThat() + .contains(String.format("Commit %s does not exist on branch refs/heads/foo", input.base)); } @Test @@ -868,12 +900,16 @@ public class RevisionIT extends AbstractDaemonTest { input.base = change2.getCommit().name(); input.message = change1.getCommit().getFullMessage(); - exception.expect(ResourceConflictException.class); - exception.expectMessage( - String.format( - "Change %s with commit %s is abandoned", - change2.getChange().getId().get(), input.base)); - gApi.changes().id(change1.getChangeId()).current().cherryPick(input); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, + () -> gApi.changes().id(change1.getChangeId()).current().cherryPick(input)); + assertThat(thrown) + .hasMessageThat() + .contains( + String.format( + "Change %s with commit %s is abandoned", + change2.getChange().getId().get(), input.base)); } @Test @@ -885,9 +921,13 @@ public class RevisionIT extends AbstractDaemonTest { input.base = "invalid-sha1"; input.message = change1.getCommit().getFullMessage(); - exception.expect(BadRequestException.class); - exception.expectMessage(String.format("Base %s doesn't represent a valid SHA-1", input.base)); - gApi.changes().id(change1.getChangeId()).current().cherryPick(input); + BadRequestException thrown = + assertThrows( + BadRequestException.class, + () -> gApi.changes().id(change1.getChangeId()).current().cherryPick(input)); + assertThat(thrown) + .hasMessageThat() + .contains(String.format("Base %s doesn't represent a valid SHA-1", input.base)); } @Test @@ -1132,9 +1172,15 @@ public class RevisionIT extends AbstractDaemonTest { PushOneCommit.Result r = createChange(); assertDescription(r, ""); requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - exception.expectMessage("edit description not permitted"); - gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).description("test"); + AuthException thrown = + assertThrows( + AuthException.class, + () -> + gApi.changes() + .id(r.getChangeId()) + .revision(r.getCommit().name()) + .description("test")); + assertThat(thrown).hasMessageThat().contains("edit description not permitted"); } @Test @@ -1317,8 +1363,8 @@ public class RevisionIT extends AbstractDaemonTest { @Test public void commentOnNonExistingFile() throws Exception { - PushOneCommit.Result r = createChange(); - r = updateChange(r, "new content"); + PushOneCommit.Result r1 = createChange(); + PushOneCommit.Result r2 = updateChange(r1, "new content"); CommentInput in = new CommentInput(); in.line = 1; in.message = "nit: trailing whitespace"; @@ -1329,10 +1375,14 @@ public class RevisionIT extends AbstractDaemonTest { reviewInput.comments = comments; reviewInput.message = "comment test"; - exception.expect(BadRequestException.class); - exception.expectMessage( - String.format("not found in revision %d,1", r.getChange().change().getId().get())); - gApi.changes().id(r.getChangeId()).revision(1).review(reviewInput); + BadRequestException thrown = + assertThrows( + BadRequestException.class, + () -> gApi.changes().id(r2.getChangeId()).revision(1).review(reviewInput)); + assertThat(thrown) + .hasMessageThat() + .contains( + String.format("not found in revision %d,1", r2.getChange().change().getId().get())); } @Test @@ -1360,9 +1410,11 @@ public class RevisionIT extends AbstractDaemonTest { String res = new String(os.toByteArray(), UTF_8); assertThat(res).isEqualTo(PATCH_FILE_ONLY); - exception.expect(ResourceNotFoundException.class); - exception.expectMessage("File not found: nonexistent-file."); - changeApi.revision(r.getCommit().name()).patch("nonexistent-file"); + ResourceNotFoundException thrown = + assertThrows( + ResourceNotFoundException.class, + () -> changeApi.revision(r.getCommit().name()).patch("nonexistent-file")); + assertThat(thrown).hasMessageThat().contains("File not found: nonexistent-file."); } @Test @@ -1410,13 +1462,16 @@ public class RevisionIT extends AbstractDaemonTest { // check if it's blocked to delete a vote on a non-current patch set. requestScopeOperations.setApiUser(admin.id()); - exception.expect(MethodNotAllowedException.class); - exception.expectMessage("Cannot access on non-current patch set"); - gApi.changes() - .id(r.getChangeId()) - .revision(r.getCommit().getName()) - .reviewer(user.id().toString()) - .deleteVote("Code-Review"); + MethodNotAllowedException thrown = + assertThrows( + MethodNotAllowedException.class, + () -> + gApi.changes() + .id(r.getChangeId()) + .revision(r.getCommit().getName()) + .reviewer(user.id().toString()) + .deleteVote("Code-Review")); + assertThat(thrown).hasMessageThat().contains("Cannot access on non-current patch set"); } @Test diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java index ba228f6e3b..62a7037f12 100644 --- a/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java +++ b/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java @@ -18,6 +18,7 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.gerrit.acceptance.PushOneCommit.SUBJECT; import static com.google.gerrit.extensions.common.testing.EditInfoSubject.assertThat; import static com.google.gerrit.extensions.common.testing.RobotCommentInfoSubject.assertThatList; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static java.util.stream.Collectors.toList; import com.google.common.collect.ImmutableList; @@ -164,9 +165,10 @@ public class RobotCommentsIT extends AbstractDaemonTest { int sizeOfRest = 451; fixReplacementInfo.replacement = getStringFor(defaultSizeLimit - sizeOfRest + 1); - exception.expect(BadRequestException.class); - exception.expectMessage("limit"); - addRobotComment(changeId, withFixRobotCommentInput); + BadRequestException thrown = + assertThrows( + BadRequestException.class, () -> addRobotComment(changeId, withFixRobotCommentInput)); + assertThat(thrown).hasMessageThat().contains("limit"); } @Test @@ -187,9 +189,10 @@ public class RobotCommentsIT extends AbstractDaemonTest { int sizeLimit = 10 * 1024; fixReplacementInfo.replacement = getStringFor(sizeLimit); - exception.expect(BadRequestException.class); - exception.expectMessage("limit"); - addRobotComment(changeId, withFixRobotCommentInput); + BadRequestException thrown = + assertThrows( + BadRequestException.class, () -> addRobotComment(changeId, withFixRobotCommentInput)); + assertThat(thrown).hasMessageThat().contains("limit"); } @Test @@ -254,12 +257,15 @@ public class RobotCommentsIT extends AbstractDaemonTest { public void descriptionOfFixSuggestionIsMandatory() throws Exception { fixSuggestionInfo.description = null; - exception.expect(BadRequestException.class); - exception.expectMessage( - String.format( - "A description is required for the suggested fix of the robot comment on %s", - withFixRobotCommentInput.path)); - addRobotComment(changeId, withFixRobotCommentInput); + BadRequestException thrown = + assertThrows( + BadRequestException.class, () -> addRobotComment(changeId, withFixRobotCommentInput)); + assertThat(thrown) + .hasMessageThat() + .contains( + String.format( + "A description is required for the suggested fix of the robot comment on %s", + withFixRobotCommentInput.path)); } @Test @@ -278,13 +284,16 @@ public class RobotCommentsIT extends AbstractDaemonTest { public void fixReplacementsAreMandatory() throws Exception { fixSuggestionInfo.replacements = Collections.emptyList(); - exception.expect(BadRequestException.class); - exception.expectMessage( - String.format( - "At least one replacement is required" - + " for the suggested fix of the robot comment on %s", - withFixRobotCommentInput.path)); - addRobotComment(changeId, withFixRobotCommentInput); + BadRequestException thrown = + assertThrows( + BadRequestException.class, () -> addRobotComment(changeId, withFixRobotCommentInput)); + assertThat(thrown) + .hasMessageThat() + .contains( + String.format( + "At least one replacement is required" + + " for the suggested fix of the robot comment on %s", + withFixRobotCommentInput.path)); } @Test @@ -305,12 +314,15 @@ public class RobotCommentsIT extends AbstractDaemonTest { public void pathOfFixReplacementIsMandatory() throws Exception { fixReplacementInfo.path = null; - exception.expect(BadRequestException.class); - exception.expectMessage( - String.format( - "A file path must be given for the replacement of the robot comment on %s", - withFixRobotCommentInput.path)); - addRobotComment(changeId, withFixRobotCommentInput); + BadRequestException thrown = + assertThrows( + BadRequestException.class, () -> addRobotComment(changeId, withFixRobotCommentInput)); + assertThat(thrown) + .hasMessageThat() + .contains( + String.format( + "A file path must be given for the replacement of the robot comment on %s", + withFixRobotCommentInput.path)); } @Test @@ -331,20 +343,24 @@ public class RobotCommentsIT extends AbstractDaemonTest { public void rangeOfFixReplacementIsMandatory() throws Exception { fixReplacementInfo.range = null; - exception.expect(BadRequestException.class); - exception.expectMessage( - String.format( - "A range must be given for the replacement of the robot comment on %s", - withFixRobotCommentInput.path)); - addRobotComment(changeId, withFixRobotCommentInput); + BadRequestException thrown = + assertThrows( + BadRequestException.class, () -> addRobotComment(changeId, withFixRobotCommentInput)); + assertThat(thrown) + .hasMessageThat() + .contains( + String.format( + "A range must be given for the replacement of the robot comment on %s", + withFixRobotCommentInput.path)); } @Test public void rangeOfFixReplacementNeedsToBeValid() throws Exception { fixReplacementInfo.range = createRange(13, 9, 5, 10); - exception.expect(BadRequestException.class); - exception.expectMessage("Range (13:9 - 5:10)"); - addRobotComment(changeId, withFixRobotCommentInput); + BadRequestException thrown = + assertThrows( + BadRequestException.class, () -> addRobotComment(changeId, withFixRobotCommentInput)); + assertThat(thrown).hasMessageThat().contains("Range (13:9 - 5:10)"); } @Test @@ -364,9 +380,10 @@ public class RobotCommentsIT extends AbstractDaemonTest { createFixSuggestionInfo(fixReplacementInfo1, fixReplacementInfo2); withFixRobotCommentInput.fixSuggestions = ImmutableList.of(fixSuggestionInfo); - exception.expect(BadRequestException.class); - exception.expectMessage("overlap"); - addRobotComment(changeId, withFixRobotCommentInput); + BadRequestException thrown = + assertThrows( + BadRequestException.class, () -> addRobotComment(changeId, withFixRobotCommentInput)); + assertThat(thrown).hasMessageThat().contains("overlap"); } @Test @@ -461,13 +478,16 @@ public class RobotCommentsIT extends AbstractDaemonTest { public void replacementStringOfFixReplacementIsMandatory() throws Exception { fixReplacementInfo.replacement = null; - exception.expect(BadRequestException.class); - exception.expectMessage( - String.format( - "A content for replacement must be " - + "indicated for the replacement of the robot comment on %s", - withFixRobotCommentInput.path)); - addRobotComment(changeId, withFixRobotCommentInput); + BadRequestException thrown = + assertThrows( + BadRequestException.class, () -> addRobotComment(changeId, withFixRobotCommentInput)); + assertThat(thrown) + .hasMessageThat() + .contains( + String.format( + "A content for replacement must be " + + "indicated for the replacement of the robot comment on %s", + withFixRobotCommentInput.path)); } @Test @@ -602,9 +622,11 @@ public class RobotCommentsIT extends AbstractDaemonTest { List fixIds = getFixIds(robotCommentInfos); gApi.changes().id(changeId).current().applyFix(fixIds.get(0)); - exception.expect(ResourceConflictException.class); - exception.expectMessage("merge"); - gApi.changes().id(changeId).current().applyFix(fixIds.get(1)); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, + () -> gApi.changes().id(changeId).current().applyFix(fixIds.get(1))); + assertThat(thrown).hasMessageThat().contains("merge"); } @Test @@ -708,8 +730,9 @@ public class RobotCommentsIT extends AbstractDaemonTest { List fixIds = getFixIds(robotCommentInfos); String fixId = Iterables.getOnlyElement(fixIds); - exception.expect(ResourceNotFoundException.class); - gApi.changes().id(changeId).current().applyFix(fixId); + assertThrows( + ResourceNotFoundException.class, + () -> gApi.changes().id(changeId).current().applyFix(fixId)); } @Test @@ -728,9 +751,11 @@ public class RobotCommentsIT extends AbstractDaemonTest { List fixIds = getFixIds(robotCommentInfos); String fixId = Iterables.getOnlyElement(fixIds); - exception.expect(ResourceConflictException.class); - exception.expectMessage("current"); - gApi.changes().id(changeId).revision(previousRevision).applyFix(fixId); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, + () -> gApi.changes().id(changeId).revision(previousRevision).applyFix(fixId)); + assertThat(thrown).hasMessageThat().contains("current"); } @Test @@ -783,9 +808,11 @@ public class RobotCommentsIT extends AbstractDaemonTest { List fixIds = getFixIds(robotCommentInfos); String fixId = Iterables.getOnlyElement(fixIds); - exception.expect(ResourceConflictException.class); - exception.expectMessage("based"); - gApi.changes().id(changeId).current().applyFix(fixId); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, + () -> gApi.changes().id(changeId).current().applyFix(fixId)); + assertThat(thrown).hasMessageThat().contains("based"); } @Test @@ -845,8 +872,9 @@ public class RobotCommentsIT extends AbstractDaemonTest { String fixId = Iterables.getOnlyElement(fixIds); String nonExistentFixId = fixId + "_non-existent"; - exception.expect(ResourceNotFoundException.class); - gApi.changes().id(changeId).current().applyFix(nonExistentFixId); + assertThrows( + ResourceNotFoundException.class, + () -> gApi.changes().id(changeId).current().applyFix(nonExistentFixId)); } @Test diff --git a/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java b/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java index 23020fde98..98420e6112 100644 --- a/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java +++ b/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java @@ -24,6 +24,7 @@ import static com.google.gerrit.extensions.common.testing.EditInfoSubject.assert import static com.google.gerrit.extensions.restapi.testing.BinaryResultSubject.assertThat; import static com.google.gerrit.reviewdb.client.Patch.COMMIT_MSG; import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.concurrent.TimeUnit.SECONDS; import static java.util.stream.Collectors.toList; @@ -325,9 +326,13 @@ public class ChangeEditIT extends AbstractDaemonTest { createEmptyEditFor(changeId); String commitMessage = gApi.changes().id(changeId).edit().getCommitMessage(); - exception.expect(ResourceConflictException.class); - exception.expectMessage("New commit message cannot be same as existing commit message"); - gApi.changes().id(changeId).edit().modifyCommitMessage(commitMessage); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, + () -> gApi.changes().id(changeId).edit().modifyCommitMessage(commitMessage)); + assertThat(thrown) + .hasMessageThat() + .contains("New commit message cannot be same as existing commit message"); } @Test @@ -335,9 +340,13 @@ public class ChangeEditIT extends AbstractDaemonTest { createEmptyEditFor(changeId); String commitMessage = gApi.changes().id(changeId).edit().getCommitMessage(); - exception.expect(ResourceConflictException.class); - exception.expectMessage("New commit message cannot be same as existing commit message"); - gApi.changes().id(changeId).edit().modifyCommitMessage(commitMessage + "\n\n"); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, + () -> gApi.changes().id(changeId).edit().modifyCommitMessage(commitMessage + "\n\n")); + assertThat(thrown) + .hasMessageThat() + .contains("New commit message cannot be same as existing commit message"); } @Test @@ -582,9 +591,15 @@ public class ChangeEditIT extends AbstractDaemonTest { @Test public void writeNoChanges() throws Exception { createEmptyEditFor(changeId); - exception.expect(ResourceConflictException.class); - exception.expectMessage("no changes were made"); - gApi.changes().id(changeId).edit().modifyFile(FILE_NAME, RawInputUtil.create(CONTENT_OLD)); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, + () -> + gApi.changes() + .id(changeId) + .edit() + .modifyFile(FILE_NAME, RawInputUtil.create(CONTENT_OLD))); + assertThat(thrown).hasMessageThat().contains("no changes were made"); } @Test @@ -692,8 +707,7 @@ public class ChangeEditIT extends AbstractDaemonTest { r1.assertOkStatus(); // Try to create edit as admin - exception.expect(AuthException.class); - createEmptyEditFor(r1.getChangeId()); + assertThrows(AuthException.class, () -> createEmptyEditFor(r1.getChangeId())); } @Test @@ -702,9 +716,11 @@ public class ChangeEditIT extends AbstractDaemonTest { gApi.changes().id(changeId).current().review(ReviewInput.approve()); gApi.changes().id(changeId).current().submit(); - exception.expect(ResourceConflictException.class); - exception.expectMessage(String.format("change %s is merged", change._number)); - createArbitraryEditFor(changeId); + ResourceConflictException thrown = + assertThrows(ResourceConflictException.class, () -> createArbitraryEditFor(changeId)); + assertThat(thrown) + .hasMessageThat() + .contains(String.format("change %s is merged", change._number)); } @Test @@ -712,9 +728,11 @@ public class ChangeEditIT extends AbstractDaemonTest { ChangeInfo change = gApi.changes().id(changeId).get(); gApi.changes().id(changeId).abandon(); - exception.expect(ResourceConflictException.class); - exception.expectMessage(String.format("change %s is abandoned", change._number)); - createArbitraryEditFor(changeId); + ResourceConflictException thrown = + assertThrows(ResourceConflictException.class, () -> createArbitraryEditFor(changeId)); + assertThat(thrown) + .hasMessageThat() + .contains(String.format("change %s is abandoned", change._number)); } private void createArbitraryEditFor(String changeId) throws Exception { diff --git a/javatests/com/google/gerrit/acceptance/git/GitmodulesIT.java b/javatests/com/google/gerrit/acceptance/git/GitmodulesIT.java index ac0cbd83e3..e2aa666275 100644 --- a/javatests/com/google/gerrit/acceptance/git/GitmodulesIT.java +++ b/javatests/com/google/gerrit/acceptance/git/GitmodulesIT.java @@ -14,6 +14,9 @@ package com.google.gerrit.acceptance.git; +import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; + import com.google.gerrit.acceptance.AbstractDaemonTest; import org.eclipse.jgit.api.errors.TransportException; import org.eclipse.jgit.junit.TestRepository; @@ -50,8 +53,15 @@ public class GitmodulesIT extends AbstractDaemonTest { .add(".gitmodules", config.toText()) .create(); - exception.expectMessage(expectedErrorMessage); - exception.expect(TransportException.class); - repo.git().push().setRemote("origin").setRefSpecs(new RefSpec("HEAD:refs/for/master")).call(); + TransportException thrown = + assertThrows( + TransportException.class, + () -> + repo.git() + .push() + .setRemote("origin") + .setRefSpecs(new RefSpec("HEAD:refs/for/master")) + .call()); + assertThat(thrown).hasMessageThat().contains(expectedErrorMessage); } } diff --git a/javatests/com/google/gerrit/acceptance/git/SubmoduleSubscriptionsWholeTopicMergeIT.java b/javatests/com/google/gerrit/acceptance/git/SubmoduleSubscriptionsWholeTopicMergeIT.java index 2cdce4df78..9ebc3de2db 100644 --- a/javatests/com/google/gerrit/acceptance/git/SubmoduleSubscriptionsWholeTopicMergeIT.java +++ b/javatests/com/google/gerrit/acceptance/git/SubmoduleSubscriptionsWholeTopicMergeIT.java @@ -17,13 +17,16 @@ package com.google.gerrit.acceptance.git; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; import static com.google.gerrit.acceptance.GitUtil.getChangeId; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import com.google.common.collect.ImmutableList; import com.google.gerrit.acceptance.NoHttpd; +import com.google.gerrit.acceptance.testsuite.ThrowingConsumer; import com.google.gerrit.common.data.Permission; import com.google.gerrit.extensions.api.changes.ReviewInput; import com.google.gerrit.extensions.client.ChangeStatus; import com.google.gerrit.extensions.client.SubmitType; +import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.reviewdb.client.BranchNameKey; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.server.change.TestSubmitInput; @@ -614,7 +617,7 @@ public class SubmoduleSubscriptionsWholeTopicMergeIT extends AbstractSubmoduleSu expectToHaveSubmoduleState(topRepo, "master", botKey, bottomRepo, "master"); } - private String prepareBranchCircularSubscription() throws Exception { + private void testBranchCircularSubscription(ThrowingConsumer apiCall) throws Exception { Project.NameKey topKey = createProjectForPush(getSubmitType()); Project.NameKey midKey = createProjectForPush(getSubmitType()); Project.NameKey botKey = createProjectForPush(getSubmitType()); @@ -634,23 +637,24 @@ public class SubmoduleSubscriptionsWholeTopicMergeIT extends AbstractSubmoduleSu String changeId = getChangeId(bottomRepo, bottomMasterHead).get(); approve(changeId); - exception.expectMessage("Branch level circular subscriptions detected"); - exception.expectMessage(topKey.get() + ",refs/heads/master"); - exception.expectMessage(midKey.get() + ",refs/heads/master"); - exception.expectMessage(botKey.get() + ",refs/heads/master"); - return changeId; + + ResourceConflictException thrown = + assertThrows(ResourceConflictException.class, () -> apiCall.accept(changeId)); + assertThat(thrown).hasMessageThat().contains("Branch level circular subscriptions detected"); + assertThat(thrown).hasMessageThat().contains(topKey.get() + ",refs/heads/master"); + assertThat(thrown).hasMessageThat().contains(midKey.get() + ",refs/heads/master"); + assertThat(thrown).hasMessageThat().contains(botKey.get() + ",refs/heads/master"); } @Test public void branchCircularSubscription() throws Exception { - String changeId = prepareBranchCircularSubscription(); - gApi.changes().id(changeId).current().submit(); + testBranchCircularSubscription(changeId -> gApi.changes().id(changeId).current().submit()); } @Test public void branchCircularSubscriptionPreview() throws Exception { - String changeId = prepareBranchCircularSubscription(); - gApi.changes().id(changeId).current().submitPreview(); + testBranchCircularSubscription( + changeId -> gApi.changes().id(changeId).current().submitPreview()); } @Test @@ -672,10 +676,13 @@ public class SubmoduleSubscriptionsWholeTopicMergeIT extends AbstractSubmoduleSu approve(getChangeId(subRepo, subMasterHead).get()); approve(getChangeId(superRepo, superDevHead).get()); - exception.expectMessage("Project level circular subscriptions detected"); - exception.expectMessage(subKey.get()); - exception.expectMessage(superKey.get()); - gApi.changes().id(getChangeId(subRepo, subMasterHead).get()).current().submit(); + Throwable thrown = + assertThrows( + Throwable.class, + () -> gApi.changes().id(getChangeId(subRepo, subMasterHead).get()).current().submit()); + assertThat(thrown).hasMessageThat().contains("Project level circular subscriptions detected"); + assertThat(thrown).hasMessageThat().contains(subKey.get()); + assertThat(thrown).hasMessageThat().contains(superKey.get()); } @Test diff --git a/javatests/com/google/gerrit/acceptance/rest/account/EmailIT.java b/javatests/com/google/gerrit/acceptance/rest/account/EmailIT.java index 84f218d091..5adf46f9db 100644 --- a/javatests/com/google/gerrit/acceptance/rest/account/EmailIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/account/EmailIT.java @@ -16,6 +16,7 @@ package com.google.gerrit.acceptance.rest.account; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth8.assertThat; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static java.util.stream.Collectors.toSet; import com.google.common.collect.ImmutableMultimap; @@ -149,16 +150,20 @@ public class EmailIT extends AbstractDaemonTest { @Test public void setPreferredEmailToNonExistingEmail() throws Exception { String email = "non-existing@example.com"; - exception.expect(ResourceNotFoundException.class); - exception.expectMessage("Not found: " + email); - gApi.accounts().self().email(email).setPreferred(); + ResourceNotFoundException thrown = + assertThrows( + ResourceNotFoundException.class, + () -> gApi.accounts().self().email(email).setPreferred()); + assertThat(thrown).hasMessageThat().contains("Not found: " + email); } @Test public void setPreferredEmailToEmailOfOtherAccount() throws Exception { - exception.expect(ResourceNotFoundException.class); - exception.expectMessage("Not found: " + user.email()); - gApi.accounts().self().email(user.email()).setPreferred(); + ResourceNotFoundException thrown = + assertThrows( + ResourceNotFoundException.class, + () -> gApi.accounts().self().email(user.email()).setPreferred()); + assertThat(thrown).hasMessageThat().contains("Not found: " + user.email()); } @Test @@ -201,9 +206,11 @@ public class EmailIT extends AbstractDaemonTest { Context oldCtx = createContextWithCustomRealm(new RealmWithAdditionalEmails(admin.id(), user.email())); try { - exception.expect(ResourceConflictException.class); - exception.expectMessage("email in use by another account"); - gApi.accounts().self().email(user.email()).setPreferred(); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, + () -> gApi.accounts().self().email(user.email()).setPreferred()); + assertThat(thrown).hasMessageThat().contains("email in use by another account"); } finally { atrScope.set(oldCtx); } @@ -248,9 +255,7 @@ public class EmailIT extends AbstractDaemonTest { // Now the email is no longer found requestScopeOperations.resetCurrentApiUser(); - emailApi = gApi.accounts().self().email(email); - exception.expect(ResourceNotFoundException.class); - emailApi.get(); + assertThrows(ResourceNotFoundException.class, () -> gApi.accounts().self().email(email).get()); } private Set getEmails() throws Exception { diff --git a/javatests/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java b/javatests/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java index 998a2a4923..c2e2a11092 100644 --- a/javatests/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java @@ -22,6 +22,7 @@ import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_MAI import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_USERNAME; import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_UUID; import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.stream.Collectors.toList; import static org.eclipse.jgit.lib.Constants.OBJ_BLOB; @@ -112,9 +113,10 @@ public class ExternalIdIT extends AbstractDaemonTest { @Test public void getExternalIdsOfOtherUserNotAllowed() throws Exception { requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - exception.expectMessage("access database not permitted"); - gApi.accounts().id(admin.id().get()).getExternalIds(); + AuthException thrown = + assertThrows( + AuthException.class, () -> gApi.accounts().id(admin.id().get()).getExternalIds()); + assertThat(thrown).hasMessageThat().contains("access database not permitted"); } @Test @@ -165,22 +167,30 @@ public class ExternalIdIT extends AbstractDaemonTest { public void deleteExternalIdsOfOtherUserNotAllowed() throws Exception { List extIds = gApi.accounts().self().getExternalIds(); requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - exception.expectMessage("access database not permitted"); - gApi.accounts() - .id(admin.id().get()) - .deleteExternalIds(extIds.stream().map(e -> e.identity).collect(toList())); + AuthException thrown = + assertThrows( + AuthException.class, + () -> + gApi.accounts() + .id(admin.id().get()) + .deleteExternalIds(extIds.stream().map(e -> e.identity).collect(toList()))); + assertThat(thrown).hasMessageThat().contains("access database not permitted"); } @Test public void deleteExternalIdOfOtherUserUnderOwnAccount_UnprocessableEntity() throws Exception { List extIds = gApi.accounts().self().getExternalIds(); requestScopeOperations.setApiUser(user.id()); - exception.expect(UnprocessableEntityException.class); - exception.expectMessage(String.format("External id %s does not exist", extIds.get(0).identity)); - gApi.accounts() - .self() - .deleteExternalIds(extIds.stream().map(e -> e.identity).collect(toList())); + UnprocessableEntityException thrown = + assertThrows( + UnprocessableEntityException.class, + () -> + gApi.accounts() + .self() + .deleteExternalIds(extIds.stream().map(e -> e.identity).collect(toList()))); + assertThat(thrown) + .hasMessageThat() + .contains(String.format("External id %s does not exist", extIds.get(0).identity)); } @Test @@ -446,9 +456,11 @@ public class ExternalIdIT extends AbstractDaemonTest { @Test public void checkConsistencyNotAllowed() throws Exception { - exception.expect(AuthException.class); - exception.expectMessage("access database not permitted"); - gApi.config().server().checkConsistency(new ConsistencyCheckInput()); + AuthException thrown = + assertThrows( + AuthException.class, + () -> gApi.config().server().checkConsistency(new ConsistencyCheckInput())); + assertThat(thrown).hasMessageThat().contains("access database not permitted"); } private ConsistencyProblemInfo consistencyError(String message) { @@ -760,8 +772,7 @@ public class ExternalIdIT extends AbstractDaemonTest { // update external ID branch so that external IDs need to be reloaded insertExtIdBehindGerritsBack(ExternalId.create("foo", "bar", admin.id())); - exception.expect(IOException.class); - externalIds.byAccount(admin.id()); + assertThrows(IOException.class, () -> externalIds.byAccount(admin.id())); } } @@ -771,8 +782,7 @@ public class ExternalIdIT extends AbstractDaemonTest { // update external ID branch so that external IDs need to be reloaded insertExtIdBehindGerritsBack(ExternalId.create("foo", "bar", admin.id())); - exception.expect(IOException.class); - externalIds.byEmail(admin.email()); + assertThrows(IOException.class, () -> externalIds.byEmail(admin.email())); } } diff --git a/javatests/com/google/gerrit/acceptance/rest/account/GetAccountIT.java b/javatests/com/google/gerrit/acceptance/rest/account/GetAccountIT.java index 11f7c0f0af..931dacebed 100644 --- a/javatests/com/google/gerrit/acceptance/rest/account/GetAccountIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/account/GetAccountIT.java @@ -15,6 +15,7 @@ package com.google.gerrit.acceptance.rest.account; import static com.google.gerrit.acceptance.rest.account.AccountAssert.assertAccountInfo; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.NoHttpd; @@ -24,9 +25,9 @@ import org.junit.Test; @NoHttpd public class GetAccountIT extends AbstractDaemonTest { - @Test(expected = ResourceNotFoundException.class) + @Test public void getNonExistingAccount_NotFound() throws Exception { - gApi.accounts().id("non-existing").get(); + assertThrows(ResourceNotFoundException.class, () -> gApi.accounts().id("non-existing").get()); } @Test diff --git a/javatests/com/google/gerrit/acceptance/rest/account/ImpersonationIT.java b/javatests/com/google/gerrit/acceptance/rest/account/ImpersonationIT.java index 0f7fdbb8f6..b1dd254446 100644 --- a/javatests/com/google/gerrit/acceptance/rest/account/ImpersonationIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/account/ImpersonationIT.java @@ -18,6 +18,7 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.gerrit.extensions.client.ListChangesOption.MESSAGES; import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS; import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -129,9 +130,10 @@ public class ImpersonationIT extends AbstractDaemonTest { in.onBehalfOf = user.id().toString(); in.message = "Message on behalf of"; - exception.expect(AuthException.class); - exception.expectMessage("label required to post review on behalf of \"" + in.onBehalfOf + '"'); - revision.review(in); + AuthException thrown = assertThrows(AuthException.class, () -> revision.review(in)); + assertThat(thrown) + .hasMessageThat() + .contains("label required to post review on behalf of \"" + in.onBehalfOf + '"'); } @Test @@ -143,9 +145,10 @@ public class ImpersonationIT extends AbstractDaemonTest { ReviewInput in = new ReviewInput().label("Not-A-Label", 5); in.onBehalfOf = user.id().toString(); - exception.expect(BadRequestException.class); - exception.expectMessage("label \"Not-A-Label\" is not a configured label"); - gApi.changes().id(changeId).current().review(in); + BadRequestException thrown = + assertThrows( + BadRequestException.class, () -> gApi.changes().id(changeId).current().review(in)); + assertThat(thrown).hasMessageThat().contains("label \"Not-A-Label\" is not a configured label"); } @Test @@ -175,10 +178,11 @@ public class ImpersonationIT extends AbstractDaemonTest { in.onBehalfOf = user.id().toString(); in.label("Verified", 1); - exception.expect(AuthException.class); - exception.expectMessage( - "not permitted to modify label \"Verified\" on behalf of \"" + in.onBehalfOf + '"'); - revision.review(in); + AuthException thrown = assertThrows(AuthException.class, () -> revision.review(in)); + assertThat(thrown) + .hasMessageThat() + .contains( + "not permitted to modify label \"Verified\" on behalf of \"" + in.onBehalfOf + '"'); } @Test @@ -266,9 +270,10 @@ public class ImpersonationIT extends AbstractDaemonTest { in.label("Code-Review", 1); in.drafts = DraftHandling.PUBLISH; - exception.expect(AuthException.class); - exception.expectMessage("not allowed to modify other user's drafts"); - gApi.changes().id(r.getChangeId()).current().review(in); + AuthException thrown = + assertThrows( + AuthException.class, () -> gApi.changes().id(r.getChangeId()).current().review(in)); + assertThat(thrown).hasMessageThat().contains("not allowed to modify other user's drafts"); } @Test @@ -281,10 +286,10 @@ public class ImpersonationIT extends AbstractDaemonTest { in.onBehalfOf = "doesnotexist"; in.label("Code-Review", 1); - exception.expect(UnprocessableEntityException.class); - exception.expectMessage("not found"); - exception.expectMessage("doesnotexist"); - revision.review(in); + UnprocessableEntityException thrown = + assertThrows(UnprocessableEntityException.class, () -> revision.review(in)); + assertThat(thrown).hasMessageThat().contains("not found"); + assertThat(thrown).hasMessageThat().contains("doesnotexist"); } @Test @@ -299,9 +304,11 @@ public class ImpersonationIT extends AbstractDaemonTest { in.onBehalfOf = user.id().toString(); in.label("Code-Review", 1); - exception.expect(UnprocessableEntityException.class); - exception.expectMessage("on_behalf_of account " + user.id() + " cannot see change"); - revision.review(in); + UnprocessableEntityException thrown = + assertThrows(UnprocessableEntityException.class, () -> revision.review(in)); + assertThat(thrown) + .hasMessageThat() + .contains("on_behalf_of account " + user.id() + " cannot see change"); } @GerritConfig(name = "accounts.visibility", value = "SAME_GROUP") @@ -318,10 +325,10 @@ public class ImpersonationIT extends AbstractDaemonTest { in.onBehalfOf = user.id().toString(); in.label("Code-Review", 1); - exception.expect(UnprocessableEntityException.class); - exception.expectMessage("not found"); - exception.expectMessage(in.onBehalfOf); - revision.review(in); + UnprocessableEntityException thrown = + assertThrows(UnprocessableEntityException.class, () -> revision.review(in)); + assertThat(thrown).hasMessageThat().contains("not found"); + assertThat(thrown).hasMessageThat().contains(in.onBehalfOf); } @Test @@ -350,10 +357,12 @@ public class ImpersonationIT extends AbstractDaemonTest { gApi.changes().id(changeId).current().review(ReviewInput.approve()); SubmitInput in = new SubmitInput(); in.onBehalfOf = "doesnotexist"; - exception.expect(UnprocessableEntityException.class); - exception.expectMessage("not found"); - exception.expectMessage("doesnotexist"); - gApi.changes().id(changeId).current().submit(in); + UnprocessableEntityException thrown = + assertThrows( + UnprocessableEntityException.class, + () -> gApi.changes().id(changeId).current().submit(in)); + assertThat(thrown).hasMessageThat().contains("not found"); + assertThat(thrown).hasMessageThat().contains("doesnotexist"); } @Test @@ -365,9 +374,15 @@ public class ImpersonationIT extends AbstractDaemonTest { .review(ReviewInput.approve()); SubmitInput in = new SubmitInput(); in.onBehalfOf = admin2.email(); - exception.expect(AuthException.class); - exception.expectMessage("submit on behalf of other users not permitted"); - gApi.changes().id(project.get() + "~master~" + r.getChangeId()).current().submit(in); + AuthException thrown = + assertThrows( + AuthException.class, + () -> + gApi.changes() + .id(project.get() + "~master~" + r.getChangeId()) + .current() + .submit(in)); + assertThat(thrown).hasMessageThat().contains("submit on behalf of other users not permitted"); } @Test @@ -380,9 +395,13 @@ public class ImpersonationIT extends AbstractDaemonTest { gApi.changes().id(changeId).current().review(ReviewInput.approve()); SubmitInput in = new SubmitInput(); in.onBehalfOf = user.email(); - exception.expect(UnprocessableEntityException.class); - exception.expectMessage("on_behalf_of account " + user.id() + " cannot see change"); - gApi.changes().id(changeId).current().submit(in); + UnprocessableEntityException thrown = + assertThrows( + UnprocessableEntityException.class, + () -> gApi.changes().id(changeId).current().submit(in)); + assertThat(thrown) + .hasMessageThat() + .contains("on_behalf_of account " + user.id() + " cannot see change"); } @GerritConfig(name = "accounts.visibility", value = "SAME_GROUP") @@ -397,10 +416,12 @@ public class ImpersonationIT extends AbstractDaemonTest { gApi.changes().id(changeId).current().review(ReviewInput.approve()); SubmitInput in = new SubmitInput(); in.onBehalfOf = user.email(); - exception.expect(UnprocessableEntityException.class); - exception.expectMessage("not found"); - exception.expectMessage(in.onBehalfOf); - gApi.changes().id(changeId).current().submit(in); + UnprocessableEntityException thrown = + assertThrows( + UnprocessableEntityException.class, + () -> gApi.changes().id(changeId).current().submit(in)); + assertThat(thrown).hasMessageThat().contains("not found"); + assertThat(thrown).hasMessageThat().contains(in.onBehalfOf); } @Test diff --git a/javatests/com/google/gerrit/acceptance/rest/account/WatchedProjectsIT.java b/javatests/com/google/gerrit/acceptance/rest/account/WatchedProjectsIT.java index d0c1fa4ff0..2c9107c325 100644 --- a/javatests/com/google/gerrit/acceptance/rest/account/WatchedProjectsIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/account/WatchedProjectsIT.java @@ -15,6 +15,7 @@ package com.google.gerrit.acceptance.rest.account; import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import com.google.common.collect.Lists; import com.google.gerrit.acceptance.AbstractDaemonTest; @@ -114,9 +115,11 @@ public class WatchedProjectsIT extends AbstractDaemonTest { pwi.notifyNewPatchSets = true; projectsToWatch.add(pwi); - exception.expect(BadRequestException.class); - exception.expectMessage("duplicate entry for project " + projectName); - gApi.accounts().self().setWatchedProjects(projectsToWatch); + BadRequestException thrown = + assertThrows( + BadRequestException.class, + () -> gApi.accounts().self().setWatchedProjects(projectsToWatch)); + assertThat(thrown).hasMessageThat().contains("duplicate entry for project " + projectName); } @Test @@ -146,9 +149,9 @@ public class WatchedProjectsIT extends AbstractDaemonTest { pwi.notifyNewChanges = true; pwi.notifyAllComments = true; projectsToWatch.add(pwi); - - exception.expect(UnprocessableEntityException.class); - gApi.accounts().self().setWatchedProjects(projectsToWatch); + assertThrows( + UnprocessableEntityException.class, + () -> gApi.accounts().self().setWatchedProjects(projectsToWatch)); } @Test diff --git a/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java b/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java index de986ea5a0..f2fc3c5aeb 100644 --- a/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java +++ b/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java @@ -24,6 +24,7 @@ import static com.google.gerrit.extensions.client.ListChangesOption.DETAILED_LAB import static com.google.gerrit.extensions.client.ListChangesOption.SUBMITTABLE; import static com.google.gerrit.server.group.SystemGroupBackend.CHANGE_OWNER; import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static java.util.concurrent.TimeUnit.SECONDS; import static java.util.stream.Collectors.toList; @@ -1019,13 +1020,15 @@ public abstract class AbstractSubmit extends AbstractDaemonTest { ChangeApi revert2 = gApi.changes().id(change.getChangeId()).revert(); approve(revert2.id()); - exception.expect(ResourceConflictException.class); - exception.expectMessage( - "Change " - + revert2.get()._number - + ": Change could not be merged because the commit is empty. " - + "Project policy requires all commits to contain modifications to at least one file."); - revert2.current().submit(); + ResourceConflictException thrown = + assertThrows(ResourceConflictException.class, () -> revert2.current().submit()); + assertThat(thrown) + .hasMessageThat() + .contains( + "Change " + + revert2.get()._number + + ": Change could not be merged because the commit is empty. Project policy" + + " requires all commits to contain modifications to at least one file."); } @Test @@ -1050,13 +1053,15 @@ public abstract class AbstractSubmit extends AbstractDaemonTest { ChangeApi change = gApi.changes().create(ci); approve(change.id()); - exception.expect(ResourceConflictException.class); - exception.expectMessage( - "Change " - + change.get()._number - + ": Change could not be merged because the commit is empty. " - + "Project policy requires all commits to contain modifications to at least one file."); - change.current().submit(); + ResourceConflictException thrown = + assertThrows(ResourceConflictException.class, () -> change.current().submit()); + assertThat(thrown) + .hasMessageThat() + .contains( + "Change " + + change.get()._number + + ": Change could not be merged because the commit is empty. Project policy" + + " requires all commits to contain modifications to at least one file."); } @Test diff --git a/javatests/com/google/gerrit/acceptance/rest/change/AssigneeIT.java b/javatests/com/google/gerrit/acceptance/rest/change/AssigneeIT.java index 2d6227bd47..bdb710c254 100644 --- a/javatests/com/google/gerrit/acceptance/rest/change/AssigneeIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/change/AssigneeIT.java @@ -18,6 +18,7 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assert_; import static com.google.gerrit.extensions.client.ListChangesOption.DETAILED_LABELS; import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static java.util.concurrent.TimeUnit.SECONDS; import com.google.common.collect.Iterables; @@ -159,18 +160,16 @@ public class AssigneeIT extends AbstractDaemonTest { git().fetch().setRefSpecs(new RefSpec("refs/meta/config:refs/meta/config")).call(); testRepo.reset(RefNames.REFS_CONFIG); PushOneCommit.Result r = createChange("refs/for/refs/meta/config"); - exception.expect(AuthException.class); - exception.expectMessage("read not permitted"); - setAssignee(r, user.email()); + AuthException thrown = assertThrows(AuthException.class, () -> setAssignee(r, user.email())); + assertThat(thrown).hasMessageThat().contains("read not permitted"); } @Test public void setAssigneeNotAllowedWithoutPermission() throws Exception { PushOneCommit.Result r = createChange(); requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - exception.expectMessage("not permitted"); - setAssignee(r, user.email()); + AuthException thrown = assertThrows(AuthException.class, () -> setAssignee(r, user.email())); + assertThat(thrown).hasMessageThat().contains("not permitted"); } @Test diff --git a/javatests/com/google/gerrit/acceptance/rest/change/ChangeOwnerIT.java b/javatests/com/google/gerrit/acceptance/rest/change/ChangeOwnerIT.java index d51221e1b2..41f31470c0 100644 --- a/javatests/com/google/gerrit/acceptance/rest/change/ChangeOwnerIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/change/ChangeOwnerIT.java @@ -14,6 +14,8 @@ package com.google.gerrit.acceptance.rest.change; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; + import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.AcceptanceTestRequestScope.Context; import com.google.gerrit.acceptance.PushOneCommit; @@ -121,8 +123,7 @@ public class ChangeOwnerIT extends AbstractDaemonTest { } private void assertApproveFails(TestAccount a, String changeId) throws Exception { - exception.expect(AuthException.class); - approve(a, changeId); + assertThrows(AuthException.class, () -> approve(a, changeId)); } private void grantApproveToChangeOwner(Project.NameKey project) throws Exception { diff --git a/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java b/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java index 173b78d9c2..76f6b98f62 100644 --- a/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java @@ -20,6 +20,7 @@ import static com.google.gerrit.extensions.client.ReviewerState.CC; import static com.google.gerrit.extensions.client.ReviewerState.REMOVED; import static com.google.gerrit.extensions.client.ReviewerState.REVIEWER; import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST; import static javax.servlet.http.HttpServletResponse.SC_OK; @@ -662,9 +663,11 @@ public class ChangeReviewersIT extends AbstractDaemonTest { requestScopeOperations.setApiUser(user.id()); gApi.changes().id(r.getChangeId()).current().review(new ReviewInput().label("Code-Review", 1)); requestScopeOperations.setApiUser(newUser.id()); - exception.expect(AuthException.class); - exception.expectMessage("remove reviewer not permitted"); - gApi.changes().id(r.getChangeId()).reviewer(user.email()).remove(); + AuthException thrown = + assertThrows( + AuthException.class, + () -> gApi.changes().id(r.getChangeId()).reviewer(user.email()).remove()); + assertThat(thrown).hasMessageThat().contains("remove reviewer not permitted"); } @Test @@ -690,9 +693,11 @@ public class ChangeReviewersIT extends AbstractDaemonTest { gApi.changes().id(r.getChangeId()).addReviewer(user.email()); requestScopeOperations.setApiUser(newUser.id()); - exception.expect(AuthException.class); - exception.expectMessage("remove reviewer not permitted"); - gApi.changes().id(r.getChangeId()).reviewer(user.email()).remove(); + AuthException thrown = + assertThrows( + AuthException.class, + () -> gApi.changes().id(r.getChangeId()).reviewer(user.email()).remove()); + assertThat(thrown).hasMessageThat().contains("remove reviewer not permitted"); } @Test @@ -705,9 +710,11 @@ public class ChangeReviewersIT extends AbstractDaemonTest { input.state = ReviewerState.CC; gApi.changes().id(r.getChangeId()).addReviewer(input); requestScopeOperations.setApiUser(newUser.id()); - exception.expect(AuthException.class); - exception.expectMessage("remove reviewer not permitted"); - gApi.changes().id(r.getChangeId()).reviewer(user.email()).remove(); + AuthException thrown = + assertThrows( + AuthException.class, + () -> gApi.changes().id(r.getChangeId()).reviewer(user.email()).remove()); + assertThat(thrown).hasMessageThat().contains("remove reviewer not permitted"); } @Test diff --git a/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java b/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java index d94b02f2c5..2eb85d2869 100644 --- a/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java @@ -18,6 +18,7 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.gerrit.common.data.Permission.READ; import static com.google.gerrit.reviewdb.client.RefNames.changeMetaRef; import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static java.util.concurrent.TimeUnit.SECONDS; import static org.eclipse.jgit.lib.Constants.SIGNED_OFF_BY_TAG; @@ -507,9 +508,8 @@ public class CreateChangeIT extends AbstractDaemonTest { private void assertCreateFails( ChangeInput in, Class errType, String errSubstring) throws Exception { - exception.expect(errType); - exception.expectMessage(errSubstring); - gApi.changes().create(in); + Throwable thrown = assertThrows(errType, () -> gApi.changes().create(in)); + assertThat(thrown).hasMessageThat().contains(errSubstring); } // TODO(davido): Expose setting of account preferences in the API diff --git a/javatests/com/google/gerrit/acceptance/rest/change/HashtagsIT.java b/javatests/com/google/gerrit/acceptance/rest/change/HashtagsIT.java index 186bf82929..0c5498b537 100644 --- a/javatests/com/google/gerrit/acceptance/rest/change/HashtagsIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/change/HashtagsIT.java @@ -17,6 +17,7 @@ package com.google.gerrit.acceptance.rest.change; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static java.util.Objects.requireNonNull; import static java.util.concurrent.TimeUnit.SECONDS; @@ -79,9 +80,9 @@ public class HashtagsIT extends AbstractDaemonTest { public void addInvalidHashtag() throws Exception { PushOneCommit.Result r = createChange(); - exception.expect(BadRequestException.class); - exception.expectMessage("hashtags may not contain commas"); - addHashtags(r, "invalid,hashtag"); + BadRequestException thrown = + assertThrows(BadRequestException.class, () -> addHashtags(r, "invalid,hashtag")); + assertThat(thrown).hasMessageThat().contains("hashtags may not contain commas"); } @Test @@ -259,9 +260,8 @@ public class HashtagsIT extends AbstractDaemonTest { public void addHashtagWithoutPermissionNotAllowed() throws Exception { PushOneCommit.Result r = createChange(); requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - exception.expectMessage("edit hashtags not permitted"); - addHashtags(r, "MyHashtag"); + AuthException thrown = assertThrows(AuthException.class, () -> addHashtags(r, "MyHashtag")); + assertThat(thrown).hasMessageThat().contains("edit hashtags not permitted"); } @Test diff --git a/javatests/com/google/gerrit/acceptance/rest/change/MoveChangeIT.java b/javatests/com/google/gerrit/acceptance/rest/change/MoveChangeIT.java index f2a39523c6..696e161d82 100644 --- a/javatests/com/google/gerrit/acceptance/rest/change/MoveChangeIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/change/MoveChangeIT.java @@ -17,6 +17,7 @@ package com.google.gerrit.acceptance.rest.change; import static com.google.common.truth.Truth.assertThat; import static com.google.gerrit.acceptance.GitUtil.pushHead; import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.GerritConfig; @@ -90,9 +91,13 @@ public class MoveChangeIT extends AbstractDaemonTest { public void moveChangeToSameRefAsCurrent() throws Exception { // Move change to the branch same as change's destination PushOneCommit.Result r = createChange(); - exception.expect(ResourceConflictException.class); - exception.expectMessage("Change is already destined for the specified branch"); - move(r.getChangeId(), r.getChange().change().getDest().branch()); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, + () -> move(r.getChangeId(), r.getChange().change().getDest().branch())); + assertThat(thrown) + .hasMessageThat() + .contains("Change is already destined for the specified branch"); } @Test @@ -103,13 +108,15 @@ public class MoveChangeIT extends AbstractDaemonTest { createBranch(newBranch); int changeNum = r.getChange().change().getChangeId(); createChange(newBranch.branch(), r.getChangeId()); - exception.expect(ResourceConflictException.class); - exception.expectMessage( - "Destination " - + newBranch.shortName() - + " has a different change with same change key " - + r.getChangeId()); - move(changeNum, newBranch.branch()); + ResourceConflictException thrown = + assertThrows(ResourceConflictException.class, () -> move(changeNum, newBranch.branch())); + assertThat(thrown) + .hasMessageThat() + .contains( + "Destination " + + newBranch.shortName() + + " has a different change with same change key " + + r.getChangeId()); } @Test @@ -118,9 +125,12 @@ public class MoveChangeIT extends AbstractDaemonTest { PushOneCommit.Result r = createChange(); BranchNameKey newBranch = BranchNameKey.create(r.getChange().change().getProject(), "does_not_exist"); - exception.expect(ResourceConflictException.class); - exception.expectMessage("Destination " + newBranch.branch() + " not found in the project"); - move(r.getChangeId(), newBranch.branch()); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, () -> move(r.getChangeId(), newBranch.branch())); + assertThat(thrown) + .hasMessageThat() + .contains("Destination " + newBranch.branch() + " not found in the project"); } @Test @@ -130,9 +140,10 @@ public class MoveChangeIT extends AbstractDaemonTest { BranchNameKey newBranch = BranchNameKey.create(r.getChange().change().getProject(), "moveTest"); createBranch(newBranch); merge(r); - exception.expect(ResourceConflictException.class); - exception.expectMessage("Change is merged"); - move(r.getChangeId(), newBranch.branch()); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, () -> move(r.getChangeId(), newBranch.branch())); + assertThat(thrown).hasMessageThat().contains("Change is merged"); } @Test @@ -156,9 +167,11 @@ public class MoveChangeIT extends AbstractDaemonTest { BranchNameKey newBranch = BranchNameKey.create(r1.getChange().change().getProject(), "moveTest"); createBranch(newBranch); - exception.expect(ResourceConflictException.class); - exception.expectMessage("Merge commit cannot be moved"); - move(GitUtil.getChangeId(testRepo, c).get(), newBranch.branch()); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, + () -> move(GitUtil.getChangeId(testRepo, c).get(), newBranch.branch())); + assertThat(thrown).hasMessageThat().contains("Merge commit cannot be moved"); } @Test @@ -172,9 +185,9 @@ public class MoveChangeIT extends AbstractDaemonTest { "refs/for/" + newBranch.branch(), Permission.PUSH, systemGroupBackend.getGroup(REGISTERED_USERS).getUUID()); - exception.expect(AuthException.class); - exception.expectMessage("move not permitted"); - move(r.getChangeId(), newBranch.branch()); + AuthException thrown = + assertThrows(AuthException.class, () -> move(r.getChangeId(), newBranch.branch())); + assertThat(thrown).hasMessageThat().contains("move not permitted"); } @Test @@ -188,9 +201,9 @@ public class MoveChangeIT extends AbstractDaemonTest { Permission.ABANDON, systemGroupBackend.getGroup(REGISTERED_USERS).getUUID()); requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - exception.expectMessage("move not permitted"); - move(r.getChangeId(), newBranch.branch()); + AuthException thrown = + assertThrows(AuthException.class, () -> move(r.getChangeId(), newBranch.branch())); + assertThat(thrown).hasMessageThat().contains("move not permitted"); } @Test @@ -209,10 +222,11 @@ public class MoveChangeIT extends AbstractDaemonTest { gApi.projects().name(newBranch.project().get()).branch(newBranch.branch()).create(bi); // Try to move the change to the branch with the same commit - exception.expect(ResourceConflictException.class); - exception.expectMessage( - "Current patchset revision is reachable from tip of " + newBranch.branch()); - move(changeNum, newBranch.branch()); + ResourceConflictException thrown = + assertThrows(ResourceConflictException.class, () -> move(changeNum, newBranch.branch())); + assertThat(thrown) + .hasMessageThat() + .contains("Current patchset revision is reachable from tip of " + newBranch.branch()); } @Test @@ -238,10 +252,13 @@ public class MoveChangeIT extends AbstractDaemonTest { grant(project, "refs/heads/*", Permission.LABEL + "Patch-Set-Lock"); revision(r).review(new ReviewInput().label("Patch-Set-Lock", 1)); - exception.expect(ResourceConflictException.class); - exception.expectMessage( - String.format("The current patch set of change %s is locked", r.getChange().getId())); - move(r.getChangeId(), newBranch.branch()); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, () -> move(r.getChangeId(), newBranch.branch())); + assertThat(thrown) + .hasMessageThat() + .contains( + String.format("The current patch set of change %s is locked", r.getChange().getId())); } @Test @@ -334,9 +351,9 @@ public class MoveChangeIT extends AbstractDaemonTest { public void moveNoDestinationBranchSpecified() throws Exception { PushOneCommit.Result r = createChange(); - exception.expect(BadRequestException.class); - exception.expectMessage("destination branch is required"); - move(r.getChangeId(), null); + BadRequestException thrown = + assertThrows(BadRequestException.class, () -> move(r.getChangeId(), null)); + assertThat(thrown).hasMessageThat().contains("destination branch is required"); } @Test @@ -344,9 +361,9 @@ public class MoveChangeIT extends AbstractDaemonTest { public void moveCanBeDisabledByConfig() throws Exception { PushOneCommit.Result r = createChange(); - exception.expect(MethodNotAllowedException.class); - exception.expectMessage("move changes endpoint is disabled"); - move(r.getChangeId(), null); + MethodNotAllowedException thrown = + assertThrows(MethodNotAllowedException.class, () -> move(r.getChangeId(), null)); + assertThat(thrown).hasMessageThat().contains("move changes endpoint is disabled"); } private void move(int changeNum, String destination) throws RestApiException { diff --git a/javatests/com/google/gerrit/acceptance/rest/change/PrivateByDefaultIT.java b/javatests/com/google/gerrit/acceptance/rest/change/PrivateByDefaultIT.java index ca4288f636..e0bca3a66c 100644 --- a/javatests/com/google/gerrit/acceptance/rest/change/PrivateByDefaultIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/change/PrivateByDefaultIT.java @@ -15,6 +15,7 @@ package com.google.gerrit.acceptance.rest.change; import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.GerritConfig; @@ -81,9 +82,9 @@ public class PrivateByDefaultIT extends AbstractDaemonTest { setPrivateByDefault(project2, InheritableBoolean.TRUE); ChangeInput input = new ChangeInput(project2.get(), "master", "empty change"); - exception.expect(MethodNotAllowedException.class); - exception.expectMessage("private changes are disabled"); - gApi.changes().create(input); + MethodNotAllowedException thrown = + assertThrows(MethodNotAllowedException.class, () -> gApi.changes().create(input)); + assertThat(thrown).hasMessageThat().contains("private changes are disabled"); } @Test diff --git a/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java b/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java index 72af07583e..8d62765123 100644 --- a/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java @@ -17,6 +17,7 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth8.assertThat; import static com.google.gerrit.extensions.client.ListChangesOption.MESSAGES; import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.GitUtil; @@ -143,8 +144,7 @@ public class AccessIT extends AbstractDaemonTest { @Test public void createAccessChangeNop() throws Exception { ProjectAccessInput accessInput = newProjectAccessInput(); - exception.expect(BadRequestException.class); - pApi().accessChange(accessInput); + assertThrows(BadRequestException.class, () -> pApi().accessChange(accessInput)); } @Test @@ -326,8 +326,7 @@ public class AccessIT extends AbstractDaemonTest { pApi().access(accessInput); requestScopeOperations.setApiUser(user.id()); - exception.expect(ResourceNotFoundException.class); - pApi().access(); + assertThrows(ResourceNotFoundException.class, () -> pApi().access()); } @Test @@ -346,8 +345,7 @@ public class AccessIT extends AbstractDaemonTest { accessInfoToApply.add.put(REFS_HEADS, accessSectionInfoToApply); requestScopeOperations.setApiUser(user.id()); - exception.expect(ResourceNotFoundException.class); - pApi().access(); + assertThrows(ResourceNotFoundException.class, () -> pApi().access()); } @Test @@ -409,9 +407,8 @@ public class AccessIT extends AbstractDaemonTest { accessInput.parent = newParentProjectName; requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - exception.expectMessage("administrate server not permitted"); - pApi().access(accessInput); + AuthException thrown = assertThrows(AuthException.class, () -> pApi().access(accessInput)); + assertThat(thrown).hasMessageThat().contains("administrate server not permitted"); } @Test @@ -436,8 +433,8 @@ public class AccessIT extends AbstractDaemonTest { accessInput.add.put(AccessSection.GLOBAL_CAPABILITIES, accessSectionInfo); requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - gApi.projects().name(allProjects.get()).access(accessInput); + assertThrows( + AuthException.class, () -> gApi.projects().name(allProjects.get()).access(accessInput)); } @Test @@ -465,8 +462,7 @@ public class AccessIT extends AbstractDaemonTest { accessInput.add.put(AccessSection.GLOBAL_CAPABILITIES, accessSectionInfo); - exception.expect(BadRequestException.class); - pApi().access(accessInput); + assertThrows(BadRequestException.class, () -> pApi().access(accessInput)); } @Test @@ -479,9 +475,9 @@ public class AccessIT extends AbstractDaemonTest { accessSectionInfo.permissions.put(Permission.PUSH, permissionInfo); accessInput.add.put(AccessSection.GLOBAL_CAPABILITIES, accessSectionInfo); - - exception.expect(BadRequestException.class); - gApi.projects().name(allProjects.get()).access(accessInput); + assertThrows( + BadRequestException.class, + () -> gApi.projects().name(allProjects.get()).access(accessInput)); } @Test @@ -492,8 +488,8 @@ public class AccessIT extends AbstractDaemonTest { accessInput.remove.put(AccessSection.GLOBAL_CAPABILITIES, accessSectionInfo); requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - gApi.projects().name(allProjects.get()).access(accessInput); + assertThrows( + AuthException.class, () -> gApi.projects().name(allProjects.get()).access(accessInput)); } @Test @@ -598,9 +594,13 @@ public class AccessIT extends AbstractDaemonTest { public void allUsersCanOnlyInheritFromAllProjects() throws Exception { ProjectAccessInput accessInput = newProjectAccessInput(); accessInput.parent = project.get(); - exception.expect(BadRequestException.class); - exception.expectMessage(allUsers.get() + " must inherit from " + allProjects.get()); - gApi.projects().name(allUsers.get()).access(accessInput); + BadRequestException thrown = + assertThrows( + BadRequestException.class, + () -> gApi.projects().name(allUsers.get()).access(accessInput)); + assertThat(thrown) + .hasMessageThat() + .contains(allUsers.get() + " must inherit from " + allProjects.get()); } @Test @@ -650,9 +650,9 @@ public class AccessIT extends AbstractDaemonTest { String invalidRef = Constants.R_HEADS + "stable_*"; accessInput.add.put(invalidRef, accessSectionInfo); - exception.expect(BadRequestException.class); - exception.expectMessage("Invalid Name: " + invalidRef); - pApi().access(accessInput); + BadRequestException thrown = + assertThrows(BadRequestException.class, () -> pApi().access(accessInput)); + assertThat(thrown).hasMessageThat().contains("Invalid Name: " + invalidRef); } @Test @@ -664,9 +664,9 @@ public class AccessIT extends AbstractDaemonTest { String invalidRef = Constants.R_HEADS + "stable_*"; accessInput.add.put(invalidRef, accessSectionInfo); - exception.expect(BadRequestException.class); - exception.expectMessage("Invalid Name: " + invalidRef); - pApi().accessChange(accessInput); + BadRequestException thrown = + assertThrows(BadRequestException.class, () -> pApi().accessChange(accessInput)); + assertThat(thrown).hasMessageThat().contains("Invalid Name: " + invalidRef); } private ProjectApi pApi() throws Exception { diff --git a/javatests/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java b/javatests/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java index 7ebf96b161..96ad91cb40 100644 --- a/javatests/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java @@ -19,6 +19,7 @@ import static com.google.common.truth.Truth8.assertThat; import static com.google.gerrit.reviewdb.client.RefNames.REFS_HEADS; import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS; import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.RestResponse; @@ -160,11 +161,10 @@ public class CreateBranchIT extends AbstractDaemonTest { throws Exception { BranchInput in = new BranchInput(); in.revision = revision; + RestApiException thrown = assertThrows(errType, () -> branch(branch).create(in)); if (errMsg != null) { - exception.expectMessage(errMsg); + assertThat(thrown).hasMessageThat().contains(errMsg); } - exception.expect(errType); - branch(branch).create(in); } private void assertCreateFails(BranchNameKey branch, Class errType) diff --git a/javatests/com/google/gerrit/acceptance/rest/project/CreateProjectIT.java b/javatests/com/google/gerrit/acceptance/rest/project/CreateProjectIT.java index e5bafc7267..894d79f236 100644 --- a/javatests/com/google/gerrit/acceptance/rest/project/CreateProjectIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/project/CreateProjectIT.java @@ -20,6 +20,7 @@ import static com.google.common.truth.Truth8.assertThat; import static com.google.gerrit.acceptance.rest.project.ProjectAssert.assertProjectInfo; import static com.google.gerrit.acceptance.rest.project.ProjectAssert.assertProjectOwners; import static com.google.gerrit.server.project.ProjectConfig.PROJECT_CONFIG; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static java.nio.charset.StandardCharsets.UTF_8; import com.google.common.collect.ImmutableList; @@ -469,8 +470,7 @@ public class CreateProjectIT extends AbstractDaemonTest { private void assertCreateFails(ProjectInput in, Class errType) throws Exception { - exception.expect(errType); - gApi.projects().create(in); + assertThrows(errType, () -> gApi.projects().create(in)); } private Optional readProjectConfig(String projectName) throws Exception { diff --git a/javatests/com/google/gerrit/acceptance/rest/project/DeleteBranchIT.java b/javatests/com/google/gerrit/acceptance/rest/project/DeleteBranchIT.java index 557ffe74e8..9492536648 100644 --- a/javatests/com/google/gerrit/acceptance/rest/project/DeleteBranchIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/project/DeleteBranchIT.java @@ -17,6 +17,7 @@ package com.google.gerrit.acceptance.rest.project; import static com.google.common.truth.Truth.assertThat; import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS; import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static org.eclipse.jgit.lib.Constants.R_HEADS; import com.google.gerrit.acceptance.AbstractDaemonTest; @@ -136,9 +137,11 @@ public class DeleteBranchIT extends AbstractDaemonTest { allow(allUsers, RefNames.REFS_USERS + "*", Permission.CREATE, REGISTERED_USERS); allow(allUsers, RefNames.REFS_USERS + "*", Permission.PUSH, REGISTERED_USERS); - exception.expect(ResourceConflictException.class); - exception.expectMessage("Not allowed to delete user branch."); - branch(BranchNameKey.create(allUsers, RefNames.refsUsers(admin.id()))).delete(); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, + () -> branch(BranchNameKey.create(allUsers, RefNames.refsUsers(admin.id()))).delete()); + assertThat(thrown).hasMessageThat().contains("Not allowed to delete user branch."); } @Test @@ -146,9 +149,13 @@ public class DeleteBranchIT extends AbstractDaemonTest { allow(allUsers, RefNames.REFS_GROUPS + "*", Permission.CREATE, REGISTERED_USERS); allow(allUsers, RefNames.REFS_GROUPS + "*", Permission.PUSH, REGISTERED_USERS); - exception.expect(ResourceConflictException.class); - exception.expectMessage("Not allowed to delete group branch."); - branch(BranchNameKey.create(allUsers, RefNames.refsGroups(adminGroupUuid()))).delete(); + ResourceConflictException thrown = + assertThrows( + ResourceConflictException.class, + () -> + branch(BranchNameKey.create(allUsers, RefNames.refsGroups(adminGroupUuid()))) + .delete()); + assertThat(thrown).hasMessageThat().contains("Not allowed to delete group branch."); } private void blockForcePush() throws Exception { @@ -179,8 +186,7 @@ public class DeleteBranchIT extends AbstractDaemonTest { + "/branches/" + IdString.fromDecoded(ref).encoded()); r.assertNoContent(); - exception.expect(ResourceNotFoundException.class); - branch(branch).get(); + assertThrows(ResourceNotFoundException.class, () -> branch(branch).get()); } private void assertDeleteSucceeds(BranchNameKey branch) throws Exception { @@ -189,14 +195,12 @@ public class DeleteBranchIT extends AbstractDaemonTest { branch(branch).delete(); eventRecorder.assertRefUpdatedEvents( project.get(), branch.branch(), null, branchRev, branchRev, null); - exception.expect(ResourceNotFoundException.class); - branch(branch).get(); + assertThrows(ResourceNotFoundException.class, () -> branch(branch).get()); } private void assertDeleteForbidden(BranchNameKey branch) throws Exception { assertThat(branch(branch).get().canDelete).isNull(); - exception.expect(AuthException.class); - exception.expectMessage("not permitted: delete"); - branch(branch).delete(); + AuthException thrown = assertThrows(AuthException.class, () -> branch(branch).delete()); + assertThat(thrown).hasMessageThat().contains("not permitted: delete"); } } diff --git a/javatests/com/google/gerrit/acceptance/rest/project/DeleteBranchesIT.java b/javatests/com/google/gerrit/acceptance/rest/project/DeleteBranchesIT.java index 47b6a78103..f640c7c0ba 100644 --- a/javatests/com/google/gerrit/acceptance/rest/project/DeleteBranchesIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/project/DeleteBranchesIT.java @@ -16,6 +16,7 @@ package com.google.gerrit.acceptance.rest.project; import static com.google.common.truth.Truth.assertThat; import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static java.util.stream.Collectors.toList; import static org.eclipse.jgit.lib.Constants.R_HEADS; import static org.eclipse.jgit.lib.Constants.R_REFS; @@ -139,26 +140,26 @@ public class DeleteBranchesIT extends AbstractDaemonTest { @Test public void missingInput() throws Exception { DeleteBranchesInput input = null; - exception.expect(BadRequestException.class); - exception.expectMessage("branches must be specified"); - project().deleteBranches(input); + BadRequestException thrown = + assertThrows(BadRequestException.class, () -> project().deleteBranches(input)); + assertThat(thrown).hasMessageThat().contains("branches must be specified"); } @Test public void missingBranchList() throws Exception { DeleteBranchesInput input = new DeleteBranchesInput(); - exception.expect(BadRequestException.class); - exception.expectMessage("branches must be specified"); - project().deleteBranches(input); + BadRequestException thrown = + assertThrows(BadRequestException.class, () -> project().deleteBranches(input)); + assertThat(thrown).hasMessageThat().contains("branches must be specified"); } @Test public void emptyBranchList() throws Exception { DeleteBranchesInput input = new DeleteBranchesInput(); input.branches = Lists.newArrayList(); - exception.expect(BadRequestException.class); - exception.expectMessage("branches must be specified"); - project().deleteBranches(input); + BadRequestException thrown = + assertThrows(BadRequestException.class, () -> project().deleteBranches(input)); + assertThat(thrown).hasMessageThat().contains("branches must be specified"); } private String errorMessageForBranches(List branches) { diff --git a/javatests/com/google/gerrit/acceptance/rest/project/DeleteTagIT.java b/javatests/com/google/gerrit/acceptance/rest/project/DeleteTagIT.java index 07bb2b1d30..bc0e2eae89 100644 --- a/javatests/com/google/gerrit/acceptance/rest/project/DeleteTagIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/project/DeleteTagIT.java @@ -17,6 +17,7 @@ package com.google.gerrit.acceptance.rest.project; import static com.google.common.truth.Truth.assertThat; import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS; import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static org.eclipse.jgit.lib.Constants.R_TAGS; import com.google.gerrit.acceptance.AbstractDaemonTest; @@ -122,14 +123,12 @@ public class DeleteTagIT extends AbstractDaemonTest { String tagRev = tagInfo.revision; tag().delete(); eventRecorder.assertRefUpdatedEvents(project.get(), TAG, null, tagRev, tagRev, null); - exception.expect(ResourceNotFoundException.class); - tag().get(); + assertThrows(ResourceNotFoundException.class, () -> tag().get()); } private void assertDeleteForbidden() throws Exception { assertThat(tag().get().canDelete).isNull(); - exception.expect(AuthException.class); - exception.expectMessage("not permitted: delete"); - tag().delete(); + AuthException thrown = assertThrows(AuthException.class, () -> tag().delete()); + assertThat(thrown).hasMessageThat().contains("not permitted: delete"); } } diff --git a/javatests/com/google/gerrit/acceptance/rest/project/FileBranchIT.java b/javatests/com/google/gerrit/acceptance/rest/project/FileBranchIT.java index ad8cf03250..e63b28bcba 100644 --- a/javatests/com/google/gerrit/acceptance/rest/project/FileBranchIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/project/FileBranchIT.java @@ -15,6 +15,7 @@ package com.google.gerrit.acceptance.rest.project; import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.NoHttpd; @@ -45,9 +46,9 @@ public class FileBranchIT extends AbstractDaemonTest { assertThat(content.asString()).isEqualTo(PushOneCommit.FILE_CONTENT); } - @Test(expected = ResourceNotFoundException.class) + @Test public void getNonExistingFile() throws Exception { - branch().file("does-not-exist"); + assertThrows(ResourceNotFoundException.class, () -> branch().file("does-not-exist")); } private BranchApi branch() throws Exception { diff --git a/javatests/com/google/gerrit/acceptance/rest/project/GetChildProjectIT.java b/javatests/com/google/gerrit/acceptance/rest/project/GetChildProjectIT.java index d7365781b0..7e45e0283d 100644 --- a/javatests/com/google/gerrit/acceptance/rest/project/GetChildProjectIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/project/GetChildProjectIT.java @@ -14,7 +14,9 @@ package com.google.gerrit.acceptance.rest.project; +import static com.google.common.truth.Truth.assertThat; import static com.google.gerrit.acceptance.rest.project.ProjectAssert.assertProjectInfo; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.NoHttpd; @@ -69,8 +71,10 @@ public class GetChildProjectIT extends AbstractDaemonTest { } private void assertChildNotFound(Project.NameKey parent, String child) throws Exception { - exception.expect(ResourceNotFoundException.class); - exception.expectMessage(child); - gApi.projects().name(parent.get()).child(child).get(); + ResourceNotFoundException thrown = + assertThrows( + ResourceNotFoundException.class, + () -> gApi.projects().name(parent.get()).child(child).get()); + assertThat(thrown).hasMessageThat().contains(child); } } diff --git a/javatests/com/google/gerrit/acceptance/rest/project/GetProjectIT.java b/javatests/com/google/gerrit/acceptance/rest/project/GetProjectIT.java index 989050cc30..e9aa589b38 100644 --- a/javatests/com/google/gerrit/acceptance/rest/project/GetProjectIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/project/GetProjectIT.java @@ -15,6 +15,7 @@ package com.google.gerrit.acceptance.rest.project; import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import com.google.common.collect.ImmutableMap; import com.google.gerrit.acceptance.AbstractDaemonTest; @@ -54,8 +55,9 @@ public class GetProjectIT extends AbstractDaemonTest { assertThat(p.name).isEqualTo(name); } - @Test(expected = ResourceNotFoundException.class) + @Test public void getProjectNotExisting() throws Exception { - gApi.projects().name("does-not-exist").get(); + assertThrows( + ResourceNotFoundException.class, () -> gApi.projects().name("does-not-exist").get()); } } diff --git a/javatests/com/google/gerrit/acceptance/rest/project/ListBranchesIT.java b/javatests/com/google/gerrit/acceptance/rest/project/ListBranchesIT.java index ec1c708c20..d1364f0851 100644 --- a/javatests/com/google/gerrit/acceptance/rest/project/ListBranchesIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/project/ListBranchesIT.java @@ -15,6 +15,7 @@ package com.google.gerrit.acceptance.rest.project; import static com.google.gerrit.acceptance.rest.project.RefAssert.assertRefs; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import com.google.common.collect.ImmutableList; import com.google.gerrit.acceptance.AbstractDaemonTest; @@ -35,16 +36,18 @@ public class ListBranchesIT extends AbstractDaemonTest { @Test public void listBranchesOfNonExistingProject_NotFound() throws Exception { - exception.expect(ResourceNotFoundException.class); - gApi.projects().name("non-existing").branches().get(); + assertThrows( + ResourceNotFoundException.class, + () -> gApi.projects().name("non-existing").branches().get()); } @Test public void listBranchesOfNonVisibleProject_NotFound() throws Exception { blockRead("refs/*"); requestScopeOperations.setApiUser(user.id()); - exception.expect(ResourceNotFoundException.class); - gApi.projects().name(project.get()).branches().get(); + assertThrows( + ResourceNotFoundException.class, + () -> gApi.projects().name(project.get()).branches().get()); } @Test diff --git a/javatests/com/google/gerrit/acceptance/rest/project/ListChildProjectsIT.java b/javatests/com/google/gerrit/acceptance/rest/project/ListChildProjectsIT.java index 7746820203..37b01a51d9 100644 --- a/javatests/com/google/gerrit/acceptance/rest/project/ListChildProjectsIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/project/ListChildProjectsIT.java @@ -14,7 +14,9 @@ package com.google.gerrit.acceptance.rest.project; +import static com.google.common.truth.Truth.assertThat; import static com.google.gerrit.acceptance.rest.project.ProjectAssert.assertThatNameList; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.NoHttpd; @@ -31,9 +33,11 @@ public class ListChildProjectsIT extends AbstractDaemonTest { @Test public void listChildrenOfNonExistingProject_NotFound() throws Exception { - exception.expect(ResourceNotFoundException.class); - exception.expectMessage("non-existing"); - gApi.projects().name(name("non-existing")).child("children"); + ResourceNotFoundException thrown = + assertThrows( + ResourceNotFoundException.class, + () -> gApi.projects().name(name("non-existing")).child("children")); + assertThat(thrown).hasMessageThat().contains("non-existing"); } @Test diff --git a/javatests/com/google/gerrit/acceptance/rest/project/TagsIT.java b/javatests/com/google/gerrit/acceptance/rest/project/TagsIT.java index 0165eade0f..2bd9460698 100644 --- a/javatests/com/google/gerrit/acceptance/rest/project/TagsIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/project/TagsIT.java @@ -16,6 +16,7 @@ package com.google.gerrit.acceptance.rest.project; import static com.google.common.truth.Truth.assertThat; import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static org.eclipse.jgit.lib.Constants.R_TAGS; import com.google.common.collect.FluentIterable; @@ -62,29 +63,31 @@ public class TagsIT extends AbstractDaemonTest { @Test public void listTagsOfNonExistingProject() throws Exception { - exception.expect(ResourceNotFoundException.class); - gApi.projects().name("does-not-exist").tags().get(); + assertThrows( + ResourceNotFoundException.class, () -> gApi.projects().name("does-not-exist").tags().get()); } @Test public void getTagOfNonExistingProject() throws Exception { - exception.expect(ResourceNotFoundException.class); - gApi.projects().name("does-not-exist").tag("tag").get(); + assertThrows( + ResourceNotFoundException.class, + () -> gApi.projects().name("does-not-exist").tag("tag").get()); } @Test public void listTagsOfNonVisibleProject() throws Exception { blockRead("refs/*"); requestScopeOperations.setApiUser(user.id()); - exception.expect(ResourceNotFoundException.class); - gApi.projects().name(project.get()).tags().get(); + assertThrows( + ResourceNotFoundException.class, () -> gApi.projects().name(project.get()).tags().get()); } @Test public void getTagOfNonVisibleProject() throws Exception { blockRead("refs/*"); - exception.expect(ResourceNotFoundException.class); - gApi.projects().name(project.get()).tag("tag").get(); + assertThrows( + ResourceNotFoundException.class, + () -> gApi.projects().name(project.get()).tag("tag").get()); } @Test @@ -247,9 +250,9 @@ public class TagsIT extends AbstractDaemonTest { assertThat(result.ref).isEqualTo(R_TAGS + "test"); input.ref = "refs/tags/test"; - exception.expect(ResourceConflictException.class); - exception.expectMessage("tag \"" + R_TAGS + "test\" already exists"); - tag(input.ref).create(input); + ResourceConflictException thrown = + assertThrows(ResourceConflictException.class, () -> tag(input.ref).create(input)); + assertThat(thrown).hasMessageThat().contains("tag \"" + R_TAGS + "test\" already exists"); } @Test @@ -257,9 +260,8 @@ public class TagsIT extends AbstractDaemonTest { block(R_TAGS + "*", Permission.CREATE, REGISTERED_USERS); TagInput input = new TagInput(); input.ref = "test"; - exception.expect(AuthException.class); - exception.expectMessage("not permitted: create"); - tag(input.ref).create(input); + AuthException thrown = assertThrows(AuthException.class, () -> tag(input.ref).create(input)); + assertThat(thrown).hasMessageThat().contains("not permitted: create"); } @Test @@ -268,9 +270,10 @@ public class TagsIT extends AbstractDaemonTest { TagInput input = new TagInput(); input.ref = "test"; input.message = "annotation"; - exception.expect(AuthException.class); - exception.expectMessage("Cannot create annotated tag \"" + R_TAGS + "test\""); - tag(input.ref).create(input); + AuthException thrown = assertThrows(AuthException.class, () -> tag(input.ref).create(input)); + assertThat(thrown) + .hasMessageThat() + .contains("Cannot create annotated tag \"" + R_TAGS + "test\""); } @Test @@ -278,9 +281,9 @@ public class TagsIT extends AbstractDaemonTest { TagInput input = new TagInput(); input.ref = "test"; input.message = SIGNED_ANNOTATION; - exception.expect(MethodNotAllowedException.class); - exception.expectMessage("Cannot create signed tag \"" + R_TAGS + "test\""); - tag(input.ref).create(input); + MethodNotAllowedException thrown = + assertThrows(MethodNotAllowedException.class, () -> tag(input.ref).create(input)); + assertThat(thrown).hasMessageThat().contains("Cannot create signed tag \"" + R_TAGS + "test\""); } @Test @@ -288,9 +291,9 @@ public class TagsIT extends AbstractDaemonTest { TagInput input = new TagInput(); input.ref = "test"; - exception.expect(BadRequestException.class); - exception.expectMessage("ref must match URL"); - tag("TEST").create(input); + BadRequestException thrown = + assertThrows(BadRequestException.class, () -> tag("TEST").create(input)); + assertThat(thrown).hasMessageThat().contains("ref must match URL"); } @Test @@ -300,9 +303,9 @@ public class TagsIT extends AbstractDaemonTest { TagInput input = new TagInput(); input.ref = "refs/heads/test"; - exception.expect(BadRequestException.class); - exception.expectMessage("invalid tag name \"" + input.ref + "\""); - tag(input.ref).create(input); + BadRequestException thrown = + assertThrows(BadRequestException.class, () -> tag(input.ref).create(input)); + assertThat(thrown).hasMessageThat().contains("invalid tag name \"" + input.ref + "\""); } @Test @@ -312,9 +315,9 @@ public class TagsIT extends AbstractDaemonTest { TagInput input = new TagInput(); input.ref = "//"; - exception.expect(BadRequestException.class); - exception.expectMessage("invalid tag name \"refs/tags/\""); - tag(input.ref).create(input); + BadRequestException thrown = + assertThrows(BadRequestException.class, () -> tag(input.ref).create(input)); + assertThat(thrown).hasMessageThat().contains("invalid tag name \"refs/tags/\""); } @Test @@ -325,9 +328,9 @@ public class TagsIT extends AbstractDaemonTest { input.ref = "test"; input.revision = "abcdefg"; - exception.expect(BadRequestException.class); - exception.expectMessage("Invalid base revision"); - tag(input.ref).create(input); + BadRequestException thrown = + assertThrows(BadRequestException.class, () -> tag(input.ref).create(input)); + assertThat(thrown).hasMessageThat().contains("Invalid base revision"); } private void assertTagList(FluentIterable expected, List actual) diff --git a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java index 15dd3fb087..ebe46bbf2b 100644 --- a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java +++ b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java @@ -18,6 +18,7 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth8.assertThat; import static com.google.gerrit.acceptance.PushOneCommit.FILE_NAME; import static com.google.gerrit.acceptance.PushOneCommit.SUBJECT; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static java.util.stream.Collectors.groupingBy; import static java.util.stream.Collectors.toList; @@ -94,8 +95,9 @@ public class CommentsIT extends AbstractDaemonTest { PushOneCommit.Result r = createChange(); String changeId = r.getChangeId(); String revId = r.getCommit().getName(); - exception.expect(ResourceNotFoundException.class); - getPublishedComment(changeId, revId, "non-existing"); + assertThrows( + ResourceNotFoundException.class, + () -> getPublishedComment(changeId, revId, "non-existing")); } @Test @@ -263,9 +265,11 @@ public class CommentsIT extends AbstractDaemonTest { CommentInput c = newComment(Patch.COMMIT_MSG, Side.PARENT, 0, "comment on auto-merge", false); input.comments = new HashMap<>(); input.comments.put(Patch.COMMIT_MSG, ImmutableList.of(c)); - exception.expect(BadRequestException.class); - exception.expectMessage("cannot comment on " + Patch.COMMIT_MSG + " on auto-merge"); - revision(r).review(input); + BadRequestException thrown = + assertThrows(BadRequestException.class, () -> revision(r).review(input)); + assertThat(thrown) + .hasMessageThat() + .contains("cannot comment on " + Patch.COMMIT_MSG + " on auto-merge"); } @Test @@ -769,8 +773,9 @@ public class CommentsIT extends AbstractDaemonTest { DeleteCommentInput input = new DeleteCommentInput("contains confidential information"); requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - gApi.changes().id(result.getChangeId()).current().comment(uuid).delete(input); + assertThrows( + AuthException.class, + () -> gApi.changes().id(result.getChangeId()).current().comment(uuid).delete(input)); } @Test diff --git a/javatests/com/google/gerrit/acceptance/server/project/CustomLabelIT.java b/javatests/com/google/gerrit/acceptance/server/project/CustomLabelIT.java index 6cbe40e56b..21a7d95186 100644 --- a/javatests/com/google/gerrit/acceptance/server/project/CustomLabelIT.java +++ b/javatests/com/google/gerrit/acceptance/server/project/CustomLabelIT.java @@ -26,6 +26,7 @@ import static com.google.gerrit.extensions.client.ListChangesOption.SUBMITTABLE; import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS; import static com.google.gerrit.server.project.testing.Util.category; import static com.google.gerrit.server.project.testing.Util.value; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.NoHttpd; @@ -265,15 +266,17 @@ public class CustomLabelIT extends AbstractDaemonTest { assertPermitted(info, P.getName(), 0, 1); assertPermitted(info, label.getName()); - ReviewInput in = new ReviewInput(); - in.label(P.getName(), P.getMax().getValue()); - revision(r).review(in); + ReviewInput postSubmitReview1 = new ReviewInput(); + postSubmitReview1.label(P.getName(), P.getMax().getValue()); + revision(r).review(postSubmitReview1); - in = new ReviewInput(); - in.label(label.getName(), label.getMax().getValue()); - exception.expect(ResourceConflictException.class); - exception.expectMessage("Voting on labels disallowed after submit: " + label.getName()); - revision(r).review(in); + ReviewInput postSubmitReview2 = new ReviewInput(); + postSubmitReview2.label(label.getName(), label.getMax().getValue()); + ResourceConflictException thrown = + assertThrows(ResourceConflictException.class, () -> revision(r).review(postSubmitReview2)); + assertThat(thrown) + .hasMessageThat() + .contains("Voting on labels disallowed after submit: " + label.getName()); } @Test diff --git a/javatests/com/google/gerrit/acceptance/server/project/ReflogIT.java b/javatests/com/google/gerrit/acceptance/server/project/ReflogIT.java index de29fa01ac..060a9c3bf0 100644 --- a/javatests/com/google/gerrit/acceptance/server/project/ReflogIT.java +++ b/javatests/com/google/gerrit/acceptance/server/project/ReflogIT.java @@ -17,6 +17,7 @@ package com.google.gerrit.acceptance.server.project; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; import static com.google.gerrit.reviewdb.client.RefNames.changeMetaRef; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.PushOneCommit; @@ -86,8 +87,8 @@ public class ReflogIT extends AbstractDaemonTest { @Test public void regularUserIsNotAllowedToGetReflog() throws Exception { requestScopeOperations.setApiUser(user.id()); - exception.expect(AuthException.class); - gApi.projects().name(project.get()).branch("master").reflog(); + assertThrows( + AuthException.class, () -> gApi.projects().name(project.get()).branch("master").reflog()); } @Test diff --git a/javatests/com/google/gerrit/acceptance/server/quota/DefaultQuotaBackendIT.java b/javatests/com/google/gerrit/acceptance/server/quota/DefaultQuotaBackendIT.java index 286e5ae06a..47d23a4fba 100644 --- a/javatests/com/google/gerrit/acceptance/server/quota/DefaultQuotaBackendIT.java +++ b/javatests/com/google/gerrit/acceptance/server/quota/DefaultQuotaBackendIT.java @@ -15,6 +15,7 @@ package com.google.gerrit.acceptance.server.quota; import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static org.easymock.EasyMock.expect; import static org.easymock.EasyMock.replay; import static org.easymock.EasyMock.resetToStrict; @@ -133,9 +134,8 @@ public class DefaultQuotaBackendIT extends AbstractDaemonTest { QuotaResponse.Aggregated result = quotaBackend.user(identifiedAdmin).requestToken("testGroup"); assertThat(result).isEqualTo(singletonAggregation(QuotaResponse.error("failed"))); - exception.expect(QuotaException.class); - exception.expectMessage("failed"); - result.throwOnError(); + QuotaException thrown = assertThrows(QuotaException.class, () -> result.throwOnError()); + assertThat(thrown).hasMessageThat().contains("failed"); } @Test @@ -143,9 +143,9 @@ public class DefaultQuotaBackendIT extends AbstractDaemonTest { QuotaRequestContext ctx = QuotaRequestContext.builder().user(identifiedAdmin).build(); expect(quotaEnforcer.requestTokens("testGroup", ctx, 1)).andThrow(new NullPointerException()); replay(quotaEnforcer); - - exception.expect(NullPointerException.class); - quotaBackend.user(identifiedAdmin).requestToken("testGroup"); + assertThrows( + NullPointerException.class, + () -> quotaBackend.user(identifiedAdmin).requestToken("testGroup")); } private static QuotaResponse.Aggregated singletonAggregation(QuotaResponse response) { diff --git a/javatests/com/google/gerrit/acceptance/testsuite/group/GroupOperationsImplTest.java b/javatests/com/google/gerrit/acceptance/testsuite/group/GroupOperationsImplTest.java index 1961294ba5..bb84689821 100644 --- a/javatests/com/google/gerrit/acceptance/testsuite/group/GroupOperationsImplTest.java +++ b/javatests/com/google/gerrit/acceptance/testsuite/group/GroupOperationsImplTest.java @@ -17,6 +17,7 @@ package com.google.gerrit.acceptance.testsuite.group; 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.testing.GerritJUnit.assertThrows; import com.google.common.collect.ImmutableSet; import com.google.common.truth.Correspondence; @@ -227,9 +228,8 @@ public class GroupOperationsImplTest extends AbstractDaemonTest { @Test public void retrievingNotExistingGroupFails() throws Exception { AccountGroup.UUID notExistingGroupUuid = AccountGroup.uuid("not-existing-group"); - - exception.expect(IllegalStateException.class); - groupOperations.group(notExistingGroupUuid).get(); + assertThrows( + IllegalStateException.class, () -> groupOperations.group(notExistingGroupUuid).get()); } @Test diff --git a/javatests/com/google/gerrit/common/data/AccessSectionTest.java b/javatests/com/google/gerrit/common/data/AccessSectionTest.java index faf9d6c752..eeebe7036a 100644 --- a/javatests/com/google/gerrit/common/data/AccessSectionTest.java +++ b/javatests/com/google/gerrit/common/data/AccessSectionTest.java @@ -15,6 +15,7 @@ package com.google.gerrit.common.data; import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import com.google.common.collect.ImmutableList; import com.google.gerrit.testing.GerritBaseTests; @@ -57,16 +58,17 @@ public class AccessSectionTest extends GerritBaseTests { Permission submitPermission = new Permission(Permission.SUBMIT); accessSection.setPermissions(ImmutableList.of(submitPermission)); assertThat(accessSection.getPermissions()).containsExactly(submitPermission); - - exception.expect(NullPointerException.class); - accessSection.setPermissions(null); + assertThrows(NullPointerException.class, () -> accessSection.setPermissions(null)); } @Test public void cannotSetDuplicatePermissions() { - exception.expect(IllegalArgumentException.class); - accessSection.setPermissions( - ImmutableList.of(new Permission(Permission.ABANDON), new Permission(Permission.ABANDON))); + assertThrows( + IllegalArgumentException.class, + () -> + accessSection.setPermissions( + ImmutableList.of( + new Permission(Permission.ABANDON), new Permission(Permission.ABANDON)))); } @Test @@ -76,9 +78,11 @@ public class AccessSectionTest extends GerritBaseTests { Permission abandonPermissionUpperCase = new Permission(Permission.ABANDON.toUpperCase(Locale.US)); - exception.expect(IllegalArgumentException.class); - accessSection.setPermissions( - ImmutableList.of(abandonPermissionLowerCase, abandonPermissionUpperCase)); + assertThrows( + IllegalArgumentException.class, + () -> + accessSection.setPermissions( + ImmutableList.of(abandonPermissionLowerCase, abandonPermissionUpperCase))); } @Test @@ -92,9 +96,7 @@ public class AccessSectionTest extends GerritBaseTests { Permission submitPermission = new Permission(Permission.SUBMIT); accessSection.setPermissions(ImmutableList.of(submitPermission)); assertThat(accessSection.getPermission(Permission.SUBMIT)).isEqualTo(submitPermission); - - exception.expect(NullPointerException.class); - accessSection.getPermission(null); + assertThrows(NullPointerException.class, () -> accessSection.getPermission(null)); } @Test @@ -112,8 +114,7 @@ public class AccessSectionTest extends GerritBaseTests { assertThat(accessSection.getPermission(Permission.SUBMIT, true)) .isEqualTo(new Permission(Permission.SUBMIT)); - exception.expect(NullPointerException.class); - accessSection.getPermission(null, true); + assertThrows(NullPointerException.class, () -> accessSection.getPermission(null, true)); } @Test @@ -130,9 +131,7 @@ public class AccessSectionTest extends GerritBaseTests { assertThat(accessSection.getPermissions()) .containsExactly(abandonPermission, rebasePermission, submitPermission) .inOrder(); - - exception.expect(NullPointerException.class); - accessSection.addPermission(null); + assertThrows(NullPointerException.class, () -> accessSection.addPermission(null)); } @Test @@ -166,9 +165,7 @@ public class AccessSectionTest extends GerritBaseTests { assertThat(accessSection.getPermissions()) .containsExactly(abandonPermission, rebasePermission) .inOrder(); - - exception.expect(NullPointerException.class); - accessSection.remove(null); + assertThrows(NullPointerException.class, () -> accessSection.remove(null)); } @Test @@ -187,8 +184,7 @@ public class AccessSectionTest extends GerritBaseTests { .containsExactly(abandonPermission, rebasePermission) .inOrder(); - exception.expect(NullPointerException.class); - accessSection.removePermission(null); + assertThrows(NullPointerException.class, () -> accessSection.removePermission(null)); } @Test @@ -229,9 +225,7 @@ public class AccessSectionTest extends GerritBaseTests { assertThat(accessSection1.getPermissions()) .containsExactly(abandonPermission, rebasePermission, submitPermission) .inOrder(); - - exception.expect(NullPointerException.class); - accessSection.mergeFrom(null); + assertThrows(NullPointerException.class, () -> accessSection.mergeFrom(null)); } @Test diff --git a/javatests/com/google/gerrit/common/data/GroupReferenceTest.java b/javatests/com/google/gerrit/common/data/GroupReferenceTest.java index 9b4f617431..8671dc971f 100644 --- a/javatests/com/google/gerrit/common/data/GroupReferenceTest.java +++ b/javatests/com/google/gerrit/common/data/GroupReferenceTest.java @@ -15,6 +15,7 @@ package com.google.gerrit.common.data; import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.AccountGroup.UUID; @@ -75,8 +76,8 @@ public class GroupReferenceTest extends GerritBaseTests { @Test public void cannotCreateWithoutName() { - exception.expect(NullPointerException.class); - new GroupReference(AccountGroup.uuid("uuid"), null); + assertThrows( + NullPointerException.class, () -> new GroupReference(AccountGroup.uuid("uuid"), null)); } @Test @@ -125,8 +126,7 @@ public class GroupReferenceTest extends GerritBaseTests { groupReference.setName(name2); assertThat(groupReference.getName()).isEqualTo(name2); - exception.expect(NullPointerException.class); - groupReference.setName(null); + assertThrows(NullPointerException.class, () -> groupReference.setName(null)); } @Test diff --git a/javatests/com/google/gerrit/common/data/PermissionRuleTest.java b/javatests/com/google/gerrit/common/data/PermissionRuleTest.java index 77b3d819cc..79e436f2a5 100644 --- a/javatests/com/google/gerrit/common/data/PermissionRuleTest.java +++ b/javatests/com/google/gerrit/common/data/PermissionRuleTest.java @@ -15,6 +15,7 @@ package com.google.gerrit.common.data; import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import com.google.gerrit.common.data.PermissionRule.Action; import com.google.gerrit.reviewdb.client.AccountGroup; @@ -42,8 +43,7 @@ public class PermissionRuleTest extends GerritBaseTests { @Test public void cannotSetActionToNull() { - exception.expect(NullPointerException.class); - permissionRule.setAction(null); + assertThrows(NullPointerException.class, () -> permissionRule.setAction(null)); } @Test diff --git a/javatests/com/google/gerrit/httpd/restapi/ParameterParserTest.java b/javatests/com/google/gerrit/httpd/restapi/ParameterParserTest.java index 30d318b654..8900154dc0 100644 --- a/javatests/com/google/gerrit/httpd/restapi/ParameterParserTest.java +++ b/javatests/com/google/gerrit/httpd/restapi/ParameterParserTest.java @@ -15,8 +15,8 @@ package com.google.gerrit.httpd.restapi; import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.fail; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -110,35 +110,26 @@ public class ParameterParserTest extends GerritBaseTests { public void rejectDuplicateMethod() { FakeHttpServletRequest req = new FakeHttpServletRequest(); req.setQueryString("$m=PUT&$m=DELETE"); - try { - ParameterParser.getQueryParams(req); - fail("expected BadRequestException"); - } catch (BadRequestException bad) { - assertThat(bad).hasMessageThat().isEqualTo("duplicate $m"); - } + BadRequestException bad = + assertThrows(BadRequestException.class, () -> ParameterParser.getQueryParams(req)); + assertThat(bad).hasMessageThat().isEqualTo("duplicate $m"); } @Test public void rejectDuplicateContentType() { FakeHttpServletRequest req = new FakeHttpServletRequest(); req.setQueryString("$ct=json&$ct=string"); - try { - ParameterParser.getQueryParams(req); - fail("expected BadRequestException"); - } catch (BadRequestException bad) { - assertThat(bad).hasMessageThat().isEqualTo("duplicate $ct"); - } + BadRequestException bad = + assertThrows(BadRequestException.class, () -> ParameterParser.getQueryParams(req)); + assertThat(bad).hasMessageThat().isEqualTo("duplicate $ct"); } @Test public void rejectInvalidMethod() { FakeHttpServletRequest req = new FakeHttpServletRequest(); req.setQueryString("$m=CONNECT"); - try { - ParameterParser.getQueryParams(req); - fail("expected BadRequestException"); - } catch (BadRequestException bad) { - assertThat(bad).hasMessageThat().isEqualTo("invalid $m"); - } + BadRequestException bad = + assertThrows(BadRequestException.class, () -> ParameterParser.getQueryParams(req)); + assertThat(bad).hasMessageThat().isEqualTo("invalid $m"); } } diff --git a/javatests/com/google/gerrit/index/SchemaUtilTest.java b/javatests/com/google/gerrit/index/SchemaUtilTest.java index d6b8421c3a..52c596d425 100644 --- a/javatests/com/google/gerrit/index/SchemaUtilTest.java +++ b/javatests/com/google/gerrit/index/SchemaUtilTest.java @@ -18,6 +18,7 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.gerrit.index.SchemaUtil.getNameParts; import static com.google.gerrit.index.SchemaUtil.getPersonParts; import static com.google.gerrit.index.SchemaUtil.schema; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import com.google.gerrit.testing.GerritBaseTests; import java.util.Map; @@ -43,9 +44,9 @@ public class SchemaUtilTest extends GerritBaseTests { assertThat(all.get(1)).isEqualTo(TestSchemas.V1); assertThat(all.get(2)).isEqualTo(TestSchemas.V2); assertThat(all.get(4)).isEqualTo(TestSchemas.V4); - - exception.expect(IllegalArgumentException.class); - SchemaUtil.schemasFromClass(TestSchemas.class, Object.class); + assertThrows( + IllegalArgumentException.class, + () -> SchemaUtil.schemasFromClass(TestSchemas.class, Object.class)); } @Test diff --git a/javatests/com/google/gerrit/index/query/AndPredicateTest.java b/javatests/com/google/gerrit/index/query/AndPredicateTest.java index 21098b315e..16828dd242 100644 --- a/javatests/com/google/gerrit/index/query/AndPredicateTest.java +++ b/javatests/com/google/gerrit/index/query/AndPredicateTest.java @@ -16,12 +16,12 @@ package com.google.gerrit.index.query; import static com.google.common.collect.ImmutableList.of; import static com.google.gerrit.index.query.Predicate.and; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotSame; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; import java.util.List; import org.junit.Test; @@ -43,28 +43,13 @@ public class AndPredicateTest extends PredicateTest { final TestPredicate b = f("author", "bob"); final Predicate n = and(a, b); - try { - n.getChildren().clear(); - fail("Expected UnsupportedOperationException"); - } catch (UnsupportedOperationException e) { - // Expected - } + assertThrows(UnsupportedOperationException.class, () -> n.getChildren().clear()); assertChildren("clear", n, of(a, b)); - try { - n.getChildren().remove(0); - fail("Expected UnsupportedOperationException"); - } catch (UnsupportedOperationException e) { - // Expected - } + assertThrows(UnsupportedOperationException.class, () -> n.getChildren().remove(0)); assertChildren("remove(0)", n, of(a, b)); - try { - n.getChildren().iterator().remove(); - fail("Expected UnsupportedOperationException"); - } catch (UnsupportedOperationException e) { - // Expected - } + assertThrows(UnsupportedOperationException.class, () -> n.getChildren().iterator().remove()); assertChildren("iterator().remove()", n, of(a, b)); } diff --git a/javatests/com/google/gerrit/index/query/FieldPredicateTest.java b/javatests/com/google/gerrit/index/query/FieldPredicateTest.java index 805f31c194..2d2c99e5ee 100644 --- a/javatests/com/google/gerrit/index/query/FieldPredicateTest.java +++ b/javatests/com/google/gerrit/index/query/FieldPredicateTest.java @@ -14,6 +14,8 @@ package com.google.gerrit.index.query; +import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertSame; @@ -61,8 +63,9 @@ public class FieldPredicateTest extends PredicateTest { assertSame(f, f.copy(Collections.emptyList())); assertSame(f, f.copy(f.getChildren())); - exception.expect(IllegalArgumentException.class); - exception.expectMessage("Expected 0 children"); - f.copy(Collections.singleton(f("owner", "bob"))); + IllegalArgumentException thrown = + assertThrows( + IllegalArgumentException.class, () -> f.copy(Collections.singleton(f("owner", "bob")))); + assertThat(thrown).hasMessageThat().contains("Expected 0 children"); } } diff --git a/javatests/com/google/gerrit/index/query/NotPredicateTest.java b/javatests/com/google/gerrit/index/query/NotPredicateTest.java index d10d2df498..27ef6619f6 100644 --- a/javatests/com/google/gerrit/index/query/NotPredicateTest.java +++ b/javatests/com/google/gerrit/index/query/NotPredicateTest.java @@ -16,12 +16,12 @@ package com.google.gerrit.index.query; import static com.google.gerrit.index.query.Predicate.and; import static com.google.gerrit.index.query.Predicate.not; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotSame; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; import java.util.Collections; import java.util.List; @@ -50,26 +50,17 @@ public class NotPredicateTest extends PredicateTest { final TestPredicate p = f("author", "bob"); final Predicate n = not(p); - try { - n.getChildren().clear(); - fail("Expected UnsupportedOperationException"); - } catch (UnsupportedOperationException e) { - assertOnlyChild("clear", p, n); - } + UnsupportedOperationException e = + assertThrows(UnsupportedOperationException.class, () -> n.getChildren().clear()); + assertOnlyChild("clear", p, n); - try { - n.getChildren().remove(0); - fail("Expected UnsupportedOperationException"); - } catch (UnsupportedOperationException e) { - assertOnlyChild("remove(0)", p, n); - } + e = assertThrows(UnsupportedOperationException.class, () -> n.getChildren().remove(0)); + assertOnlyChild("remove(0)", p, n); - try { - n.getChildren().iterator().remove(); - fail("Expected UnsupportedOperationException"); - } catch (UnsupportedOperationException e) { - assertOnlyChild("remove()", p, n); - } + e = + assertThrows( + UnsupportedOperationException.class, () -> n.getChildren().iterator().remove()); + assertOnlyChild("remove()", p, n); } private static void assertOnlyChild(String o, Predicate c, Predicate p) { @@ -112,18 +103,11 @@ public class NotPredicateTest extends PredicateTest { assertNotSame(n, n.copy(sb)); assertEquals(sb, n.copy(sb).getChildren()); - try { - n.copy(Collections.emptyList()); - fail("Expected IllegalArgumentException"); - } catch (IllegalArgumentException e) { - assertEquals("Expected exactly one child", e.getMessage()); - } + IllegalArgumentException e = + assertThrows(IllegalArgumentException.class, () -> n.copy(Collections.emptyList())); + assertEquals("Expected exactly one child", e.getMessage()); - try { - n.copy(and(a, b).getChildren()); - fail("Expected IllegalArgumentException"); - } catch (IllegalArgumentException e) { - assertEquals("Expected exactly one child", e.getMessage()); - } + e = assertThrows(IllegalArgumentException.class, () -> n.copy(and(a, b).getChildren())); + assertEquals("Expected exactly one child", e.getMessage()); } } diff --git a/javatests/com/google/gerrit/index/query/OrPredicateTest.java b/javatests/com/google/gerrit/index/query/OrPredicateTest.java index 255a3f8cce..1cbcb7567a 100644 --- a/javatests/com/google/gerrit/index/query/OrPredicateTest.java +++ b/javatests/com/google/gerrit/index/query/OrPredicateTest.java @@ -16,12 +16,12 @@ package com.google.gerrit.index.query; import static com.google.common.collect.ImmutableList.of; import static com.google.gerrit.index.query.Predicate.or; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotSame; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; import java.util.List; import org.junit.Test; @@ -43,28 +43,13 @@ public class OrPredicateTest extends PredicateTest { final TestPredicate b = f("author", "bob"); final Predicate n = or(a, b); - try { - n.getChildren().clear(); - fail("Expected UnsupportedOperationException"); - } catch (UnsupportedOperationException e) { - // Expected - } + assertThrows(UnsupportedOperationException.class, () -> n.getChildren().clear()); assertChildren("clear", n, of(a, b)); - try { - n.getChildren().remove(0); - fail("Expected UnsupportedOperationException"); - } catch (UnsupportedOperationException e) { - // Expected - } + assertThrows(UnsupportedOperationException.class, () -> n.getChildren().remove(0)); assertChildren("remove(0)", n, of(a, b)); - try { - n.getChildren().iterator().remove(); - fail("Expected UnsupportedOperationException"); - } catch (UnsupportedOperationException e) { - // Expected - } + assertThrows(UnsupportedOperationException.class, () -> n.getChildren().iterator().remove()); assertChildren("iterator().remove()", n, of(a, b)); } diff --git a/javatests/com/google/gerrit/reviewdb/client/RefNamesTest.java b/javatests/com/google/gerrit/reviewdb/client/RefNamesTest.java index b4010947ed..7f2227510e 100644 --- a/javatests/com/google/gerrit/reviewdb/client/RefNamesTest.java +++ b/javatests/com/google/gerrit/reviewdb/client/RefNamesTest.java @@ -20,18 +20,14 @@ import static com.google.gerrit.reviewdb.client.RefNames.parseRefSuffix; import static com.google.gerrit.reviewdb.client.RefNames.parseShardedRefPart; import static com.google.gerrit.reviewdb.client.RefNames.parseShardedUuidFromRefPart; import static com.google.gerrit.reviewdb.client.RefNames.skipShardedRefPart; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; -import org.junit.Rule; import org.junit.Test; -import org.junit.rules.ExpectedException; public class RefNamesTest { private static final String TEST_GROUP_UUID = "ccab3195282a8ce4f5014efa391e82d10f884c64"; private static final String TEST_SHARDED_GROUP_UUID = TEST_GROUP_UUID.substring(0, 2) + "/" + TEST_GROUP_UUID; - - @Rule public ExpectedException expectedException = ExpectedException.none(); - private final Account.Id accountId = Account.id(1011123); private final Change.Id changeId = Change.id(67473); private final PatchSet.Id psId = PatchSet.id(changeId, 42); @@ -66,8 +62,7 @@ public class RefNamesTest { @Test public void refForGroupWithUuidLessThanTwoCharsIsRejected() throws Exception { AccountGroup.UUID groupUuid = AccountGroup.uuid("A"); - expectedException.expect(IllegalArgumentException.class); - RefNames.refsGroups(groupUuid); + assertThrows(IllegalArgumentException.class, () -> RefNames.refsGroups(groupUuid)); } @Test @@ -80,8 +75,7 @@ public class RefNamesTest { @Test public void refForDeletedGroupWithUuidLessThanTwoCharsIsRejected() throws Exception { AccountGroup.UUID groupUuid = AccountGroup.uuid("A"); - expectedException.expect(IllegalArgumentException.class); - RefNames.refsDeletedGroups(groupUuid); + assertThrows(IllegalArgumentException.class, () -> RefNames.refsDeletedGroups(groupUuid)); } @Test diff --git a/javatests/com/google/gerrit/server/account/HashedPasswordTest.java b/javatests/com/google/gerrit/server/account/HashedPasswordTest.java index 9a0c9cb972..7ef8d99709 100644 --- a/javatests/com/google/gerrit/server/account/HashedPasswordTest.java +++ b/javatests/com/google/gerrit/server/account/HashedPasswordTest.java @@ -15,6 +15,7 @@ package com.google.gerrit.server.account; import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import com.google.common.base.Strings; import com.google.gerrit.testing.GerritBaseTests; @@ -41,9 +42,9 @@ public class HashedPasswordTest extends GerritBaseTests { assertThat(roundtrip.checkPassword("not the password")).isFalse(); } - @Test(expected = DecoderException.class) + @Test public void invalidDecode() throws Exception { - HashedPassword.decode("invalid"); + assertThrows(DecoderException.class, () -> HashedPassword.decode("invalid")); } @Test diff --git a/javatests/com/google/gerrit/server/cache/PerThreadCacheTest.java b/javatests/com/google/gerrit/server/cache/PerThreadCacheTest.java index 6a42577d1e..335335662f 100644 --- a/javatests/com/google/gerrit/server/cache/PerThreadCacheTest.java +++ b/javatests/com/google/gerrit/server/cache/PerThreadCacheTest.java @@ -15,6 +15,7 @@ package com.google.gerrit.server.cache; import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import com.google.gerrit.testing.GerritBaseTests; import java.util.function.Supplier; @@ -75,9 +76,9 @@ public class PerThreadCacheTest extends GerritBaseTests { @Test public void doubleInstantiationFails() { try (PerThreadCache ignored = PerThreadCache.create()) { - exception.expect(IllegalStateException.class); - exception.expectMessage("called create() twice on the same request"); - PerThreadCache.create(); + IllegalStateException thrown = + assertThrows(IllegalStateException.class, () -> PerThreadCache.create()); + assertThat(thrown).hasMessageThat().contains("called create() twice on the same request"); } } diff --git a/javatests/com/google/gerrit/server/config/SitePathsTest.java b/javatests/com/google/gerrit/server/config/SitePathsTest.java index b4cde14e7e..1106fca985 100644 --- a/javatests/com/google/gerrit/server/config/SitePathsTest.java +++ b/javatests/com/google/gerrit/server/config/SitePathsTest.java @@ -16,6 +16,7 @@ package com.google.gerrit.server.config; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth8.assertThat; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import com.google.gerrit.server.ioutil.HostPlatform; import com.google.gerrit.testing.GerritBaseTests; @@ -72,8 +73,8 @@ public class SitePathsTest extends GerritBaseTests { final Path root = random(); try { Files.createFile(root); - exception.expect(NotDirectoryException.class); - new SitePaths(root); + assertThrows(NotDirectoryException.class, () -> new SitePaths(root)); + } finally { Files.delete(root); } diff --git a/javatests/com/google/gerrit/server/fixes/FixReplacementInterpreterTest.java b/javatests/com/google/gerrit/server/fixes/FixReplacementInterpreterTest.java index cc648bfa7a..142be4aa68 100644 --- a/javatests/com/google/gerrit/server/fixes/FixReplacementInterpreterTest.java +++ b/javatests/com/google/gerrit/server/fixes/FixReplacementInterpreterTest.java @@ -15,6 +15,7 @@ package com.google.gerrit.server.fixes; import static com.google.gerrit.server.edit.tree.TreeModificationSubject.assertThatList; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static org.easymock.EasyMock.createMock; import static org.easymock.EasyMock.replay; @@ -256,9 +257,7 @@ public class FixReplacementInterpreterTest extends GerritBaseTests { mockFileContent(filePath1, "First line\nSecond line\nThird line\n"); replay(fileContentUtil); - - exception.expect(ResourceConflictException.class); - toTreeModifications(fixReplacement); + assertThrows(ResourceConflictException.class, () -> toTreeModifications(fixReplacement)); } @Test @@ -269,8 +268,7 @@ public class FixReplacementInterpreterTest extends GerritBaseTests { replay(fileContentUtil); - exception.expect(ResourceConflictException.class); - toTreeModifications(fixReplacement); + assertThrows(ResourceConflictException.class, () -> toTreeModifications(fixReplacement)); } @Test @@ -280,9 +278,7 @@ public class FixReplacementInterpreterTest extends GerritBaseTests { mockFileContent(filePath1, "First line\nSecond line\nThird line\n"); replay(fileContentUtil); - - exception.expect(ResourceConflictException.class); - toTreeModifications(fixReplacement); + assertThrows(ResourceConflictException.class, () -> toTreeModifications(fixReplacement)); } @Test @@ -293,8 +289,7 @@ public class FixReplacementInterpreterTest extends GerritBaseTests { replay(fileContentUtil); - exception.expect(ResourceConflictException.class); - toTreeModifications(fixReplacement); + assertThrows(ResourceConflictException.class, () -> toTreeModifications(fixReplacement)); } @Test @@ -304,9 +299,7 @@ public class FixReplacementInterpreterTest extends GerritBaseTests { mockFileContent(filePath1, "First line\nSecond line\nThird line\n"); replay(fileContentUtil); - - exception.expect(ResourceConflictException.class); - toTreeModifications(fixReplacement); + assertThrows(ResourceConflictException.class, () -> toTreeModifications(fixReplacement)); } private void mockFileContent(String filePath, String fileContent) throws Exception { diff --git a/javatests/com/google/gerrit/server/fixes/LineIdentifierTest.java b/javatests/com/google/gerrit/server/fixes/LineIdentifierTest.java index 309f726ed2..110edff65e 100644 --- a/javatests/com/google/gerrit/server/fixes/LineIdentifierTest.java +++ b/javatests/com/google/gerrit/server/fixes/LineIdentifierTest.java @@ -15,6 +15,7 @@ package com.google.gerrit.server.fixes; import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import com.google.gerrit.testing.GerritBaseTests; import org.junit.Test; @@ -23,17 +24,19 @@ public class LineIdentifierTest extends GerritBaseTests { @Test public void lineNumberMustBePositive() { LineIdentifier lineIdentifier = new LineIdentifier("First line\nSecond line"); - exception.expect(StringIndexOutOfBoundsException.class); - exception.expectMessage("positive"); - lineIdentifier.getStartIndexOfLine(0); + StringIndexOutOfBoundsException thrown = + assertThrows( + StringIndexOutOfBoundsException.class, () -> lineIdentifier.getStartIndexOfLine(0)); + assertThat(thrown).hasMessageThat().contains("positive"); } @Test public void lineNumberMustIndicateAnAvailableLine() { LineIdentifier lineIdentifier = new LineIdentifier("First line\nSecond line"); - exception.expect(StringIndexOutOfBoundsException.class); - exception.expectMessage("Line 3 isn't available"); - lineIdentifier.getStartIndexOfLine(3); + StringIndexOutOfBoundsException thrown = + assertThrows( + StringIndexOutOfBoundsException.class, () -> lineIdentifier.getStartIndexOfLine(3)); + assertThat(thrown).hasMessageThat().contains("Line 3 isn't available"); } @Test diff --git a/javatests/com/google/gerrit/server/fixes/StringModifierTest.java b/javatests/com/google/gerrit/server/fixes/StringModifierTest.java index 185b58c21c..0f1ecf472d 100644 --- a/javatests/com/google/gerrit/server/fixes/StringModifierTest.java +++ b/javatests/com/google/gerrit/server/fixes/StringModifierTest.java @@ -15,6 +15,7 @@ package com.google.gerrit.server.fixes; import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import com.google.gerrit.testing.GerritBaseTests; import org.junit.Before; @@ -63,20 +64,20 @@ public class StringModifierTest extends GerritBaseTests { @Test public void replacedPartsMustNotOverlap() { stringModifier.replace(0, 9, ""); - exception.expect(StringIndexOutOfBoundsException.class); - stringModifier.replace(8, 32, "The modified"); + assertThrows( + StringIndexOutOfBoundsException.class, () -> stringModifier.replace(8, 32, "The modified")); } @Test public void startIndexMustNotBeGreaterThanEndIndex() { - exception.expect(StringIndexOutOfBoundsException.class); - stringModifier.replace(10, 9, "something"); + assertThrows( + StringIndexOutOfBoundsException.class, () -> stringModifier.replace(10, 9, "something")); } @Test public void startIndexMustNotBeNegative() { - exception.expect(StringIndexOutOfBoundsException.class); - stringModifier.replace(-1, 9, "something"); + assertThrows( + StringIndexOutOfBoundsException.class, () -> stringModifier.replace(-1, 9, "something")); } @Test @@ -90,13 +91,17 @@ public class StringModifierTest extends GerritBaseTests { @Test public void startIndexMustNotBeGreaterThanLengthOfString() { - exception.expect(StringIndexOutOfBoundsException.class); - stringModifier.replace(originalString.length() + 1, originalString.length() + 1, "something"); + assertThrows( + StringIndexOutOfBoundsException.class, + () -> + stringModifier.replace( + originalString.length() + 1, originalString.length() + 1, "something")); } @Test public void endIndexMustNotBeGreaterThanLengthOfString() { - exception.expect(StringIndexOutOfBoundsException.class); - stringModifier.replace(8, originalString.length() + 1, "something"); + assertThrows( + StringIndexOutOfBoundsException.class, + () -> stringModifier.replace(8, originalString.length() + 1, "something")); } } diff --git a/javatests/com/google/gerrit/server/git/LocalDiskRepositoryManagerTest.java b/javatests/com/google/gerrit/server/git/LocalDiskRepositoryManagerTest.java index 68afd78c24..5ff67cff5e 100644 --- a/javatests/com/google/gerrit/server/git/LocalDiskRepositoryManagerTest.java +++ b/javatests/com/google/gerrit/server/git/LocalDiskRepositoryManagerTest.java @@ -16,6 +16,7 @@ package com.google.gerrit.server.git; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.TruthJUnit.assume; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.server.config.SitePaths; @@ -52,9 +53,10 @@ public class LocalDiskRepositoryManagerTest extends GerritBaseTests { repoManager = new LocalDiskRepositoryManager(site, cfg); } - @Test(expected = IllegalStateException.class) + @Test public void testThatNullBasePathThrowsAnException() { - new LocalDiskRepositoryManager(site, new Config()); + assertThrows( + IllegalStateException.class, () -> new LocalDiskRepositoryManager(site, new Config())); } @Test @@ -69,107 +71,144 @@ public class LocalDiskRepositoryManagerTest extends GerritBaseTests { assertThat(repoManager.list()).containsExactly(projectA); } - @Test(expected = RepositoryNotFoundException.class) + @Test public void testProjectCreationWithEmptyName() throws Exception { - repoManager.createRepository(Project.nameKey("")); + assertThrows( + RepositoryNotFoundException.class, () -> repoManager.createRepository(Project.nameKey(""))); } - @Test(expected = RepositoryNotFoundException.class) + @Test public void testProjectCreationWithTrailingSlash() throws Exception { - repoManager.createRepository(Project.nameKey("projectA/")); + assertThrows( + RepositoryNotFoundException.class, + () -> repoManager.createRepository(Project.nameKey("projectA/"))); } - @Test(expected = RepositoryNotFoundException.class) + @Test public void testProjectCreationWithBackSlash() throws Exception { - repoManager.createRepository(Project.nameKey("a\\projectA")); + assertThrows( + RepositoryNotFoundException.class, + () -> repoManager.createRepository(Project.nameKey("a\\projectA"))); } - @Test(expected = RepositoryNotFoundException.class) + @Test public void testProjectCreationAbsolutePath() throws Exception { - repoManager.createRepository(Project.nameKey("/projectA")); + assertThrows( + RepositoryNotFoundException.class, + () -> repoManager.createRepository(Project.nameKey("/projectA"))); } - @Test(expected = RepositoryNotFoundException.class) + @Test public void testProjectCreationStartingWithDotDot() throws Exception { - repoManager.createRepository(Project.nameKey("../projectA")); + assertThrows( + RepositoryNotFoundException.class, + () -> repoManager.createRepository(Project.nameKey("../projectA"))); } - @Test(expected = RepositoryNotFoundException.class) + @Test public void testProjectCreationContainsDotDot() throws Exception { - repoManager.createRepository(Project.nameKey("a/../projectA")); + assertThrows( + RepositoryNotFoundException.class, + () -> repoManager.createRepository(Project.nameKey("a/../projectA"))); } - @Test(expected = RepositoryNotFoundException.class) + @Test public void testProjectCreationDotPathSegment() throws Exception { - repoManager.createRepository(Project.nameKey("a/./projectA")); + assertThrows( + RepositoryNotFoundException.class, + () -> repoManager.createRepository(Project.nameKey("a/./projectA"))); } - @Test(expected = RepositoryNotFoundException.class) + @Test public void testProjectCreationWithTwoSlashes() throws Exception { - repoManager.createRepository(Project.nameKey("a//projectA")); + assertThrows( + RepositoryNotFoundException.class, + () -> repoManager.createRepository(Project.nameKey("a//projectA"))); } - @Test(expected = RepositoryNotFoundException.class) + @Test public void testProjectCreationWithPathSegmentEndingByDotGit() throws Exception { - repoManager.createRepository(Project.nameKey("a/b.git/projectA")); + assertThrows( + RepositoryNotFoundException.class, + () -> repoManager.createRepository(Project.nameKey("a/b.git/projectA"))); } - @Test(expected = RepositoryNotFoundException.class) + @Test public void testProjectCreationWithQuestionMark() throws Exception { - repoManager.createRepository(Project.nameKey("project?A")); + assertThrows( + RepositoryNotFoundException.class, + () -> repoManager.createRepository(Project.nameKey("project?A"))); } - @Test(expected = RepositoryNotFoundException.class) + @Test public void testProjectCreationWithPercentageSign() throws Exception { - repoManager.createRepository(Project.nameKey("project%A")); + assertThrows( + RepositoryNotFoundException.class, + () -> repoManager.createRepository(Project.nameKey("project%A"))); } - @Test(expected = RepositoryNotFoundException.class) + @Test public void testProjectCreationWithWidlcard() throws Exception { - repoManager.createRepository(Project.nameKey("project*A")); + assertThrows( + RepositoryNotFoundException.class, + () -> repoManager.createRepository(Project.nameKey("project*A"))); } - @Test(expected = RepositoryNotFoundException.class) + @Test public void testProjectCreationWithColon() throws Exception { - repoManager.createRepository(Project.nameKey("project:A")); + assertThrows( + RepositoryNotFoundException.class, + () -> repoManager.createRepository(Project.nameKey("project:A"))); } - @Test(expected = RepositoryNotFoundException.class) + @Test public void testProjectCreationWithLessThatSign() throws Exception { - repoManager.createRepository(Project.nameKey("project repoManager.createRepository(Project.nameKey("projectA")); + assertThrows( + RepositoryNotFoundException.class, + () -> repoManager.createRepository(Project.nameKey("project>A"))); } - @Test(expected = RepositoryNotFoundException.class) + @Test public void testProjectCreationWithPipe() throws Exception { - repoManager.createRepository(Project.nameKey("project|A")); + assertThrows( + RepositoryNotFoundException.class, + () -> repoManager.createRepository(Project.nameKey("project|A"))); } - @Test(expected = RepositoryNotFoundException.class) + @Test public void testProjectCreationWithDollarSign() throws Exception { - repoManager.createRepository(Project.nameKey("project$A")); + assertThrows( + RepositoryNotFoundException.class, + () -> repoManager.createRepository(Project.nameKey("project$A"))); } - @Test(expected = RepositoryNotFoundException.class) + @Test public void testProjectCreationWithCarriageReturn() throws Exception { - repoManager.createRepository(Project.nameKey("project\\rA")); + assertThrows( + RepositoryNotFoundException.class, + () -> repoManager.createRepository(Project.nameKey("project\\rA"))); } - @Test(expected = IllegalStateException.class) + @Test public void testProjectRecreation() throws Exception { repoManager.createRepository(Project.nameKey("a")); - repoManager.createRepository(Project.nameKey("a")); + assertThrows( + IllegalStateException.class, () -> repoManager.createRepository(Project.nameKey("a"))); } - @Test(expected = IllegalStateException.class) + @Test public void testProjectRecreationAfterRestart() throws Exception { repoManager.createRepository(Project.nameKey("a")); LocalDiskRepositoryManager newRepoManager = new LocalDiskRepositoryManager(site, cfg); - newRepoManager.createRepository(Project.nameKey("a")); + assertThrows( + IllegalStateException.class, () -> newRepoManager.createRepository(Project.nameKey("a"))); } @Test @@ -182,30 +221,36 @@ public class LocalDiskRepositoryManagerTest extends GerritBaseTests { assertThat(repoManager.list()).containsExactly(projectA); } - @Test(expected = RepositoryCaseMismatchException.class) + @Test public void testNameCaseMismatch() throws Exception { assume().that(HostPlatform.isWin32() || HostPlatform.isMac()).isTrue(); repoManager.createRepository(Project.nameKey("a")); - repoManager.createRepository(Project.nameKey("A")); + assertThrows( + RepositoryCaseMismatchException.class, + () -> repoManager.createRepository(Project.nameKey("A"))); } - @Test(expected = RepositoryCaseMismatchException.class) + @Test public void testNameCaseMismatchWithSymlink() throws Exception { assume().that(HostPlatform.isWin32() || HostPlatform.isMac()).isTrue(); Project.NameKey name = Project.nameKey("a"); repoManager.createRepository(name); createSymLink(name, "b.git"); - repoManager.createRepository(Project.nameKey("B")); + assertThrows( + RepositoryCaseMismatchException.class, + () -> repoManager.createRepository(Project.nameKey("B"))); } - @Test(expected = RepositoryCaseMismatchException.class) + @Test public void testNameCaseMismatchAfterRestart() throws Exception { assume().that(HostPlatform.isWin32() || HostPlatform.isMac()).isTrue(); Project.NameKey name = Project.nameKey("a"); repoManager.createRepository(name); LocalDiskRepositoryManager newRepoManager = new LocalDiskRepositoryManager(site, cfg); - newRepoManager.createRepository(Project.nameKey("A")); + assertThrows( + RepositoryCaseMismatchException.class, + () -> newRepoManager.createRepository(Project.nameKey("A"))); } private void createSymLink(Project.NameKey project, String link) throws IOException { @@ -215,9 +260,11 @@ public class LocalDiskRepositoryManagerTest extends GerritBaseTests { Files.createSymbolicLink(symlink, projectDir); } - @Test(expected = RepositoryNotFoundException.class) + @Test public void testOpenRepositoryInvalidName() throws Exception { - repoManager.openRepository(Project.nameKey("project%?|<>A")); + assertThrows( + RepositoryNotFoundException.class, + () -> repoManager.openRepository(Project.nameKey("project%?|<>A"))); } @Test diff --git a/javatests/com/google/gerrit/server/git/MultiBaseLocalDiskRepositoryManagerTest.java b/javatests/com/google/gerrit/server/git/MultiBaseLocalDiskRepositoryManagerTest.java index 550e379b37..f5fe7df42b 100644 --- a/javatests/com/google/gerrit/server/git/MultiBaseLocalDiskRepositoryManagerTest.java +++ b/javatests/com/google/gerrit/server/git/MultiBaseLocalDiskRepositoryManagerTest.java @@ -15,6 +15,7 @@ package com.google.gerrit.server.git; import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static org.easymock.EasyMock.createNiceMock; import static org.easymock.EasyMock.expect; import static org.easymock.EasyMock.replay; @@ -150,11 +151,17 @@ public class MultiBaseLocalDiskRepositoryManagerTest extends GerritBaseTests { } } - @Test(expected = IllegalStateException.class) + @Test public void testRelativeAlternateLocation() { - configMock = createNiceMock(RepositoryConfig.class); - expect(configMock.getAllBasePaths()).andReturn(ImmutableList.of(Paths.get("repos"))).anyTimes(); - replay(configMock); - repoManager = new MultiBaseLocalDiskRepositoryManager(site, cfg, configMock); + assertThrows( + IllegalStateException.class, + () -> { + configMock = createNiceMock(RepositoryConfig.class); + expect(configMock.getAllBasePaths()) + .andReturn(ImmutableList.of(Paths.get("repos"))) + .anyTimes(); + replay(configMock); + repoManager = new MultiBaseLocalDiskRepositoryManager(site, cfg, configMock); + }); } } diff --git a/javatests/com/google/gerrit/server/group/db/GroupConfigTest.java b/javatests/com/google/gerrit/server/group/db/GroupConfigTest.java index 7be7f1c82a..773c2ee183 100644 --- a/javatests/com/google/gerrit/server/group/db/GroupConfigTest.java +++ b/javatests/com/google/gerrit/server/group/db/GroupConfigTest.java @@ -17,8 +17,10 @@ package com.google.gerrit.server.group.db; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; import static com.google.gerrit.server.group.testing.InternalGroupSubject.internalGroups; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static com.google.gerrit.truth.OptionalSubject.assertThat; import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.MatcherAssert.assertThat; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; @@ -113,9 +115,9 @@ public class GroupConfigTest extends GerritBaseTests { GroupConfig groupConfig = GroupConfig.createForNewGroup(projectName, repository, groupCreation); try (MetaDataUpdate metaDataUpdate = createMetaDataUpdate()) { - exception.expectCause(instanceOf(ConfigInvalidException.class)); - exception.expectMessage("Name of the group " + groupUuid); - groupConfig.commit(metaDataUpdate); + Throwable thrown = assertThrows(Throwable.class, () -> groupConfig.commit(metaDataUpdate)); + assertThat(thrown.getCause(), instanceOf(ConfigInvalidException.class)); + assertThat(thrown).hasMessageThat().contains("Name of the group " + groupUuid); } } @@ -135,9 +137,9 @@ public class GroupConfigTest extends GerritBaseTests { GroupConfig groupConfig = GroupConfig.createForNewGroup(projectName, repository, groupCreation); try (MetaDataUpdate metaDataUpdate = createMetaDataUpdate()) { - exception.expectCause(instanceOf(ConfigInvalidException.class)); - exception.expectMessage("ID of the group " + groupUuid); - groupConfig.commit(metaDataUpdate); + Throwable thrown = assertThrows(Throwable.class, () -> groupConfig.commit(metaDataUpdate)); + assertThat(thrown.getCause(), instanceOf(ConfigInvalidException.class)); + assertThat(thrown).hasMessageThat().contains("ID of the group " + groupUuid); } } @@ -214,9 +216,9 @@ public class GroupConfigTest extends GerritBaseTests { groupConfig.setGroupUpdate(groupUpdate, auditLogFormatter); try (MetaDataUpdate metaDataUpdate = createMetaDataUpdate()) { - exception.expectCause(instanceOf(ConfigInvalidException.class)); - exception.expectMessage("Owner UUID of the group " + groupUuid); - groupConfig.commit(metaDataUpdate); + Throwable thrown = assertThrows(Throwable.class, () -> groupConfig.commit(metaDataUpdate)); + assertThat(thrown.getCause(), instanceOf(ConfigInvalidException.class)); + assertThat(thrown).hasMessageThat().contains("Owner UUID of the group " + groupUuid); } } @@ -325,9 +327,11 @@ public class GroupConfigTest extends GerritBaseTests { public void idInConfigMustBeDefined() throws Exception { populateGroupConfig(groupUuid, "[group]\n\tname = users\n\townerGroupUuid = owners\n"); - exception.expect(ConfigInvalidException.class); - exception.expectMessage("ID of the group " + groupUuid); - GroupConfig.loadForGroup(projectName, repository, groupUuid); + ConfigInvalidException thrown = + assertThrows( + ConfigInvalidException.class, + () -> GroupConfig.loadForGroup(projectName, repository, groupUuid)); + assertThat(thrown).hasMessageThat().contains("ID of the group " + groupUuid); } @Test @@ -335,9 +339,11 @@ public class GroupConfigTest extends GerritBaseTests { populateGroupConfig( groupUuid, "[group]\n\tname = users\n\tid = -5\n\townerGroupUuid = owners\n"); - exception.expect(ConfigInvalidException.class); - exception.expectMessage("ID of the group " + groupUuid); - GroupConfig.loadForGroup(projectName, repository, groupUuid); + ConfigInvalidException thrown = + assertThrows( + ConfigInvalidException.class, + () -> GroupConfig.loadForGroup(projectName, repository, groupUuid)); + assertThat(thrown).hasMessageThat().contains("ID of the group " + groupUuid); } @Test @@ -361,9 +367,11 @@ public class GroupConfigTest extends GerritBaseTests { public void ownerGroupUuidInConfigMustBeDefined() throws Exception { populateGroupConfig(groupUuid, "[group]\n\tname = users\n\tid = 42\n"); - exception.expect(ConfigInvalidException.class); - exception.expectMessage("Owner UUID of the group " + groupUuid); - GroupConfig.loadForGroup(projectName, repository, groupUuid); + ConfigInvalidException thrown = + assertThrows( + ConfigInvalidException.class, + () -> GroupConfig.loadForGroup(projectName, repository, groupUuid)); + assertThat(thrown).hasMessageThat().contains("Owner UUID of the group " + groupUuid); } @Test @@ -415,9 +423,9 @@ public class GroupConfigTest extends GerritBaseTests { populateGroupConfig(groupUuid, "[group]\n\tname=users\n\tid = 42\n\townerGroupUuid = owners\n"); populateMembersFile(groupUuid, "One"); - exception.expect(ConfigInvalidException.class); - exception.expectMessage("Invalid file members"); - loadGroup(groupUuid); + ConfigInvalidException thrown = + assertThrows(ConfigInvalidException.class, () -> loadGroup(groupUuid)); + assertThat(thrown).hasMessageThat().contains("Invalid file members"); } @Test @@ -425,9 +433,9 @@ public class GroupConfigTest extends GerritBaseTests { populateGroupConfig(groupUuid, "[group]\n\tname=users\n\tid = 42\n\townerGroupUuid = owners\n"); populateMembersFile(groupUuid, "1\t2"); - exception.expect(ConfigInvalidException.class); - exception.expectMessage("Invalid file members"); - loadGroup(groupUuid); + ConfigInvalidException thrown = + assertThrows(ConfigInvalidException.class, () -> loadGroup(groupUuid)); + assertThat(thrown).hasMessageThat().contains("Invalid file members"); } @Test @@ -517,9 +525,9 @@ public class GroupConfigTest extends GerritBaseTests { groupConfig.setGroupUpdate(groupUpdate, auditLogFormatter); try (MetaDataUpdate metaDataUpdate = createMetaDataUpdate()) { - exception.expectCause(instanceOf(ConfigInvalidException.class)); - exception.expectMessage("Name of the group " + groupUuid); - groupConfig.commit(metaDataUpdate); + Throwable thrown = assertThrows(Throwable.class, () -> groupConfig.commit(metaDataUpdate)); + assertThat(thrown.getCause(), instanceOf(ConfigInvalidException.class)); + assertThat(thrown).hasMessageThat().contains("Name of the group " + groupUuid); } } @@ -584,9 +592,9 @@ public class GroupConfigTest extends GerritBaseTests { groupConfig.setGroupUpdate(groupUpdate, auditLogFormatter); try (MetaDataUpdate metaDataUpdate = createMetaDataUpdate()) { - exception.expectCause(instanceOf(ConfigInvalidException.class)); - exception.expectMessage("Owner UUID of the group " + groupUuid); - groupConfig.commit(metaDataUpdate); + Throwable thrown = assertThrows(Throwable.class, () -> groupConfig.commit(metaDataUpdate)); + assertThat(thrown.getCause(), instanceOf(ConfigInvalidException.class)); + assertThat(thrown).hasMessageThat().contains("Owner UUID of the group " + groupUuid); } } diff --git a/javatests/com/google/gerrit/server/group/db/GroupNameNotesTest.java b/javatests/com/google/gerrit/server/group/db/GroupNameNotesTest.java index 085e2ad079..b2580f8549 100644 --- a/javatests/com/google/gerrit/server/group/db/GroupNameNotesTest.java +++ b/javatests/com/google/gerrit/server/group/db/GroupNameNotesTest.java @@ -20,6 +20,7 @@ import static com.google.gerrit.common.data.testing.GroupReferenceSubject.groupR import static com.google.gerrit.extensions.common.testing.CommitInfoSubject.assertThat; import static com.google.gerrit.extensions.common.testing.CommitInfoSubject.commits; import static com.google.gerrit.reviewdb.client.RefNames.REFS_GROUPNAMES; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static com.google.gerrit.truth.OptionalSubject.assertThat; import static java.nio.charset.StandardCharsets.UTF_8; import static org.eclipse.jgit.lib.Constants.OBJ_BLOB; @@ -105,14 +106,16 @@ public class GroupNameNotesTest extends GerritBaseTests { @Test public void uuidOfNewGroupMustNotBeNull() throws Exception { - exception.expect(NullPointerException.class); - GroupNameNotes.forNewGroup(allUsersName, repo, null, groupName); + assertThrows( + NullPointerException.class, + () -> GroupNameNotes.forNewGroup(allUsersName, repo, null, groupName)); } @Test public void nameOfNewGroupMustNotBeNull() throws Exception { - exception.expect(NullPointerException.class); - GroupNameNotes.forNewGroup(allUsersName, repo, groupUuid, null); + assertThrows( + NullPointerException.class, + () -> GroupNameNotes.forNewGroup(allUsersName, repo, groupUuid, null)); } @Test @@ -129,9 +132,11 @@ public class GroupNameNotesTest extends GerritBaseTests { createGroup(groupUuid, groupName); AccountGroup.UUID anotherGroupUuid = AccountGroup.uuid("AnotherGroup"); - exception.expect(DuplicateKeyException.class); - exception.expectMessage(groupName.get()); - GroupNameNotes.forNewGroup(allUsersName, repo, anotherGroupUuid, groupName); + DuplicateKeyException thrown = + assertThrows( + DuplicateKeyException.class, + () -> GroupNameNotes.forNewGroup(allUsersName, repo, anotherGroupUuid, groupName)); + assertThat(thrown).hasMessageThat().contains(groupName.get()); } @Test @@ -173,9 +178,9 @@ public class GroupNameNotesTest extends GerritBaseTests { @Test public void groupCannotBeRenamedToNull() throws Exception { createGroup(groupUuid, groupName); - - exception.expect(NullPointerException.class); - GroupNameNotes.forRename(allUsersName, repo, groupUuid, groupName, null); + assertThrows( + NullPointerException.class, + () -> GroupNameNotes.forRename(allUsersName, repo, groupUuid, groupName, null)); } @Test @@ -183,8 +188,9 @@ public class GroupNameNotesTest extends GerritBaseTests { createGroup(groupUuid, groupName); AccountGroup.NameKey anotherName = AccountGroup.nameKey("admins"); - exception.expect(NullPointerException.class); - GroupNameNotes.forRename(allUsersName, repo, groupUuid, null, anotherName); + assertThrows( + NullPointerException.class, + () -> GroupNameNotes.forRename(allUsersName, repo, groupUuid, null, anotherName)); } @Test @@ -193,9 +199,13 @@ public class GroupNameNotesTest extends GerritBaseTests { AccountGroup.NameKey anotherOldName = AccountGroup.nameKey("contributors"); AccountGroup.NameKey anotherName = AccountGroup.nameKey("admins"); - exception.expect(ConfigInvalidException.class); - exception.expectMessage(anotherOldName.get()); - GroupNameNotes.forRename(allUsersName, repo, groupUuid, anotherOldName, anotherName); + ConfigInvalidException thrown = + assertThrows( + ConfigInvalidException.class, + () -> + GroupNameNotes.forRename( + allUsersName, repo, groupUuid, anotherOldName, anotherName)); + assertThat(thrown).hasMessageThat().contains(anotherOldName.get()); } @Test @@ -205,9 +215,13 @@ public class GroupNameNotesTest extends GerritBaseTests { AccountGroup.NameKey anotherGroupName = AccountGroup.nameKey("admins"); createGroup(anotherGroupUuid, anotherGroupName); - exception.expect(DuplicateKeyException.class); - exception.expectMessage(anotherGroupName.get()); - GroupNameNotes.forRename(allUsersName, repo, groupUuid, groupName, anotherGroupName); + DuplicateKeyException thrown = + assertThrows( + DuplicateKeyException.class, + () -> + GroupNameNotes.forRename( + allUsersName, repo, groupUuid, groupName, anotherGroupName)); + assertThat(thrown).hasMessageThat().contains(anotherGroupName.get()); } @Test @@ -215,8 +229,9 @@ public class GroupNameNotesTest extends GerritBaseTests { createGroup(groupUuid, groupName); AccountGroup.NameKey anotherName = AccountGroup.nameKey("admins"); - exception.expect(NullPointerException.class); - GroupNameNotes.forRename(allUsersName, repo, null, groupName, anotherName); + assertThrows( + NullPointerException.class, + () -> GroupNameNotes.forRename(allUsersName, repo, null, groupName, anotherName)); } @Test @@ -225,9 +240,13 @@ public class GroupNameNotesTest extends GerritBaseTests { AccountGroup.UUID anotherGroupUuid = AccountGroup.uuid("admins-ABC"); AccountGroup.NameKey anotherName = AccountGroup.nameKey("admins"); - exception.expect(ConfigInvalidException.class); - exception.expectMessage(groupUuid.get()); - GroupNameNotes.forRename(allUsersName, repo, anotherGroupUuid, groupName, anotherName); + ConfigInvalidException thrown = + assertThrows( + ConfigInvalidException.class, + () -> + GroupNameNotes.forRename( + allUsersName, repo, anotherGroupUuid, groupName, anotherName)); + assertThat(thrown).hasMessageThat().contains(groupUuid.get()); } @Test diff --git a/javatests/com/google/gerrit/server/index/change/ChangeIndexRewriterTest.java b/javatests/com/google/gerrit/server/index/change/ChangeIndexRewriterTest.java index fd23da3679..0581fddcff 100644 --- a/javatests/com/google/gerrit/server/index/change/ChangeIndexRewriterTest.java +++ b/javatests/com/google/gerrit/server/index/change/ChangeIndexRewriterTest.java @@ -21,6 +21,7 @@ import static com.google.gerrit.index.query.Predicate.or; import static com.google.gerrit.reviewdb.client.Change.Status.MERGED; import static com.google.gerrit.reviewdb.client.Change.Status.NEW; import static com.google.gerrit.server.index.change.IndexedChangeQuery.convertOptions; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static org.junit.Assert.assertEquals; import com.google.common.collect.ImmutableSet; @@ -196,9 +197,8 @@ public class ChangeIndexRewriterTest extends GerritBaseTests { indexes.setSearchIndex(new FakeChangeIndex(FakeChangeIndex.V1)); - exception.expect(QueryParseException.class); - exception.expectMessage("Unsupported index predicate: file:a"); - rewrite(in); + QueryParseException thrown = assertThrows(QueryParseException.class, () -> rewrite(in)); + assertThat(thrown).hasMessageThat().contains("Unsupported index predicate: file:a"); } @Test @@ -207,9 +207,9 @@ public class ChangeIndexRewriterTest extends GerritBaseTests { Predicate in = parse(q); assertEquals(query(in), rewrite(in)); - exception.expect(QueryParseException.class); - exception.expectMessage("too many terms in query"); - rewrite(parse(q + " OR file:d")); + QueryParseException thrown = + assertThrows(QueryParseException.class, () -> rewrite(parse(q + " OR file:d"))); + assertThat(thrown).hasMessageThat().contains("too many terms in query"); } @Test diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java index fe01bc727c..5f57b92ab5 100644 --- a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java +++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java @@ -22,10 +22,10 @@ import static com.google.gerrit.reviewdb.client.RefNames.refsDraftComments; import static com.google.gerrit.server.notedb.ReviewerStateInternal.CC; import static com.google.gerrit.server.notedb.ReviewerStateInternal.REMOVED; import static com.google.gerrit.server.notedb.ReviewerStateInternal.REVIEWER; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.Comparator.comparing; import static org.eclipse.jgit.lib.Constants.OBJ_BLOB; -import static org.junit.Assert.fail; import com.google.common.base.Throwables; import com.google.common.collect.ImmutableList; @@ -819,14 +819,17 @@ public class ChangeNotesTest extends AbstractChangeNotesTest { // Trying to set another Change-Id fails String otherChangeId = "I577fb248e474018276351785930358ec0450e9f7"; - update = newUpdate(c, changeOwner); - exception.expect(IllegalArgumentException.class); - exception.expectMessage( - "The Change-Id was already set to " - + c.getKey() - + ", so we cannot set this Change-Id: " - + otherChangeId); - update.setChangeId(otherChangeId); + ChangeUpdate failingUpdate = newUpdate(c, changeOwner); + IllegalArgumentException thrown = + assertThrows( + IllegalArgumentException.class, () -> failingUpdate.setChangeId(otherChangeId)); + assertThat(thrown) + .hasMessageThat() + .contains( + "The Change-Id was already set to " + + c.getKey() + + ", so we cannot set this Change-Id: " + + otherChangeId); } @Test @@ -990,19 +993,15 @@ public class ChangeNotesTest extends AbstractChangeNotesTest { update.setCommit(rw, commit); update.commit(); - try { - newNotes(c); - fail("Expected IOException"); - } catch (StorageException e) { - assertCause( - e, - ConfigInvalidException.class, - "Multiple revisions parsed for patch set 1:" - + " " - + commit.name() - + " and " - + ps.getCommitId().name()); - } + StorageException e = assertThrows(StorageException.class, () -> newNotes(c)); + assertCause( + e, + ConfigInvalidException.class, + "Multiple revisions parsed for patch set 1:" + + " " + + commit.name() + + " and " + + ps.getCommitId().name()); } @Test @@ -2533,9 +2532,9 @@ public class ChangeNotesTest extends AbstractChangeNotesTest { assertThat(msg.getMessage()).isEqualTo("A message."); assertThat(msg.getAuthor()).isNull(); - update = newUpdate(c, internalUser); - exception.expect(IllegalStateException.class); - update.putApproval("Code-Review", (short) 1); + ChangeUpdate failingUpdate = newUpdate(c, internalUser); + assertThrows( + IllegalStateException.class, () -> failingUpdate.putApproval("Code-Review", (short) 1)); } @Test @@ -3004,9 +3003,9 @@ public class ChangeNotesTest extends AbstractChangeNotesTest { public void setRevertOfToCurrentChangeFails() throws Exception { Change c = newChange(); ChangeUpdate update = newUpdate(c, changeOwner); - exception.expect(IllegalArgumentException.class); - exception.expectMessage("A change cannot revert itself"); - update.setRevertOf(c.getId().get()); + IllegalArgumentException thrown = + assertThrows(IllegalArgumentException.class, () -> update.setRevertOf(c.getId().get())); + assertThat(thrown).hasMessageThat().contains("A change cannot revert itself"); } @Test @@ -3014,9 +3013,10 @@ public class ChangeNotesTest extends AbstractChangeNotesTest { Change c = newChange(); ChangeUpdate update = newUpdate(c, changeOwner); update.setRevertOf(newChange().getId().get()); - exception.expect(StorageException.class); - exception.expectMessage("Given ChangeUpdate is only allowed on initial commit"); - update.commit(); + StorageException thrown = assertThrows(StorageException.class, () -> update.commit()); + assertThat(thrown) + .hasMessageThat() + .contains("Given ChangeUpdate is only allowed on initial commit"); } @Test diff --git a/javatests/com/google/gerrit/server/notedb/RepoSequenceTest.java b/javatests/com/google/gerrit/server/notedb/RepoSequenceTest.java index 3a4bc5cbd7..66c79b8b21 100644 --- a/javatests/com/google/gerrit/server/notedb/RepoSequenceTest.java +++ b/javatests/com/google/gerrit/server/notedb/RepoSequenceTest.java @@ -16,9 +16,9 @@ package com.google.gerrit.server.notedb; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static java.nio.charset.StandardCharsets.UTF_8; import static org.eclipse.jgit.lib.Constants.OBJ_BLOB; -import static org.junit.Assert.fail; import com.github.rholder.retry.Retryer; import com.github.rholder.retry.RetryerBuilder; @@ -169,9 +169,11 @@ public class RepoSequenceTest extends GerritBaseTests { @Test public void failOnInvalidValue() throws Exception { ObjectId id = writeBlob("id", "not a number"); - exception.expect(StorageException.class); - exception.expectMessage("invalid value in refs/sequences/id blob at " + id.name()); - newSequence("id", 1, 3).next(); + StorageException thrown = + assertThrows(StorageException.class, () -> newSequence("id", 1, 3).next()); + assertThat(thrown) + .hasMessageThat() + .contains("invalid value in refs/sequences/id blob at " + id.name()); } @Test @@ -179,13 +181,10 @@ public class RepoSequenceTest extends GerritBaseTests { try (Repository repo = repoManager.openRepository(project)) { TestRepository tr = new TestRepository<>(repo); tr.branch(RefNames.REFS_SEQUENCES + "id").commit().create(); - try { - newSequence("id", 1, 3).next(); - fail(); - } catch (StorageException e) { - assertThat(e.getCause()).isInstanceOf(ExecutionException.class); - assertThat(e.getCause().getCause()).isInstanceOf(IncorrectObjectTypeException.class); - } + StorageException e = + assertThrows(StorageException.class, () -> newSequence("id", 1, 3).next()); + assertThat(e.getCause()).isInstanceOf(ExecutionException.class); + assertThat(e.getCause().getCause()).isInstanceOf(IncorrectObjectTypeException.class); } } @@ -201,9 +200,10 @@ public class RepoSequenceTest extends GerritBaseTests { RetryerBuilder.newBuilder() .withStopStrategy(StopStrategies.stopAfterAttempt(3)) .build()); - exception.expect(StorageException.class); - exception.expectMessage("Failed to update refs/sequences/id: LOCK_FAILURE"); - s.next(); + StorageException thrown = assertThrows(StorageException.class, () -> s.next()); + assertThat(thrown) + .hasMessageThat() + .contains("Failed to update refs/sequences/id: LOCK_FAILURE"); } @Test @@ -336,9 +336,10 @@ public class RepoSequenceTest extends GerritBaseTests { RetryerBuilder.newBuilder() .withStopStrategy(StopStrategies.stopAfterAttempt(3)) .build()); - exception.expect(StorageException.class); - exception.expectMessage("Failed to update refs/sequences/id: LOCK_FAILURE"); - s.increaseTo(2); + StorageException thrown = assertThrows(StorageException.class, () -> s.increaseTo(2)); + assertThat(thrown) + .hasMessageThat() + .contains("Failed to update refs/sequences/id: LOCK_FAILURE"); } private RepoSequence newSequence(String name, int start, int batchSize) { diff --git a/javatests/com/google/gerrit/server/patch/IntraLineLoaderTest.java b/javatests/com/google/gerrit/server/patch/IntraLineLoaderTest.java index 0cc4b00786..16cf696951 100644 --- a/javatests/com/google/gerrit/server/patch/IntraLineLoaderTest.java +++ b/javatests/com/google/gerrit/server/patch/IntraLineLoaderTest.java @@ -15,6 +15,7 @@ package com.google.gerrit.server.patch; import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static java.nio.charset.StandardCharsets.UTF_8; import com.google.common.collect.ImmutableList; @@ -88,22 +89,30 @@ public class IntraLineLoaderTest extends GerritBaseTests { // TODO: expected failure // the current code does not work on the first line // and the insert marker is in the wrong location - @Test(expected = AssertionError.class) + @Test public void preferInsertAtLineBreak2() throws Exception { - String a = " abc\n def\n"; - String b = " abc\n def\n"; - assertThat(intraline(a, b)) - .isEqualTo(ref().insert(" ").common(" abc\n").insert(" ").common(" def\n").edits); + assertThrows( + AssertionError.class, + () -> { + String a = " abc\n def\n"; + String b = " abc\n def\n"; + assertThat(intraline(a, b)) + .isEqualTo(ref().insert(" ").common(" abc\n").insert(" ").common(" def\n").edits); + }); } // TODO: expected failure // the current code does not work on the first line - @Test(expected = AssertionError.class) + @Test public void preferDeleteAtLineBreak() throws Exception { - String a = " abc\n def\n"; - String b = " abc\n def\n"; - assertThat(intraline(a, b)) - .isEqualTo(ref().remove(" ").common(" abc\n").remove(" ").common(" def\n").edits); + assertThrows( + AssertionError.class, + () -> { + String a = " abc\n def\n"; + String b = " abc\n def\n"; + assertThat(intraline(a, b)) + .isEqualTo(ref().remove(" ").common(" abc\n").remove(" ").common(" def\n").edits); + }); } @Test diff --git a/javatests/com/google/gerrit/server/permissions/RefControlTest.java b/javatests/com/google/gerrit/server/permissions/RefControlTest.java index 0c40d4e153..df174f2a44 100644 --- a/javatests/com/google/gerrit/server/permissions/RefControlTest.java +++ b/javatests/com/google/gerrit/server/permissions/RefControlTest.java @@ -32,6 +32,7 @@ import static com.google.gerrit.server.project.testing.Util.allowExclusive; import static com.google.gerrit.server.project.testing.Util.block; import static com.google.gerrit.server.project.testing.Util.deny; import static com.google.gerrit.server.project.testing.Util.doNotInherit; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static com.google.gerrit.testing.InMemoryRepositoryManager.newRepository; import com.google.common.cache.Cache; @@ -915,14 +916,16 @@ public class RefControlTest extends GerritBaseTests { RefPattern.validate("refs/heads/review/${username}/*"); } - @Test(expected = InvalidNameException.class) + @Test public void testValidateBadRefPatternDoubleCaret() throws Exception { - RefPattern.validate("^^refs/*"); + assertThrows(InvalidNameException.class, () -> RefPattern.validate("^^refs/*")); } - @Test(expected = InvalidNameException.class) + @Test public void testValidateBadRefPatternDanglingCharacter() throws Exception { - RefPattern.validate("^refs/heads/tmp/sdk/[0-9]{3,3}_R[1-9][A-Z][0-9]{3,3}*"); + assertThrows( + InvalidNameException.class, + () -> RefPattern.validate("^refs/heads/tmp/sdk/[0-9]{3,3}_R[1-9][A-Z][0-9]{3,3}*")); } @Test diff --git a/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java b/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java index 1076edac45..2a7523b3ac 100644 --- a/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java +++ b/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java @@ -17,6 +17,7 @@ package com.google.gerrit.server.query.account; 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.testing.GerritJUnit.assertThrows; import static java.util.stream.Collectors.toList; import static org.junit.Assert.fail; @@ -584,9 +585,9 @@ public abstract class AbstractQueryAccountsTest extends GerritServerTests { List result = newQuery(otherUser.username).withSuggest(true).get(); assertThat(result.get(0).secondaryEmails).isNull(); - - exception.expect(AuthException.class); - newQuery(otherUser.username).withOption(ListAccountsOption.ALL_EMAILS).get(); + assertThrows( + AuthException.class, + () -> newQuery(otherUser.username).withOption(ListAccountsOption.ALL_EMAILS).get()); } @Test diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java index 0106ba40d2..2514b5bf10 100644 --- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java +++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java @@ -25,6 +25,7 @@ import static com.google.gerrit.server.project.testing.Util.allow; import static com.google.gerrit.server.project.testing.Util.category; import static com.google.gerrit.server.project.testing.Util.value; import static com.google.gerrit.server.project.testing.Util.verified; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static java.util.concurrent.TimeUnit.HOURS; import static java.util.concurrent.TimeUnit.MILLISECONDS; import static java.util.concurrent.TimeUnit.MINUTES; @@ -703,10 +704,10 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { assertQuery(searchOperator + "\"John Smith\""); // By invalid query. - exception.expect(BadRequestException.class); - exception.expectMessage("invalid value"); // SchemaUtil.getNameParts will return an empty set for query only containing these characters. - assertQuery(searchOperator + "@.- /_"); + BadRequestException thrown = + assertThrows(BadRequestException.class, () -> assertQuery(searchOperator + "@.- /_")); + assertThat(thrown).hasMessageThat().contains("invalid value"); } private Change createChange(TestRepository repo, PersonIdent person) throws Exception { diff --git a/javatests/com/google/gerrit/server/query/change/LuceneQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/LuceneQueryChangesTest.java index 2ea198f6d8..621f4742d1 100644 --- a/javatests/com/google/gerrit/server/query/change/LuceneQueryChangesTest.java +++ b/javatests/com/google/gerrit/server/query/change/LuceneQueryChangesTest.java @@ -14,6 +14,9 @@ package com.google.gerrit.server.query.change; +import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; + import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.server.index.change.ChangeSchemaDefinitions; @@ -76,8 +79,10 @@ public class LuceneQueryChangesTest extends AbstractQueryChangesTest { Change change1 = insert(repo, newChange(repo), userId); String nameEmail = user.asIdentifiedUser().getNameEmail(); - exception.expect(BadRequestException.class); - exception.expectMessage("Cannot create full-text query with value: \\"); - assertQuery("owner: \"" + nameEmail + "\"\\", change1); + BadRequestException thrown = + assertThrows( + BadRequestException.class, + () -> assertQuery("owner: \"" + nameEmail + "\"\\", change1)); + assertThat(thrown).hasMessageThat().contains("Cannot create full-text query with value: \\"); } } diff --git a/javatests/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java b/javatests/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java index 44ea675670..3b1304110f 100644 --- a/javatests/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java +++ b/javatests/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java @@ -18,6 +18,7 @@ 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.common.truth.TruthJUnit.assume; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static java.util.stream.Collectors.toList; import com.google.common.base.CharMatcher; @@ -209,9 +210,9 @@ public abstract class AbstractQueryGroupsTest extends GerritServerTests { assertQuery("description:non-existing"); - exception.expect(BadRequestException.class); - exception.expectMessage("description operator requires a value"); - assertQuery("description:\"\""); + BadRequestException thrown = + assertThrows(BadRequestException.class, () -> assertQuery("description:\"\"")); + assertThat(thrown).hasMessageThat().contains("description operator requires a value"); } @Test diff --git a/javatests/com/google/gerrit/server/query/project/AbstractQueryProjectsTest.java b/javatests/com/google/gerrit/server/query/project/AbstractQueryProjectsTest.java index 1b1e49b2ca..8f13099502 100644 --- a/javatests/com/google/gerrit/server/query/project/AbstractQueryProjectsTest.java +++ b/javatests/com/google/gerrit/server/query/project/AbstractQueryProjectsTest.java @@ -14,8 +14,10 @@ package com.google.gerrit.server.query.project; +import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; import static com.google.common.truth.TruthJUnit.assume; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static java.util.stream.Collectors.toList; import com.google.common.base.CharMatcher; @@ -211,9 +213,9 @@ public abstract class AbstractQueryProjectsTest extends GerritServerTests { assertQuery("description:non-existing"); - exception.expect(BadRequestException.class); - exception.expectMessage("description operator requires a value"); - assertQuery("description:\"\""); + BadRequestException thrown = + assertThrows(BadRequestException.class, () -> assertQuery("description:\"\"")); + assertThat(thrown).hasMessageThat().contains("description operator requires a value"); } @Test @@ -228,16 +230,18 @@ public abstract class AbstractQueryProjectsTest extends GerritServerTests { @Test public void byState_emptyQuery() throws Exception { - exception.expect(BadRequestException.class); - exception.expectMessage("state operator requires a value"); - assertQuery("state:\"\""); + BadRequestException thrown = + assertThrows(BadRequestException.class, () -> assertQuery("state:\"\"")); + assertThat(thrown).hasMessageThat().contains("state operator requires a value"); } @Test public void byState_badQuery() throws Exception { - exception.expect(BadRequestException.class); - exception.expectMessage("state operator must be either 'active' or 'read-only'"); - assertQuery("state:bla"); + BadRequestException thrown = + assertThrows(BadRequestException.class, () -> assertQuery("state:bla")); + assertThat(thrown) + .hasMessageThat() + .contains("state operator must be either 'active' or 'read-only'"); } @Test diff --git a/javatests/com/google/gerrit/server/rules/GerritCommonTest.java b/javatests/com/google/gerrit/server/rules/GerritCommonTest.java index 180c16b26d..655baa06b8 100644 --- a/javatests/com/google/gerrit/server/rules/GerritCommonTest.java +++ b/javatests/com/google/gerrit/server/rules/GerritCommonTest.java @@ -14,6 +14,8 @@ package com.google.gerrit.server.rules; +import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static org.easymock.EasyMock.expect; import com.google.gerrit.common.data.LabelTypes; @@ -82,11 +84,14 @@ public class GerritCommonTest extends PrologTestCase { throw new CompileException("Cannot consult " + nameTerm); } - exception.expect(ReductionLimitException.class); - exception.expectMessage("exceeded reduction limit of 1300"); - env.once( - Prolog.BUILTIN, - "call", - new StructureTerm(":", SymbolTerm.create("user"), SymbolTerm.create("loopy"))); + ReductionLimitException thrown = + assertThrows( + ReductionLimitException.class, + () -> + env.once( + Prolog.BUILTIN, + "call", + new StructureTerm(":", SymbolTerm.create("user"), SymbolTerm.create("loopy")))); + assertThat(thrown).hasMessageThat().contains("exceeded reduction limit of 1300"); } } diff --git a/javatests/com/google/gerrit/server/util/SocketUtilTest.java b/javatests/com/google/gerrit/server/util/SocketUtilTest.java index 018b8dbee6..3cc340f03b 100644 --- a/javatests/com/google/gerrit/server/util/SocketUtilTest.java +++ b/javatests/com/google/gerrit/server/util/SocketUtilTest.java @@ -14,10 +14,12 @@ package com.google.gerrit.server.util; +import static com.google.common.truth.Truth.assertThat; import static com.google.gerrit.server.util.SocketUtil.hostname; import static com.google.gerrit.server.util.SocketUtil.isIPv6; import static com.google.gerrit.server.util.SocketUtil.parse; import static com.google.gerrit.server.util.SocketUtil.resolve; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static java.net.InetAddress.getByName; import static java.net.InetSocketAddress.createUnresolved; import static org.junit.Assert.assertEquals; @@ -105,16 +107,16 @@ public class SocketUtilTest extends GerritBaseTests { @Test public void testParseInvalidIPv6() { - exception.expect(IllegalArgumentException.class); - exception.expectMessage("invalid IPv6: [:3"); - parse("[:3", 80); + IllegalArgumentException thrown = + assertThrows(IllegalArgumentException.class, () -> parse("[:3", 80)); + assertThat(thrown).hasMessageThat().contains("invalid IPv6: [:3"); } @Test public void testParseInvalidPort() { - exception.expect(IllegalArgumentException.class); - exception.expectMessage("invalid port: localhost:A"); - parse("localhost:A", 80); + IllegalArgumentException thrown = + assertThrows(IllegalArgumentException.class, () -> parse("localhost:A", 80)); + assertThat(thrown).hasMessageThat().contains("invalid port: localhost:A"); } @Test diff --git a/javatests/com/google/gerrit/testing/IndexVersionsTest.java b/javatests/com/google/gerrit/testing/IndexVersionsTest.java index 36247f8d6a..04cdfcaa0c 100644 --- a/javatests/com/google/gerrit/testing/IndexVersionsTest.java +++ b/javatests/com/google/gerrit/testing/IndexVersionsTest.java @@ -16,6 +16,7 @@ package com.google.gerrit.testing; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.TruthJUnit.assume; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static com.google.gerrit.testing.IndexVersions.ALL; import static com.google.gerrit.testing.IndexVersions.CURRENT; import static com.google.gerrit.testing.IndexVersions.PREVIOUS; @@ -133,8 +134,8 @@ public class IndexVersionsTest extends GerritBaseTests { } private void assertIllegalArgument(String value, String expectedMessage) { - exception.expect(IllegalArgumentException.class); - exception.expectMessage(expectedMessage); - get(value); + IllegalArgumentException thrown = + assertThrows(IllegalArgumentException.class, () -> get(value)); + assertThat(thrown).hasMessageThat().contains(expectedMessage); } }