Migrate url package to new test framework by michalvavrik · Pull Request #48212 · keycloak/keycloak

michalvavrik · 2026-04-17T17:06:42Z

Closes: Migrate url package to new test framework #47813
I kept the default hostname-strict as setting it makes no difference. It only enforces that a hostname must be provided — when one is set, it has no effect on URL resolution (the new framework uses start-dev which defaults to false)
"Server failed to start" scenarios are moved to Quarkus where we have suitable way to test server start failure
I didn't see a way to test "restart with a different options" so I simply used a separate test classes with following mapping:

Old `HostnameV2Test` method	New counterpart
`testFixedFrontendHostname`	`HostnameV2FixedHostnameTest.testMasterRealmFrontendAndBackendUrls` + `testMasterRealmAdminUrls`
`testFixedFrontendHostnameUrl`	`HostnameV2FixedUrlTest.testFrontendAndBackendUrls` + `testAdminUrls`
`testFixedFrontendAndAdminHostnameUrl`	`HostnameV2FrontendAndAdminTest.testFrontendAndBackendUrls` + `testAdminUrls`
`testFixedFrontendHostnameUrlWithDefaultPort`	`HostnameV2DefaultPortTest.testFrontendAndBackendUrls` + `testAdminUrls`
`testDynamicBackend`	`HostnameV2BackchannelDynamicTest.testFrontendAndBackendUrls` + `testAdminUrls`
`testDynamicEverything`	`HostnameV2NoHostnameConfiguredTest.testFrontendAndBackendUrls` + `testAdminUrls`
`testRealmFrontendUrlWithOtherUrlsSet`	`HostnameV2RealmFrontendUrlOverrideTest.testFrontendAndBackendUrls` + `testAdminUrls`
`testRealmFrontendUrl`	`HostnameV2FixedHostnameTest.testCustomRealmFrontendAndBackendUrls` + `testCustomRealmAdminUrls`
`testStrictMode`	`HostnameV2DistTest.testServerFailsToStartWithoutHostnameSpecified` (already existed)
`testStrictModeMustBeDisabledWhenHostnameIsSpecified` (commented out)	Not migrated — validation was relaxed to a log message in `HostnameV2ProviderFactory` (line 61)
`testInvalidHostnameUrl`	`HostnameV2DistTest.testInvalidHostnameUrl`
`testInvalidAdminUrl`	`HostnameV2DistTest.testInvalidAdminUrl`
`testBackchannelDynamicRequiresHostname`	`HostnameV2DistTest.testBackchannelDynamicRequiresHostname`
`testBackchannelDynamicRequiresFullHostnameUrl`	`HostnameV2DistTest.testBackchannelDynamicRequiresFullHostnameUrl`

shawkins · 2026-04-20T16:19:18Z

+
+    @Test
+    @Launch({"start", "--db=dev-file", "--http-enabled=true", "--hostname=htt://localtest.me"})
+    public void testInvalidHostnameUrl(LaunchResult result) {


@vmuzikar and others - just a general question on the role of the quarkus integration tests moving forward. Do we even want to keep most of what exists separate from the new test suite? I'd be fine with removing most of our quarkus integration tests in favor of Picocli test cases and the new test suite. Maybe start with converting enough such that we only need a single CI test run, instead of the two we currently have.

Potentially yes, but not sure it's worth the effort to rewrite it to the new testsuite. We don't see many specific downsides of the current Quarkus testsuite.

We don't see many specific downsides of the current Quarkus testsuite.

We've had to optimize things several times, and introduce a split into two test runs. If we keep adding to it, we'll need to add a third. Seems better to work towards shifting to unit tests and the general test suite so that we can be less on the hook for optimization / splitting test runs.

So rather than a question whether we should migrate to the new test framework, it's a question whether we should migrate some of the Quarkus integration tests to unit tests?

So rather than a question whether we should migrate to the new test framework, it's a question whether we should migrate some of the Quarkus integration tests to unit tests?

That's down to the individual test.

We absolutely need coverage of our scripts - but we have removed command / option specific logic (dealing with --optimized, help, etc.) from kc.sh and kc.bat, so I think we can streamline what we're covering. I'd at least say anything that is marked as DryRun which is not specifically for exercising a script should just be a unit test.

If a test is actually starting the server and is something that is expressable in the new test framework, I would be in favor of migrating it or identifying why it can't.

@shawkins Can you please create a discussion with the proposal for migrating the Quarkus tests? This is not the correct place to discuss it as it will probably require more thought.

It'd be good to have a summarization of the specific proposal and the expected benefits.

Moved to #48399

michalvavrik · 2026-04-22T10:10:21Z

Resolved merge conflicts.

michalvavrik · 2026-04-22T11:25:01Z

I don't think the CI failure is related. I'll just leave it as is, I presume there will be more pushes and merge conflicts over time anyway.

michalvavrik · 2026-04-23T12:40:03Z

Resolved merge conflicts and addressed refactoring done by Stian (that renaming of builders).

keycloak-github-bot

Unreported flaky test detected, please review

keycloak-github-bot · 2026-04-23T13:45:39Z

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.authz.PolicyEvaluationTest#

Keycloak CI - Base IT (3)

org.opentest4j.AssertionFailedError: expected: <PERMIT> but was: <null>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
...

org.opentest4j.AssertionFailedError: expected: <PERMIT> but was: <null>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
...

org.opentest4j.AssertionFailedError: expected: <PERMIT> but was: <null>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
...

Report flaky test

vmuzikar

@michalvavrik Thank you for the PR, LGTM.

vmuzikar · 2026-04-23T16:43:03Z

@@ -27,6 +27,8 @@
        "org.keycloak.tests.session",
        "org.keycloak.tests.tracing",
        "org.keycloak.tests.transactions",
+        "org.keycloak.tests.ssl",


Why are we adding the ssl package here in this PR?

Why are we adding the ssl package here in this PR?

I was fixing merge conflicts after working on ssl and I probably missed some uncommitted changes. I'll fix it.

Fixed, thanks.

vmuzikar · 2026-04-23T16:58:05Z

+
+    static void assertAdminUrls(SimpleHttp simpleHttp, String serverBaseUrl, String realm,
+                                String expectedFrontendUrl, String expectedAdminUrl) throws IOException {
+        final String adminConsoleUrl = serverBaseUrl + "/admin/" + realm + "/console";


Isn't serverBaseUrl always keycloakUrls.getBase(), hence this parameter is not needed?

Isn't serverBaseUrl always keycloakUrls.getBase(), hence this parameter is not needed?

I have switched to keycloakUrls. I hope that is what you meant. We still need to construct the adminConsoleUrl manually as there is more than one realm. I checked and SimpleHttp doesn't seem to have a base url, so we need it. Hope its better now.

vmuzikar · 2026-04-23T18:36:07Z

The added comments are not blocking, let me know if you want to merge as is.

* Closes: keycloak#47813 * I kept the default `hostname-strict` as setting it makes no difference. It only enforces that a hostname must be provided — when one is set, it has no effect on URL resolution (the new framework uses `start-dev` which defaults to `false`) * I didn't see a way to test "restart with a different options" so I simply used a separate test classes with following mapping: | Old `HostnameV2Test` method | New counterpart | |--------------------------------------------------|--------------------------------------------------------------------------------------------------| | `testFixedFrontendHostname` | `HostnameV2FixedHostnameTest.testMasterRealmFrontendAndBackendUrls` + `testMasterRealmAdminUrls` | | `testFixedFrontendHostnameUrl` | `HostnameV2FixedUrlTest.testFrontendAndBackendUrls` + `testAdminUrls` | | `testFixedFrontendAndAdminHostnameUrl` | `HostnameV2FrontendAndAdminTest.testFrontendAndBackendUrls` + `testAdminUrls` | | `testFixedFrontendHostnameUrlWithDefaultPort` | `HostnameV2DefaultPortTest.testFrontendAndBackendUrls` + `testAdminUrls` | | `testDynamicBackend` | `HostnameV2BackchannelDynamicTest.testFrontendAndBackendUrls` + `testAdminUrls` | | `testDynamicEverything` | `HostnameV2NoHostnameConfiguredTest.testFrontendAndBackendUrls` + `testAdminUrls` | | `testRealmFrontendUrlWithOtherUrlsSet` | `HostnameV2RealmFrontendUrlOverrideTest.testFrontendAndBackendUrls` + `testAdminUrls` | | `testRealmFrontendUrl` | `HostnameV2FixedHostnameTest.testCustomRealmFrontendAndBackendUrls` + `testCustomRealmAdminUrls` | | `testStrictMode` | `HostnameV2DistTest.testServerFailsToStartWithoutHostnameSpecified` (already existed) | | `testStrictModeMustBeDisabledWhenHostnameIsSpecified` (commented out) | Not migrated — validation was relaxed to a log message in `HostnameV2ProviderFactory` (line 61) | | `testInvalidHostnameUrl` | `HostnameV2DistTest.testInvalidHostnameUrl` | | `testInvalidAdminUrl` | `HostnameV2DistTest.testInvalidAdminUrl` | | `testBackchannelDynamicRequiresHostname` | `HostnameV2DistTest.testBackchannelDynamicRequiresHostname` | | `testBackchannelDynamicRequiresFullHostnameUrl` | `HostnameV2DistTest.testBackchannelDynamicRequiresFullHostnameUrl` | Signed-off-by: Michal Vavřík <michal.vavrik@aol.com>

keycloak-github-bot

Unreported flaky test detected, please review

keycloak-github-bot · 2026-04-23T20:10:23Z

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.authz.PolicyEvaluationTest#

Keycloak CI - Base IT (3)

org.opentest4j.AssertionFailedError: expected: <PERMIT> but was: <null>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
...

org.opentest4j.AssertionFailedError: expected: <PERMIT> but was: <null>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
...

org.opentest4j.AssertionFailedError: expected: <PERMIT> but was: <null>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
...

Report flaky test

vmuzikar

LGTM

* Closes: keycloak#47813 * I kept the default `hostname-strict` as setting it makes no difference. It only enforces that a hostname must be provided — when one is set, it has no effect on URL resolution (the new framework uses `start-dev` which defaults to `false`) * I didn't see a way to test "restart with a different options" so I simply used a separate test classes with following mapping: | Old `HostnameV2Test` method | New counterpart | |--------------------------------------------------|--------------------------------------------------------------------------------------------------| | `testFixedFrontendHostname` | `HostnameV2FixedHostnameTest.testMasterRealmFrontendAndBackendUrls` + `testMasterRealmAdminUrls` | | `testFixedFrontendHostnameUrl` | `HostnameV2FixedUrlTest.testFrontendAndBackendUrls` + `testAdminUrls` | | `testFixedFrontendAndAdminHostnameUrl` | `HostnameV2FrontendAndAdminTest.testFrontendAndBackendUrls` + `testAdminUrls` | | `testFixedFrontendHostnameUrlWithDefaultPort` | `HostnameV2DefaultPortTest.testFrontendAndBackendUrls` + `testAdminUrls` | | `testDynamicBackend` | `HostnameV2BackchannelDynamicTest.testFrontendAndBackendUrls` + `testAdminUrls` | | `testDynamicEverything` | `HostnameV2NoHostnameConfiguredTest.testFrontendAndBackendUrls` + `testAdminUrls` | | `testRealmFrontendUrlWithOtherUrlsSet` | `HostnameV2RealmFrontendUrlOverrideTest.testFrontendAndBackendUrls` + `testAdminUrls` | | `testRealmFrontendUrl` | `HostnameV2FixedHostnameTest.testCustomRealmFrontendAndBackendUrls` + `testCustomRealmAdminUrls` | | `testStrictMode` | `HostnameV2DistTest.testServerFailsToStartWithoutHostnameSpecified` (already existed) | | `testStrictModeMustBeDisabledWhenHostnameIsSpecified` (commented out) | Not migrated — validation was relaxed to a log message in `HostnameV2ProviderFactory` (line 61) | | `testInvalidHostnameUrl` | `HostnameV2DistTest.testInvalidHostnameUrl` | | `testInvalidAdminUrl` | `HostnameV2DistTest.testInvalidAdminUrl` | | `testBackchannelDynamicRequiresHostname` | `HostnameV2DistTest.testBackchannelDynamicRequiresHostname` | | `testBackchannelDynamicRequiresFullHostnameUrl` | `HostnameV2DistTest.testBackchannelDynamicRequiresFullHostnameUrl` | Signed-off-by: Michal Vavřík <michal.vavrik@aol.com>

michalvavrik requested review from a team as code owners April 17, 2026 17:06

keycloak-github-bot Bot added team/cloud-native team/core-clients team/core-iam labels Apr 17, 2026

michalvavrik force-pushed the chore/issues/47813 branch from 4866c14 to ead0338 Compare April 17, 2026 17:34

michalvavrik requested a review from vmuzikar April 17, 2026 18:29

shawkins reviewed Apr 20, 2026

View reviewed changes

michalvavrik force-pushed the chore/issues/47813 branch from ead0338 to 49ed722 Compare April 22, 2026 10:10

michalvavrik force-pushed the chore/issues/47813 branch from 49ed722 to 5a7f722 Compare April 22, 2026 10:17

shawkins mentioned this pull request Apr 23, 2026

Minimize quarkus integration tests #48398

Closed

michalvavrik force-pushed the chore/issues/47813 branch from 5a7f722 to 2774a6d Compare April 23, 2026 12:39

michalvavrik force-pushed the chore/issues/47813 branch from 2774a6d to 72a52e5 Compare April 23, 2026 12:46

keycloak-github-bot Bot added the flaky-test label Apr 23, 2026

keycloak-github-bot Bot mentioned this pull request Apr 23, 2026

Flaky test: org.keycloak.testsuite.forms.AuthenticatorSubflowsTest2#testSubflow2 #47477

Closed

keycloak-github-bot Bot reviewed Apr 23, 2026

View reviewed changes

vmuzikar previously approved these changes Apr 23, 2026

View reviewed changes

michalvavrik dismissed vmuzikar’s stale review via fe7aa78 April 23, 2026 19:13

michalvavrik force-pushed the chore/issues/47813 branch from 72a52e5 to fe7aa78 Compare April 23, 2026 19:13

keycloak-github-bot Bot reviewed 10BC8 Apr 23, 2026

View reviewed changes

vmuzikar approved these changes Apr 24, 2026

View reviewed changes

vmuzikar merged commit aad0d3d into keycloak:main Apr 24, 2026
89 checks passed

michalvavrik deleted the chore/issues/47813 branch April 24, 2026 08:31

stianst mentioned this pull request Jun 1, 2026

js deps uuid 26.6 #49539

Closed

Navigation Menu

Repository navigation

Migrate url package to new test framework - #48212

Migrate url package to new test framework#48212
vmuzikar merged 1 commit into
keycloak:mainfrom
michalvavrik:chore/issues/47813

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Unreported flaky test detected

org.keycloak.testsuite.authz.PolicyEvaluationTest#

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Unreported flaky test detected

org.keycloak.testsuite.authz.PolicyEvaluationTest#

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants