Migrate all tests off Hamcrest to AssertJ and remove the dependency#3975
Merged
Conversation
Replace the ProxyingBeanInfoMatcher/UaaClientDetailsMatcher dynamic proxy matchers and JsonMatcher with direct AssertJ assertions and a Jackson-tree comparison helper in JsonTranslation. Remove the module's hamcrest dependency. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JTS9GWnfqfT56JVPQw4b9c
Convert all remaining org.hamcrest usage (~66 files) to AssertJ or native MockMvc matchers, per the AssertJ-only testing rule: - Delete the custom hamcrest matcher infrastructure: ProxyingBeanInfoMatcher, UaaClientDetailsMatcher, UaaUserMatcher, PredicateMatcher, JsonObjectMatcherUtils, and the OAuth2 token matchers package. - Replace the token matchers with TokenClaims, a plain static helper that keeps the same claim extraction, opaque-token resolution, and signature verification, consumed via direct assertThat chains. - JSON body comparisons now use Jackson tree equality; regex assertions use AssertJ matches/containsPattern instead of a custom TypeSafeMatcher; jsonPath/content/header matcher forms use native variants or andReturn() + AssertJ. - Remove the hamcrest dependency declaration from all six modules and the version catalog alias. Verified: full unit suite matches baseline (13 pre-existing JDK-26 SAML failures, no new failures) across all modules. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JTS9GWnfqfT56JVPQw4b9c
Contributor
There was a problem hiding this comment.
Pull request overview
Migrates remaining Java test assertions away from Hamcrest matchers to AssertJ and native MockMvc matchers, and removes the direct org.hamcrest:hamcrest dependency from all modules (while acknowledging it may still appear transitively via Spring test tooling).
Changes:
- Replaced Hamcrest matcher-based assertions across tests with AssertJ chains and MockMvc matcher overloads.
- Removed Hamcrest-based custom matcher infrastructure and introduced
TokenClaimsas a value-oriented JWT-claims test helper. - Dropped the
libs.hamcrestversion-catalog alias and removedtestImplementation(libs.hamcrest)from all affected modules.
Reviewed changes
Copilot reviewed 81 out of 81 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| uaa/src/test/java/org/cloudfoundry/identity/uaa/scim/endpoints/ScimUserEndpointsMockMvcZonePathTests.java | Replace Hamcrest/JsonObjectMatcherUtils assertions with AssertJ + Jackson tree equality / jsonPath .value(...). |
| uaa/src/test/java/org/cloudfoundry/identity/uaa/scim/endpoints/ScimUserEndpointsMockMvcTests.java | Same migration as zone-path variant. |
| uaa/src/test/java/org/cloudfoundry/identity/uaa/scim/endpoints/PasswordResetEndpointMockMvcTests.java | Replace content().string(containsString(...)) with MvcResult + AssertJ string contains. |
| uaa/src/test/java/org/cloudfoundry/identity/uaa/oauth/UaaTokenServicesTests.java | Replace Hamcrest-based issuer matcher with TokenClaims value assertion. |
| uaa/src/test/java/org/cloudfoundry/identity/uaa/oauth/UaaAuthorizationEndpointMockMvcZonePathTest.java | Replace Hamcrest header/model matchers with MvcResult + AssertJ assertions. |
| uaa/src/test/java/org/cloudfoundry/identity/uaa/oauth/UaaAuthorizationEndpointMockMvcTest.java | Same migration as zone-path variant. |
| uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/zones/IdentityZoneEndpointsMockMvcZonePathTests.java | Replace containsString matchers with AssertJ response body assertions. |
| uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/zones/IdentityZoneEndpointsMockMvcTests.java | Same migration as zone-path variant. |
| uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/util/MockMvcUtils.java | Replace Hamcrest not(500) status matcher with explicit status assertion. |
| uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/token/UserTokenMockMvcTests.java | Replace content().string(containsString(...)) with MvcResult + AssertJ. |
| uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/token/TokenRevocationEndpointMockMvcZonePathTest.java | Replace content().string(containsString(...)) with MvcResult + AssertJ. |
| uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/token/TokenRevocationEndpointMockMvcTest.java | Same migration as zone-path variant. |
| uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/token/TokenMvcMockZonePathTests.java | Replace Hamcrest header/content negation and substring matchers with AssertJ assertions. |
| uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/token/TokenMvcMockTests.java | Same migration as zone-path variant. |
| uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/token/TokenKeyEndpointMockMvcZonePathTests.java | Replace Hamcrest any(String.class) with header().exists(...). |
| uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/token/TokenKeyEndpointMockMvcTests.java | Same migration as zone-path variant. |
| uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/saml/SamlAuthenticationMockMvcTests.java | Replace HTML substring Hamcrest matcher usage with AssertJ. |
| uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/limited/LimitedModeTokenMockMvcTests.java | Replace containsInAnyOrder matcher with JsonPath read + AssertJ containsExactlyInAnyOrder. |
| uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/ldap/LdapMockMvcTests.java | Replace response substring Hamcrest assertions with AssertJ. |
| uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/ldap/AbstractLdapMockMvcTest.java | Replace multiple content().string(containsString(...)) checks with consolidated AssertJ assertions. |
| uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/clients/ClientAdminEndpointsMockMvcZonePathTests.java | Replace containsString matcher with AssertJ response contains. |
| uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/audit/AuditCheckMockMvcTests.java | Replace multiple Hamcrest substring checks with AssertJ response contains. |
| uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/approvals/ApprovalsMockMvcTests.java | Replace content().string(containsString(...)) with MvcResult + AssertJ. |
| uaa/src/test/java/org/cloudfoundry/identity/uaa/login/ZonePathSessionMockMvcTests.java | Replace content substring Hamcrest matchers with MvcResult + AssertJ. |
| uaa/src/test/java/org/cloudfoundry/identity/uaa/login/TokenEndpointDocs.java | Replace Hamcrest substring matcher assertions with AssertJ for docs test. |
| uaa/src/test/java/org/cloudfoundry/identity/uaa/login/ResetPasswordControllerMockMvcZonePathTests.java | Replace Hamcrest matchers for request attributes / response HTML with AssertJ and MockMvc overloads. |
| uaa/src/test/java/org/cloudfoundry/identity/uaa/login/ResetPasswordControllerMockMvcTests.java | Same migration as zone-path variant. |
| uaa/src/test/java/org/cloudfoundry/identity/uaa/login/InvitationsServiceMockMvcZonePathTests.java | Replace Hamcrest content assertions with AssertJ. |
| uaa/src/test/java/org/cloudfoundry/identity/uaa/login/InvitationsServiceMockMvcTests.java | Same migration as zone-path variant. |
| uaa/src/test/java/org/cloudfoundry/identity/uaa/login/ForcePasswordChangeControllerZonePathMockMvcTest.java | Replace Hamcrest content substring checks with AssertJ. |
| uaa/src/test/java/org/cloudfoundry/identity/uaa/login/AccountsControllerMockMvcZonePathTests.java | Replace Hamcrest content assertions and negations with AssertJ. |
| uaa/src/test/java/org/cloudfoundry/identity/uaa/login/AccountsControllerMockMvcTests.java | Same migration as zone-path variant. |
| uaa/src/test/java/org/cloudfoundry/identity/uaa/invitations/InvitationsEndpointMockMvcZonePathTests.java | Replace Hamcrest HTML content checks with AssertJ, including negative assertions. |
| uaa/src/test/java/org/cloudfoundry/identity/uaa/invitations/InvitationsEndpointMockMvcTests.java | Same migration as zone-path variant. |
| uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/util/IntegrationTestUtils.java | Remove Hamcrest-based RegexMatcher helper. |
| uaa/build.gradle.kts | Remove direct Hamcrest test dependency. |
| statsd/build.gradle.kts | Remove direct Hamcrest test dependency. |
| statsd-lib/build.gradle.kts | Remove direct Hamcrest test dependency. |
| server/src/test/java/org/cloudfoundry/identity/uaa/util/PredicateMatcher.java | Delete Hamcrest-based predicate matcher helper. |
| server/src/test/java/org/cloudfoundry/identity/uaa/util/JwtTokenSignedByThisUAATest.java | Replace MockitoHamcrest usage with Mockito argThat + AssertJ-based ArgumentMatcher. |
| server/src/test/java/org/cloudfoundry/identity/uaa/user/UaaUserMatcher.java | Delete Hamcrest proxy matcher interface. |
| server/src/test/java/org/cloudfoundry/identity/uaa/scim/test/JsonObjectMatcherUtils.java | Delete Hamcrest JSON-object matcher utility. |
| server/src/test/java/org/cloudfoundry/identity/uaa/ProxyingBeanInfoMatcher.java | Delete Hamcrest proxy-bean matcher implementation. |
| server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManagerIT.java | Replace Hamcrest request-body matchers with a custom RequestMatcher using AssertJ. |
| server/src/test/java/org/cloudfoundry/identity/uaa/oauth/TokenTestSupport.java | Switch revocable-token threadlocal wiring from removed matcher hierarchy to TokenClaims. |
| server/src/test/java/org/cloudfoundry/identity/uaa/oauth/token/TokenClaims.java | Add new AssertJ-friendly JWT claims helper to replace Hamcrest token matchers. |
| server/src/test/java/org/cloudfoundry/identity/uaa/oauth/token/matchers/OAuth2RefreshTokenMatchers.java | Remove Hamcrest token matcher class. |
| server/src/test/java/org/cloudfoundry/identity/uaa/oauth/token/matchers/OAuth2AccessTokenMatchers.java | Remove Hamcrest token matcher class. |
| server/src/test/java/org/cloudfoundry/identity/uaa/oauth/token/matchers/AbstractOAuth2AccessTokenMatchers.java | Remove Hamcrest token matcher base class and its threadlocal. |
| server/src/test/java/org/cloudfoundry/identity/uaa/oauth/RefreshRotationTest.java | Update threadlocal cleanup to use TokenClaims. |
| server/src/test/java/org/cloudfoundry/identity/uaa/login/SessionControllerViewZonePathTests.java | Replace content matchers with MvcResult + AssertJ. |
| server/src/test/java/org/cloudfoundry/identity/uaa/login/ResetPasswordControllerViewZonePathTests.java | Replace content matchers with MvcResult + AssertJ. |
| server/src/test/java/org/cloudfoundry/identity/uaa/login/ResetPasswordControllerTest.java | Replace content matchers with MvcResult + AssertJ. |
| server/src/test/java/org/cloudfoundry/identity/uaa/login/ProfileControllerMockMvcZonePathTests.java | Replace Hamcrest model/content matchers with explicit model extraction + AssertJ. |
| server/src/test/java/org/cloudfoundry/identity/uaa/login/ProfileControllerMockMvcTests.java | Same migration as zone-path variant. |
| server/src/test/java/org/cloudfoundry/identity/uaa/login/HomeControllerViewZonePathTests.java | Replace content matchers with MvcResult + AssertJ. |
| server/src/test/java/org/cloudfoundry/identity/uaa/login/HomeControllerViewTests.java | Same migration as zone-path variant. |
| server/src/test/java/org/cloudfoundry/identity/uaa/invitations/InvitationsControllerZonePathTest.java | Replace content matchers with MvcResult + AssertJ and simplify flow. |
| server/src/test/java/org/cloudfoundry/identity/uaa/invitations/InvitationsControllerTest.java | Same migration as zone-path variant. |
| server/src/test/java/org/cloudfoundry/identity/uaa/client/UaaClientDetailsMatcher.java | Delete Hamcrest proxy matcher interface. |
| server/src/test/java/org/cloudfoundry/identity/uaa/authentication/manager/LoginAuthenticationManagerTests.java | Replace Hamcrest proxy matcher usage with explicit field assertions. |
| server/src/test/java/org/cloudfoundry/identity/uaa/account/PasswordResetEndpointTest.java | Replace Hamcrest content/JSON matcher assertions with AssertJ and Jackson tree equality. |
| server/src/test/java/org/cloudfoundry/identity/uaa/account/ChangePasswordControllerViewZonePathTests.java | Replace content matchers with MvcResult + AssertJ. |
| server/src/test/java/org/cloudfoundry/identity/uaa/account/ChangeEmailControllerViewZonePathTests.java | Replace content matchers with MvcResult + AssertJ. |
| server/src/test/java/org/cloudfoundry/identity/uaa/account/AccountsControllerViewZonePathTests.java | Replace content matchers with MvcResult + AssertJ. |
| server/build.gradle.kts | Remove direct Hamcrest test dependency. |
| model/src/test/java/org/cloudfoundry/identity/uaa/test/JsonTranslation.java | Replace Hamcrest JSON matcher with structural Jackson tree equality helper. |
| model/src/test/java/org/cloudfoundry/identity/uaa/test/JsonMatcher.java | Delete Hamcrest JSON matcher utility. |
| model/src/test/java/org/cloudfoundry/identity/uaa/ProxyingBeanInfoMatcher.java | Delete Hamcrest proxy-bean matcher implementation. |
| model/src/test/java/org/cloudfoundry/identity/uaa/client/UaaClientDetailsTest.java | Replace Hamcrest proxy matcher assertions with explicit AssertJ assertions. |
| model/src/test/java/org/cloudfoundry/identity/uaa/client/UaaClientDetailsMatcher.java | Delete Hamcrest proxy matcher interface. |
| model/build.gradle.kts | Remove direct Hamcrest test dependency. |
| metrics-data/build.gradle.kts | Remove direct Hamcrest test dependency. |
| gradle/libs.versions.toml | Remove Hamcrest library alias from the version catalog. |
Comment on lines
+104
to
108
| /** Compares two JSON documents structurally (field order and formatting insensitive). */ | ||
| private void assertJsonEquals(String actualJson, String expectedJson) { | ||
| assertThat(objectMapper.readTree(actualJson)) | ||
| .isEqualTo(objectMapper.readTree(expectedJson)); | ||
| } |
Post-migration review found the SAML metadata suites' xpath-scoped containsString assertions (Signature/SignedInfo algorithm attributes and AssertionConsumerService/@location) had been flattened to whole-response- body contains(), which would let a signature-algorithm downgrade or a wrong ACS Location pass as long as the expected substring appeared anywhere in the XML. Re-scope them via an XpathExpectationsHelper-based evaluateXpath helper + AssertJ contains, matching original semantics. Verified: all 29 tests in both classes pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JTS9GWnfqfT56JVPQw4b9c
Comment on lines
+436
to
+448
| // Matches when the actual collection has exactly the same elements as expected, ignoring order, | ||
| // for use with Mockito's argThat. | ||
| private static ArgumentMatcher<Collection<String>> containsInAnyOrder(Object... expected) { | ||
| List<String> expectedStrings = Arrays.stream(expected).map(String::valueOf).toList(); | ||
| return actual -> { | ||
| try { | ||
| assertThat(actual).containsExactlyInAnyOrderElementsOf(expectedStrings); | ||
| return true; | ||
| } catch (AssertionError e) { | ||
| return false; | ||
| } | ||
| }; | ||
| } |
strehle
approved these changes
Jul 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Completes the AssertJ-only testing rule (
ai/rules/java-testing.mdc): migrates every remaining Hamcrest usage (~70 test files across model, server, uaa) to AssertJ or native MockMvc matchers, then removes theorg.hamcrest:hamcrestdeclaration from all six modules and the version-catalog alias. Hamcrest upstream has been effectively dormant since 2019.Conversions
jsonPath("$.x", is(v))→jsonPath("$.x").value(v);content().string(containsString(...))/header().string(name, matcher)/model().attribute(name, matcher)→ native overloads orandReturn()+ AssertJ on the responseassertThat(x).is(matching(matcher))bridges and plain matchers (hasEntry,hasKey,containsInAnyOrder,not,allOf,isEmptyOrNullString, …) → direct AssertJ chainsxpath(...).string(containsString(...))→XpathExpectationsHelper+ AssertJ, keeping the check scoped to the xpath nodeCustom matcher infrastructure deleted
ProxyingBeanInfoMatcher(model + server copies),UaaClientDetailsMatcher(both),UaaUserMatcher,PredicateMatcher,JsonMatcher,JsonObjectMatcherUtils, theRegexMatcherinner class ofIntegrationTestUtils, and theoauth/token/matcherspackage. The token matchers are replaced by a new plainTokenClaimstest helper preserving the same claim extraction, opaque-token resolution (ThreadLocalregistry ported verbatim), and signature verification — consumed as values viaassertThat(TokenClaims.audience(token))...instead of matcher composition.JSON body comparisons now use Jackson tree equality (order-insensitive, slightly stronger than the old top-level-only
JSONObjectcomparison). Two other deliberate strengthenings:not(emptyString())→isNotEmpty()(now also rejects null jti), andcookie().value(name, isEmptyOrNullString())gains an explicit existence check matching Spring's matcher-variant behavior.Note: the hamcrest jar still reaches the test classpath transitively via
spring-boot-starter-test(spring-test internals need it); this PR removes our direct dependency and all our usage of it.Test plan
docsTestRestDocs(excluded from the unit task; covers convertedTokenEndpointDocs): 114/114 passserver:integrationTestgreen