Repository navigation
Migrate organization member package to the new testsuite - #50672
Merged
Merged
Conversation
vramik
force-pushed
the
48184-migrate-oeg-member-tests
branch
from
July 7, 2026 19:32
af8ea9a to
0e6b945
Compare
vramik
marked this pull request as ready for review
July 8, 2026 07:24
Contributor
There was a problem hiding this comment.
Pull request overview
This PR migrates Organization Member-related tests from the legacy testsuite layout to the newer testframework-based tests in tests/base, aligning organization member coverage with the new integration test approach.
Changes:
- Migrates
OrganizationMemberTestinto the new test framework (injection-based realms/users, UI login flow helpers, run-on-server usage). - Consolidates/shared org-broker setup logic into
AbstractOrganizationTestand updates dependent tests accordingly. - Adds small test-framework utilities needed by migrated tests (
ManagedUser.getEmail(),LoginPage.isPasswordInputPresent()), and adjusts package placement for an LDAP federation test.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/federation/OrganizationMemberWithLdapTest.java | Fixes package to match its directory/package structure under organization.federation. |
| tests/base/src/test/java/org/keycloak/tests/organization/member/OrganizationMemberTest.java | Migrated organization member test suite to the new testframework with UI-driven broker login helpers and s
10000
erver-side execution. |
| tests/base/src/test/java/org/keycloak/tests/organization/member/OrganizationMemberIdpLinkTest.java | Moves test into correct package and reuses shared broker setup/constants from AbstractOrganizationTest. |
| tests/base/src/test/java/org/keycloak/tests/organization/admin/OrganizationTest.java | Updates usage to new createOrganization(ManagedRealm, ...) and avoids the removed helper signature. |
| tests/base/src/test/java/org/keycloak/tests/organization/admin/OrganizationMemberTest.java | Removes old admin-package test now covered by the migrated suite. |
| tests/base/src/test/java/org/keycloak/tests/organization/admin/AbstractOrganizationTest.java | Refactors createOrganization to accept ManagedRealm and adds shared org-broker setup/constants. |
| test-framework/ui/src/main/java/org/keycloak/testframework/ui/page/LoginPage.java | Adds isPasswordInputPresent() utility for login-flow assertions. |
| test-framework/core/src/main/java/org/keycloak/testframework/realm/ManagedUser.java | Adds getEmail() to expose the created user’s email to tests. |
Comments suppressed due to low confidence (1)
tests/base/src/test/java/org/keycloak/tests/organization/admin/AbstractOrganizationTest.java:123
- Same issue as above: organization deletion cleanup is added to
realm.cleanup()even when the organization was created in themanagedRealmparameter. This can leave organizations behind in non-default realms. UsemanagedRealm.cleanup()here.
String orgId = id;
realm.cleanup().add(r -> {
try {
r.organizations().get(orgId).delete().close();
} catch (NotFoundException ignored) {}
});
vramik
force-pushed
the
48184-migrate-oeg-member-tests
branch
from
July 8, 2026 08:04
0e6b945 to
96e52e1
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
tests/base/src/test/java/org/keycloak/tests/organization/member/OrganizationMemberTest.java:203
- This test updates the realm user-profile configuration (unmanagedAttributePolicy) but never restores the original configuration. That can leak state across tests and cause ordering-dependent failures.
tests/base/src/test/java/org/keycloak/tests/organization/member/OrganizationMemberTest.java:404 - testDeleteUnmanagedMember() sets UnmanagedAttributePolicy to ENABLED but never applies it via userProfile().update(...), so the configuration change is ineffective. If this policy is required for the test scenario, it should be updated (and restored) to avoid leaking state.
vramik
force-pushed
the
48184-migrate-oeg-member-tests
branch
from
July 8, 2026 08:24
96e52e1 to
9b609ed
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
tests/base/src/test/java/org/keycloak/tests/organization/admin/AbstractOrganizationTest.java:123
- In createOrganization(ManagedRealm,...), the organization cleanup is registered on the field 'realm' instead of the 'managedRealm' parameter. When callers pass a different realm (e.g., secondRealm), the organization deletion will be registered on the wrong cleanup and can leak organizations across tests.
String orgId = id;
realm.cleanup().add(r -> {
try {
r.organizations().get(orgId).delete().close();
} catch (NotFoundException ignored) {}
});
tests/base/src/test/java/org/keycloak/tests/organization/member/OrganizationMemberTest.java:945
- loginViaBroker() submits the provider login (and optionally update-profile) and immediately searches the admin API for the federated user. Without waiting for the OAuth callback redirect, this can race and make the test flaky. Other tests in the new framework explicitly wait for the callback (e.g., tests/base/src/test/java/org/keycloak/tests/forms/SSOTest.java:121-123).
tests/base/src/test/java/org/keycloak/tests/organization/member/OrganizationMemberTest.java:967 - loginViaNonOrgIdP() returns immediately after submitting the provider login without waiting for the OAuth callback/redirect back to the consumer realm. This can race with the subsequent user search and lead to intermittent failures.
vramik
force-pushed
the
48184-migrate-oeg-member-tests
branch
from
July 10, 2026 08:43
9b609ed to
f06f097
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
tests/base/src/test/java/org/keycloak/tests/organization/member/OrganizationMemberTest.java:943
- This branch assumes the broker flow is displaying the update-profile page, but
AliceUserConfsupplies username, email, first name, and last name. The defaultIdpReviewProfileAuthenticatormode only shows that page when mandatory profile data is missing, so the first org-A login completes directly andloginUpdateProfilePage.update(...)operates on the wrong page. Configure this login to require profile review (or create the provider user with missing required data) before attempting to update the email.
Closes keycloak#48184 Signed-off-by: vramik <vramik@redhat.com>
vramik
force-pushed
the
48184-migrate-oeg-member-tests
branch
from
July 10, 2026 09:06
f06f097 to
e2e9967
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (4)
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/federation/OrganizationMemberWithLdapTest.java:18
- This change only moves the LDAP member test to another package in the legacy Arquillian suite; the class still uses the old
org.keycloak.testsuiteframework, JUnit 4, andLDAPRule. Issue #48184 requests migration of the tests from the oldorganization/memberdirectory to the new framework, so closing it with this test merely relocated leaves that requirement incomplete. Port this test to the new testsuite or document an explicit scope exception.
tests/base/src/test/java/org/keycloak/tests/organization/member/OrganizationMemberTest.java:202 - The inherited
realmhas class lifecycle, so this direct user-profile update survives the test. Unlike the temporary configuration intestUserProfileAttributePermissions, it is never restored, making later tests run with unmanaged attributes enabled and their results order-dependent. Register restoration with the managed-realm cleanup before updating it.
tests/base/src/test/java/org/keycloak/tests/organization/member/OrganizationMemberTest.java:881 - This updates the class-scoped realm's user-profile configuration without registering cleanup, so unmanaged attributes remain enabled for whichever tests execute afterward. Preserve and restore the original configuration through
realm.cleanup()to keep the migrated tests isolated.
tests/base/src/test/java/org/keycloak/tests/organization/member/OrganizationMemberTest.java:655 - The broker login creates a user in the class-scoped consumer realm, but this path does not register user cleanup as
loginViaBrokerdoes. The user therefore remains after the IdP and organization are removed and can interfere with later broker tests for the same provider account. Add the created user torealm.cleanup().
sguilhen
approved these changes
Jul 13, 2026
ahus1
approved these changes
Jul 14, 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.
Closes #48184