Move UserTest.java to the new testsuite #39805

lhanusov · 2025-05-19T11:03:08Z

Part of: #34494

mhajas · 2025-05-27T13:28:15Z

@lhanusov FYI the FIPS failures are unrelated to this PR. It happened in other runs as well. It seems it is now passing. I wanted to restart the tests but it seems there are other failures that may be related so I did not do it.

test-framework/core/src/main/java/org/keycloak/testframework/events/AdminEventAssertion.java

test-framework/core/src/main/java/org/keycloak/testframework/realm/UserConfigBuilder.java

...viders/src/main/java/org/keycloak/testframework/remote/providers/runonserver/RunHelpers.java

test-framework/ui/src/main/java/org/keycloak/testframework/ui/page/AbstractPage.java

test-framework/ui/src/main/java/org/keycloak/testframework/ui/page/ErrorPage.java

test-framework/ui/src/main/java/org/keycloak/testframework/ui/util/PageUtils.java

tests/utils-shared/pom.xml

tests/utils-shared/src/main/java/org/keycloak/testsuite/util/webdriver/PageUtils.java

tests/utils/src/main/java/org/keycloak/tests/utils/admin/ApiUtil.java

...tegration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/AbstractPage.java

lhanusov · 2025-06-06T13:08:10Z

@stianst I've changed the pages stuff and other things, but still there are some questionable, please take a look, thanks

vaceksimon · 2025-06-17T15:00:19Z

...viders/src/main/java/org/keycloak/testframework/remote/providers/runonserver/RunHelpers.java

+                    UserModel user = session.users().getUserByUsername(realm, username);
+                    List<CredentialModel> storedCredentialsByType = user.credentialManager().getStoredCredentialsByTypeStream(CredentialRepresentation.PASSWORD)
+                            .collect(Collectors.toList());
+                    System.out.println(storedCredentialsByType.size());


Is this print statement intentional?

@vaceksimon this was already there, I can remove it wdyt?

I would remove this, it seems to me that it was forgotten after a debugging session

removed, thanks

vaceksimon · 2025-06-19T11:44:02Z

test-framework/core/src/main/java/org/keycloak/testframework/events/AdminEventAssertion.java

+        } else if (expectedResourcePath.length == 1) {
+            Assertions.assertEquals(expectedResourcePath[0], event.getResourcePath());
+        } else {
+            Assertions.assertEquals(String.join("/", expectedResourcePath), event.getResourcePath());
+        }


if there is one parameter, doesn't String.join return just the same string?

String.join("/", "myString") // returns "myString"

Oh, I see. The issue is with the null parameters. According to Java, String.join("/", new String[] {"a", null, "a"}); is "a/null/a" 😄

What you can do is, instead of

AdminEventAssertion.assertEvent(adminEvents.poll(), OperationType.UPDATE, null, rep, ResourceType.REALM);

use

AdminEventAssertion.assertSuccess(adminEvents.poll()) .operationType(OperationType.UPDATE) .representation(rep) .resourceType(ResourceType.REALM);

to avoid the null value in the AbstractUserTest methods. That should solve the problem, and this method can stay unchanged.

thanks for the suggestion 👍

vaceksimon · 2025-06-19T14:23:47Z

tests/base/src/test/java/org/keycloak/tests/admin/user/UserCreateTest.java

+    }
+
+    @Test
+    public void createUserWithTempolaryCredentials() {


Suggested change

public void createUserWithTempolaryCredentials() {

public void createUserWithTemporaryCredentials() {

fixed, thanks

vaceksimon · 2025-06-19T14:50:02Z

tests/base/src/test/java/org/keycloak/tests/admin/user/UserCredentialTest.java

+
+    @Test
+    public void testGetAndMoveCredentials() {
+        UserResource user = ApiUtil.findUserByUsernameId(managedRealm.admin(), userOtp2.getUsername());


Maybe this would be faster?

Suggested change

UserResource user = ApiUtil.findUserByUsernameId(managedRealm.admin(), userOtp2.getUsername());

UserResource user = userOtp2.admin();

updated, thanks

vaceksimon · 2025-06-24T13:24:13Z

tests/base/src/test/java/org/keycloak/tests/admin/user/UserFedarationTest.java

+        addFederatedIdentity(id, "social-provider-id", link);
+
+        // Verify social link is here
+        user = managedRealm.admin().users().get(id);


Is it necessary to get the UserResource here again?

updated, thanks

vaceksimon · 2025-06-24T13:50:53Z

tests/base/src/test/java/org/keycloak/tests/admin/user/UserRequiredActionsTest.java

+        assertTrue(user.toRepresentation().getRequiredActions().isEmpty());
+
+        UserRepresentation userRep = user.toRepresentation();
+        userRep.getRequiredActions().add("UPDATE_PASSWORD");


It's strange that we use a hardcoded "UPDATE_PASSWORD" action here and also in other tests. Do we have a constant somewhere, or maybe use UserModel.RequiredAction.UPDATE_PASSWORD.toString() like in the last test method testDefaultRequiredActionAdded?

updated, thanks

vaceksimon · 2025-06-24T14:27:04Z

tests/base/src/test/java/org/keycloak/tests/admin/user/UserSearchTest.java

+        WebApplicationException ex = assertThrows(WebApplicationException.class, () -> createUser(userRep2));
+        assertThat(ex.getResponse().getStatusInfo().getStatusCode(), equalTo(400));
+        assertThat(ex.getResponse().readEntity(ErrorRepresentation.class).getErrorMessage(), equalTo("error-invalid-length"));


Instead of rewriting ApiUtil#getCreatedId to throw WebApplicationException, I'd change this and similar statements to this:

Suggested change

WebApplicationException ex = assertThrows(WebApplicationException.class, () -> createUser(userRep2));

assertThat(ex.getResponse().getStatusInfo().getStatusCode(), equalTo(400));

assertThat(ex.getResponse().readEntity(ErrorRepresentation.class).getErrorMessage(), equalTo("error-invalid-length"));

Response response = managedRealm.admin().users().create(userRep2);

assertThat(response.getStatus(), equalTo(400));

assertThat(response.readEntity(ErrorRepresentation.class).getErrorMessage(), equalTo("error-invalid-length"));

very nice proposal, thanks :-)

vaceksimon · 2025-06-24T14:39:41Z

tests/base/src/test/resources/org/keycloak/tests/admin/user/testrealm.json

Is this file used? I don't see it anywhere

removed, thanks

vaceksimon · 2025-06-24T14:47:32Z

tests/base/src/test/java/org/keycloak/tests/admin/user/UserUpdateTest.java

+import jakarta.ws.rs.BadRequestException;
+import jakarta.ws.rs.ClientErrorException;
+import jakarta.ws.rs.NotFoundException;
+import org.junit.Assert;


There is also a forgotten JUnit4 Assert import

updated, thanks

keycloak-github-bot

Unreported flaky test detected, please review

keycloak-github-bot · 2025-07-15T10:57:43Z

Unreported flaky test detected

If the flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR.

org.keycloak.testsuite.forms.BruteForceTest#testExceedMaxTemporaryLockouts

Keycloak CI - Base IT (5)

java.lang.AssertionError: 
type
Expected: is "LOGIN_ERROR"
     but: was "USER_DISABLED_BY_PERMANENT_LOCKOUT"
	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
...

Report flaky test

stianst · 2025-07-15T11:44:46Z

test-framework/core/src/main/java/org/keycloak/testframework/realm/UserConfigBuilder.java

+        if (rep.getCredentials() == null) {
+            rep.setCredentials(new LinkedList<>());
+        }
+
+        rep.getCredentials().add(credential);


You can use Collections.combine like how it's done for passwords:

keycloak/test-framework/core/src/main/java/org/keycloak/testframework/realm/UserConfigBuilder.java

Lines 60 to 63 in 5e805ac

public UserConfigBuilder password(String password) {

rep.setCredentials(Collections.combine(rep.getCredentials(), Representations.toCredential(CredentialRepresentation.PASSWORD, password)));

return this;

}

stianst · 2025-07-15T11:46:07Z

test-framework/core/src/main/java/org/keycloak/testframework/realm/UserConfigBuilder.java

    }

+    public UserConfigBuilder federatedLink(String identityProvider, String federatedUserId, String federatedUsername) {
+        if (rep.getFederatedIdentities() == null) {


Use Collections.combine instead like mentioned below for totpSecret

stianst · 2025-07-15T11:50:28Z

test-framework/ui/src/main/java/org/keycloak/testframework/ui/page/AbstractPage.java


+    public void assertCurrent() {
+        String name = getClass().getSimpleName();
+        Assertions.assertTrue(isActivePage(), "Expected " + name + " but was " + driver.getTitle() + " (" + driver.getCurrentUrl() + ")");


I would just use the data-page-id stuff in the message here as well. Perhaps:

Assertions.assertTrue(isActivePage(), "Expected " + getExpectedPageId() + " but was " + getCurrentPageId());

Or, maybe this is even better:

Assertions.assertEquals(getExpectedPageId(), getCurrentPageId(), "Not on expected page");

stianst · 2025-07-15T11:51:08Z

test-framework/ui/src/main/java/org/keycloak/testframework/ui/page/AbstractPage.java

        return body.getAttribute("data-page-id");
    }

+    public String getPageTitle() {


Looking at the usage of getPageTitle it's asserting that it's on a specific page, so perhaps just do loginPage.assertCurrent() instead in the test of asserting the page title?

stianst · 2025-07-15T11:53:42Z

test-framework/ui/src/main/java/org/keycloak/testframework/ui/page/InfoPage.java

+                : null;
+    }
+
+    public void clickToContinueDe() {


This is horrible; is there no id set on the link that we can use instead of looking up the link based on link-text?

:-D, I've just copy/pasted what was already there, I will check the id instead and come back with a new solution.

I've decided to remove those unused links. It will be added once those tests are migrated.

keycloak-github-bot · 2025-07-15T14:42:14Z

Unreported flaky test detected

If the flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR.

org.keycloak.testsuite.federation.ldap.LDAPReadOnlyTest#testReadOnlyUserGetsPermanentlyLocked

Keycloak CI - Base IT (5)

java.lang.AssertionError
	at org.junit.Assert.fail(Assert.java:87)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.junit.Assert.assertFalse(Assert.java:65)
	at org.junit.Assert.assertFalse(Assert.java:75)
...

Report flaky test

keycloak-github-bot

Unreported flaky test detected, please review

Part of: keycloak#34494 Signed-off-by: Lukas Hanusovsky <[email protected]>

lhanusov requested review from a team as code owners May 19, 2025 11:03

keycloak-github-bot bot added team/cloud-native team/core-clients team/core-iam labels May 19, 2025

lhanusov added the area/test-framework label May 19, 2025

lhanusov marked this pull request as draft May 19, 2025 11:03

lhanusov force-pushed the 34494/usertest branch 3 times, most recently from 3b55c86 to eb1e156 Compare May 27, 2025 12:13

lhanusov marked this pull request as ready for review May 27, 2025 12:14

lhanusov force-pushed the 34494/usertest branch 4 times, most recently from 65ab1ca to 822d4cf Compare June 5, 2025 06:48