8000 Refactor builders in testsuite part 2 by stianst · Pull Request #48454 · keycloak/keycloak · GitHub
[go: up one dir, main page]

Skip to content

Refactor builders in testsuite part 2 - #48454

Merged
stianst merged 1 commit into
keycloak:mainfrom
stianst:tf-realm-cleanup-refactor-2
Apr 28, 2026
Merged

stianst merged 1 commit into
keycloak:mainfrom
stianst:tf-realm-cleanup-refactor-2

Conversation

@stianst
@stianst stianst commented Apr 24, 2026
Copy link
Copy Markdown
Contributor

Closes #48452

Signed-off-by: stianst stianst@gmail.com

@stianst
stianst force-pushed the tf-realm-cleanup-refactor-2 branch 3 times, most recently from 1d98d90 to 802958a Compare April 27, 2026 05:51
@stianst
stianst force-pushed the tf-realm-cleanup-refactor-2 branch 3 times, most recently from 291847d to 8426330 Compare April 27, 2026 08:26
@stianst
stianst force-pushed the tf-realm-cleanup-refactor-2 branch 3 times, most recently from 9ecc43b to 55e4634 Compare April 27, 2026 09:10
@stianst
stianst force-pushed the tf-realm-cleanup-refactor-2 branch 4 times, most recently from 7f23cae to 8c75187 Compare April 27, 2026 12:27
@lhanusov lhanusov left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stianst leaving few comments, just minor changes, thanks for this PR, it will bring more consistency inside tests.


import static org.keycloak.models.jpa.entities.RealmAttributes.ACTION_TOKEN_GENERATED_BY_USER_LIFESPAN;

public class RealmAttributesBuilder {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you plan to have it as an extension to Builder class?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, no need for that

public RealmBuilder client(ClientRepresentation client) {
rep.setClients(Collections.combine(rep.getClients(), client));
@Deprecated
public RealmBuilder publicKey(String publicKey) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it for backward compatibility?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

rep.setClients(Collections.combine(rep.getClients(), client));
return ClientBuilder.update(client).enabled(true).clientId(clientId);
@Deprecated
public RealmBuilder privateKey(String privateKey) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it for backward compatibility?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

@@ -75,7 +74,7 @@ public void beforeBrokerTest() {
// refresh flow to find its id
clientFlow = findFlowByAlias(clientFlow.getAlias(), realmResource.flows().getFlows());

AuthenticationExecutionRepresentation execution = ExecutionBuilder.create()
AuthenticationExecutionRepresentation execution = org.keycloak.testframework.realm.AuthenticationExecutionBuilder.create()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

plus add import

Suggested change
AuthenticationExecutionRepresentation execution = org.keycloak.testframework.realm.AuthenticationExecutionBuilder.create()
AuthenticationExecutionRepresentation execution = AuthenticationExecutionBuilder.create()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updating

@@ -63,7 +62,7 @@ public void beforeBrokerTest() {
}

private void addExecution(AuthenticationManagementResource authMgmtResource, String flowId, String providerId, int priority) {
AuthenticationExecutionRepresentation exec = ExecutionBuilder.create()
AuthenticationExecutionRepresentation exec = org.keycloak.testframework.realm.AuthenticationExecutionBuilder.create()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

plus add import

Suggested change
AuthenticationExecutionRepresentation exec = org.keycloak.testframework.realm.AuthenticationExecutionBuilder.create()
AuthenticationExecutionRepresentation exec = AuthenticationExecutionBuilder.create()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updating

.id("username password form")
.parentFlow(this.flow.getId())
.requirement(AuthenticationExecutionModel.Requirement.REQUIRED.name())
.authenticator(UsernamePasswordFormFactory.PROVIDER_ID)
.build();

AuthenticationExecutionRepresentation authScriptExecution = ExecutionBuilder.create()
AuthenticationExecutionRepresentation authScriptExecution = org.keycloak.testframework.realm.AuthenticationExecutionBuilder.create()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
AuthenticationExecutionRepresentation authScriptExecution = org.keycloak.testframework.realm.AuthenticationExecutionBuilder.create()
AuthenticationExecutionRepresentation authScriptExecution = AuthenticationExecutionBuilder.create()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updating

@@ -185,7 +184,7 @@ public void loginShouldFailWithScriptAuthenticator() {
*/
@Test
public void scriptWithClientSession() {
AuthenticationExecutionRepresentation authScriptExecution = ExecutionBuilder.create()
AuthenticationExecutionRepresentation authScriptExecution = org.keycloak.testframework.realm.AuthenticationExecutionBuilder.create()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
AuthenticationExecutionRepresentation authScriptExecution = org.keycloak.testframework.realm.AuthenticationExecutionBuilder.create()
AuthenticationExecutionRepresentation authScriptExecution = AuthenticationExecutionBuilder.create()

Copy link
Copy Markdown
Contributor Author F438

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updating

@@ -152,14 +151,14 @@ public void configureFlows() throws Exception {

this.flow = findFlowByAlias(scriptFlow);

AuthenticationExecutionRepresentation usernamePasswordFormExecution = ExecutionBuilder.create()
AuthenticationExecutionRepresentation usernamePasswordFormExecution = org.keycloak.testframework.realm.AuthenticationExecutionBuilder.create()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

plus add import

Suggested change
AuthenticationExecutionRepresentation usernamePasswordFormExecution = org.keycloak.testframework.realm.AuthenticationExecutionBuilder.create()
AuthenticationExecutionRepresentation usernamePasswordFormExecution = AuthenticationExecutionBuilder.create()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updating

.id("username password form")
.parentFlow(this.flow.getId())
.requirement(AuthenticationExecutionModel.Requirement.REQUIRED.name())
.authenticator(UsernamePasswordFormFactory.PROVIDER_ID)
.build();

AuthenticationExecutionRepresentation authScriptExecution = ExecutionBuilder.create()
AuthenticationExecutionRepresentation authScriptExecution = org.keycloak.testframework.realm.AuthenticationExecutionBuilder.create()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
AuthenticationExecutionRepresentation authScriptExecution = org.keycloak.testframework.realm.AuthenticationExecutionBuilder.create()
AuthenticationExecutionRepresentation authScriptExecution = AuthenticationExecutionBuilder.create()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

< 10BC8 task-lists disabled sortable>

Updating

@@ -35,38 +35,25 @@
<dependency>
<groupId>org.keycloak</groupId>
<artifactId>keycloak-core</artifactId>
<exclusions>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removing this exclusion -> wouldn't it bring multiple unused dependencies inside the test framework?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't matter as the framework as a dep on this regardless due to the embedded server

@stianst
stianst marked this pull request as ready for review April 27, 2026 13:09
@stianst
stianst requested review from a team as code owners April 27, 2026 13:09
@keycloak-github-bot keycloak-github-bot Bot left a comment
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unreported flaky test detected, please review

@keycloak-github-bot
Copy link
Copy Markdown

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.model.user.UserModelTest#testAddDirtyRemoveFederationUsersInTheSameGroupConcurrent

Keycloak CI - Store Model Tests

org.opentest4j.MultipleFailuresError: 
Multiple Failures (2 failures)
	java.lang.NullPointerException: Cannot invoke "org.keycloak.models.KeycloakSessionFactory.create()" because "factory" is null
	org.infinispan.commons.IllegalLifecycleStateException: ISPN000323: realms is in 'TERMINATED' state and so it does not accept new invocations. Either restart it or recreate the cache container.
	Suppressed: java.lang.NullPointerException: Cannot invoke "org.keycloak.models.KeycloakSessionFactory.create()" because "factory" is null
...

Report flaky test

Closes keycloak#48452

Signed-off-by: stianst <stianst@gmail.com>
@stianst
stianst force-pushed the tf-realm-cleanup-refactor-2 branch from 8c75187 to a140e2b Compare April 27, 2026 13:48
@stianst
stianst requested a review from lhanusov April 27, 2026 13:48
@lhanusov lhanusov left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@stianst
stianst merged commit e1adb5f into keycloak:main Apr 28, 2026
95 of 97 checks passed
@stianst
stianst deleted the tf-realm-cleanup-refactor-2 branch April 28, 2026 08:55
@stianst stianst mentioned this pull request Jun 1, 2026
sashyo pushed a commit to tide-foundation/keycloak-IGA that referenced this pull request Jul 20, 2026
Closes keycloak#48452

Signed-off-by: stianst <stianst@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor builders in testsuite part 2

3 participants

0