Skip to content

JUnit 5 test framework PoC#29517

Merged
stianst merged 1 commit into
keycloak:mainfrom
stianst:test-skeleton
May 27, 2024
Merged

JUnit 5 test framework PoC#29517
stianst merged 1 commit into
keycloak:mainfrom
stianst:test-skeleton

Conversation

@stianst

@stianst stianst commented May 14, 2024

Copy link
Copy Markdown
Contributor

Closes #29516

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

@stianst stianst requested review from a team as code owners May 14, 2024 13:30
stianst
stianst commented May 14, 2024
View reviewed changes
Comment thread ...src/main/java/org/keycloak/quarkus/runtime/integration/jaxrs/QuarkusKeycloakApplication.java Outdated

@stianst stianst May 14, 2024

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.

Changes here are a bit of a hack, but need a way to bootstrap the admin account that doesn't require setting environment variables.

@jonkoops jonkoops May 21, 2024

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 this something you want to improve now, or do a follow up for? If it's the latter then let's log an issue so it is not forgotten.

@stianst stianst May 22, 2024

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.

@vmuzikar @shawkins could you comment on this? Basically need a way to create the initial admin account using system properties as we are starting the server in an embedded mode, and can't set environment variables

@vmuzikar vmuzikar May 22, 2024

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.

The change around the admin user looks good to me. It's temporary in any case since the admin bootstrapping will be redesigned as part of #9829. The sys prop is not documented so we can freely remove/change without notice as needed.

@jonkoops jonkoops May 22, 2024

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.

If there is a follow up for this I am fine with considering this resolved for the time being.

stianst
stianst commented May 14, 2024
View reviewed changes
Comment thread pom.xml Outdated
pedroigor
pedroigor previously approved these changes May 15, 2024
View reviewed changes

@pedroigor pedroigor 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.

As discussed, approving to make it possible to start collaborating on it and working on the design.

lhanusov
lhanusov approved these changes May 20, 2024
View reviewed changes

@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 - as it helps to move forward.

jonkoops
jonkoops reviewed May 21, 2024
View reviewed changes
Comment thread test-poc/base/pom.xml Outdated
Comment thread test-poc/base/pom.xml Outdated
Comment thread test-poc/pom.xml Outdated
Comment thread test-poc/base/src/test/java/org/keycloak/test/base/DefaultConfig1Test.java Outdated
Comment thread test-poc/base/src/test/java/org/keycloak/test/base/DefaultConfig2Test.java Outdated
Comment thread test-poc/framework/src/main/java/org/keycloak/test/framework/injection/InstanceWrapper.java Outdated
Comment thread ...poc/framework/src/main/java/org/keycloak/test/framework/server/RemoteKeycloakTestServer.java Outdated
Comment thread test-poc/framework/src/main/java/org/keycloak/test/framework/injection/Registry.java Outdated
Comment thread test-poc/framework/src/main/java/org/keycloak/test/framework/injection/Registry.java Outdated
Comment thread test-poc/framework/src/main/java/org/keycloak/test/framework/realm/ClientConfig.java Outdated
vmuzikar
vmuzikar previously approved these changes May 22, 2024
View reviewed changes

@vmuzikar vmuzikar 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.

Looks good from the QuarkusKeycloakApplication perspective.

jonkoops
jonkoops previously approved these changes May 22, 2024
View reviewed changes

@jonkoops jonkoops 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.

Nice, LGTM, let's get this merged in

@stianst
JUnit 5 test framework PoC
7eab5fa 
Closes keycloak#29516

Signed-off-by: stianst <stianst@gmail.com>
@stianst stianst dismissed stale reviews from jonkoops and vmuzikar via 7eab5fa May 22, 2024 10:40
@stianst stianst enabled auto-merge (squash) May 22, 2024 10:43
keycloak-github-bot[bot]
keycloak-github-bot Bot reviewed May 22, 2024
View reviewed changes

@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

keycloak-github-bot Bot commented May 22, 2024

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.session.UserSessionPersisterProviderTest#testMigrateSession

Keycloak CI - Store Model Tests

java.lang.AssertionError: expected:<3> but was:<2>
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.failNotEquals(Assert.java:835)
	at org.junit.Assert.assertEquals(Assert.java:647)
	at org.junit.Assert.assertEquals(Assert.java:633)
...

Report flaky test

@lhanusov lhanusov mentioned this pull request May 23, 2024
Closed
@stianst stianst disabled auto-merge May 27, 2024 12:59
@stianst stianst enabled auto-merge (squash) May 27, 2024 13:00
@stianst stianst merged commit 568a5cb into keycloak:main May 27, 2024
This was referenced May 28, 2024
Closed
Closed
Closed
Closed
@lhanusov lhanusov mentioned this pull request Jul 11, 2024
Closed
@stianst stianst deleted the test-skeleton branch July 17, 2024 09:00
@stianst stianst restored the test-skeleton branch July 17, 2024 09:00
@stianst stianst deleted the test-skeleton branch July 17, 2024 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pedroigor pedroigor pedroigor approved these changes

@vmuzikar vmuzikar vmuzikar left review comments

+4 more reviewers

@keycloak-github-bot keycloak-github-bot[bot] keycloak-github-bot[bot] left review comments

@jonkoops jonkoops jonkoops approved these changes

@miquelsi miquelsi miquelsi approved these changes

@lhanusov lhanusov lhanusov approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

flaky-test team/cloud-native

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

New testsuite framework PoC

6 participants

@stianst @jonkoops @pedroigor @miquelsi @vmuzikar @lhanusov