Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Switching to Resteasy Reactive #15728

Merged
merged 1 commit into from
Sep 18, 2023
Merged

Switching to Resteasy Reactive #15728

merged 1 commit into from
Sep 18, 2023

Conversation

pedroigor
Copy link
Contributor

@pedroigor pedroigor commented Nov 28, 2022 •
edited
Loading

Closes #10713
Closes #20451

@pedroigor pedroigor added the status/hold PR should not be merged. On hold for later. label Nov 28, 2022
@pedroigor pedroigor force-pushed the tmp-reactive branch 12 times, most recently from 335a920 to bd8f646 Compare December 3, 2022 01:20
@pedroigor pedroigor force-pushed the tmp-reactive branch 5 times, most recently from 5991fa1 to 34f40ec Compare December 9, 2022 14:07
@pedroigor pedroigor force-pushed the tmp-reactive branch 5 times, most recently from 215afa7 to b8d1ce1 Compare December 21, 2022 14:04
@ghost ghost added the flaky-test label Dec 21, 2022
ghost

This comment was marked as outdated.

Sign in to view

@pedroigor pedroigor force-pushed the tmp-reactive branch 2 times, most recently from 9e6d2ce to 35fad94 Compare December 22, 2022 15:33
@ghost ghost mentioned this pull request Dec 22, 2022
Closed
ghost

This comment was marked as outdated.

Sign in to view

ghost
ghost reviewed Aug 23, 2023
View reviewed changes
Copy link

@ghost ghost left a comment

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

@ghost
Copy link

ghost commented Aug 23, 2023

Unreported flaky test detected

If the below 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.ui.account2.PersonalInfoTest#navigationTest

Keycloak CI - Account Console IT (firefox)

java.lang.AssertionError: Expected PersonalInfoPage but was Keycloak Account Management (https://localhost:8543/auth/realms/test/account/#/personal-info&state=20617980-20f7-4ed0-92d4-be248ddfe337&session_state=562e2ce1-d669-4bc2-a3c8-9198d9d676e2&iss=https%3A%2F%2Flocalhost%3A8543%2Fauth%2Frealms%2Ftest&code=ca3b4410-77a0-47a4-aac7-f810d12ad859.562e2ce1-d669-4bc2-a3c8-9198d9d676e2.07725521-2024-429a-b0b4-bd1d00765300)
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.keycloak.testsuite.page.AbstractPage.assertCurrent(AbstractPage.java:110)
	at jdk.internal.reflect.GeneratedMethodAccessor101.invoke(Unknown Source)
...

Report flaky test

org.keycloak.testsuite.webauthn.account.WebAuthnSigningInTest#twoFactorWebAuthnTest

Keycloak CI - WebAuthn IT (chrome)

java.lang.AssertionError: Expected OIDCLogin but was  (https://localhost:8543/auth/realms/test/protocol/openid-connect/auth?client_id=account-console&redirect_uri=https%3A%2F%2Flocalhost%3A8543%2Fauth%2Frealms%2Ftest%2Faccount%2F%23%2Fsecurity%2Fsigningin&state=0d455899-f34d-4398-833b-5e11de43f7eb&response_mode=fragment&response_type=code&scope=openid&nonce=931ad461-1b17-412b-88df-bf9c6e9c9947&code_challenge=cR4dwUZCpv9fCzI9s6dtDwJcg8l4xkNbbrx0smdieQc&code_challenge_method=S256)
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.keycloak.testsuite.page.AbstractPage.assertCurrent(AbstractPage.java:110)
	at jdk.internal.reflect.GeneratedMethodAccessor622.invoke(Unknown Source)
...

Report flaky test

org.keycloak.testsuite.webauthn.account.WebAuthnSigningInTest#categoriesTest

Keycloak CI - WebAuthn IT (chrome)

java.lang.AssertionError: 

Expected: is <2>
     but: was <3>
	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
...

Report flaky test

ghost
ghost reviewed Aug 23, 2023
View reviewed changes
Copy link

@ghost ghost left a comment

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

@ghost
Copy link

ghost commented Aug 23, 2023

Unreported flaky test detected

If the below 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.ui.account2.PersonalInfoTest#navigationTest

Keycloak CI - Account Console IT (firefox)

java.lang.AssertionError: Expected PersonalInfoPage but was Keycloak Account Management (https://localhost:8543/auth/realms/test/account/#/personal-info&state=20617980-20f7-4ed0-92d4-be248ddfe337&session_state=562e2ce1-d669-4bc2-a3c8-9198d9d676e2&iss=https%3A%2F%2Flocalhost%3A8543%2Fauth%2Frealms%2Ftest&code=ca3b4410-77a0-47a4-aac7-f810d12ad859.562e2ce1-d669-4bc2-a3c8-9198d9d676e2.07725521-2024-429a-b0b4-bd1d00765300)
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.keycloak.testsuite.page.AbstractPage.assertCurrent(AbstractPage.java:110)
	at jdk.internal.reflect.GeneratedMethodAccessor101.invoke(Unknown Source)
...

Report flaky test

org.keycloak.testsuite.webauthn.account.WebAuthnSigningInTest#twoFactorWebAuthnTest

Keycloak CI - WebAuthn IT (chrome)

java.lang.AssertionError: Expected OIDCLogin but was  (https://localhost:8543/auth/realms/test/protocol/openid-connect/auth?client_id=account-console&redirect_uri=https%3A%2F%2Flocalhost%3A8543%2Fauth%2Frealms%2Ftest%2Faccount%2F%23%2Fsecurity%2Fsigningin&state=0d455899-f34d-4398-833b-5e11de43f7eb&response_mode=fragment&response_type=code&scope=openid&nonce=931ad461-1b17-412b-88df-bf9c6e9c9947&code_challenge=cR4dwUZCpv9fCzI9s6dtDwJcg8l4xkNbbrx0smdieQc&code_challenge_method=S256)
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.keycloak.testsuite.page.AbstractPage.assertCurrent(AbstractPage.java:110)
	at jdk.internal.reflect.GeneratedMethodAccessor622.invoke(Unknown Source)
...

Report flaky test

org.keycloak.testsuite.webauthn.account.WebAuthnSigningInTest#categoriesTest

Keycloak CI - WebAuthn IT (chrome)

java.lang.AssertionError: 

Expected: is <2>
     but: was <3>
	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
...

Report flaky test

This was referenced Aug 28, 2023
Closed
Closed
ghost
ghost reviewed Aug 28, 2023
View reviewed changes
Copy link

@ghost ghost left a comment

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

@ghost
Copy link

ghost commented Aug 28, 2023

Unreported flaky test detected

If the below 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.ui.account2.ApplicationsTest#toggleApplicationDetailsTest

Keycloak CI - Account Console IT (firefox)

org.awaitility.core.ConditionTimeoutException: Assertion condition defined as a org.keycloak.testsuite.util.URLAssert URL expected to begin with: https://localhost:8543/auth/realms/test/protocol/openid-connect/auth ; actual URL: https://localhost:8543/auth/realms/test/account/#/applications within 10 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:119)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:31)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:985)
...

Report flaky test

org.keycloak.testsuite.ui.account2.MyResourcesTest#shouldAllowRequestToShare

Keycloak CI - Account Console IT (firefox)

org.openqa.selenium.UnhandledAlertException: 
Dismissed user prompt dialog: failed to initialize keycloak: 
Build info: version: '3.14.0', revision: 'aacccce0', time: '2018-08-02T20:19:58.91Z'
System info: host: 'fv-az337-514', ip: '10.1.0.161', os.name: 'Linux', os.arch: 'amd64', os.version: '5.15.0-1041-azure', java.version: '17.0.8'
Driver info: org.openqa.selenium.firefox.FirefoxDriver
...

Report flaky test

@ahus1 ahus1 mentioned this pull request Sep 13, 2023
Closed
This was referenced Sep 14, 2023
Closed
Closed
@ghost
Copy link

ghost commented Sep 14, 2023

Unreported flaky test detected

If the below 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.ui.account2.ApplicationsTest#toggleApplicationDetailsTest

Keycloak CI - Account Console IT (firefox)

org.awaitility.core.ConditionTimeoutException: Assertion condition defined as a org.keycloak.testsuite.util.URLAssert URL expected to begin with: https://localhost:8543/auth/realms/test/protocol/openid-connect/auth ; actual URL: https://localhost:8543/auth/realms/test/account/#/applications within 10 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:119)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:31)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:985)
...

Report flaky test

org.keycloak.testsuite.ui.account2.SessionTest#welcomeScreenSsoTimeoutTest

Keycloak CI - Account Console IT (firefox)

org.openqa.selenium.UnhandledAlertException: 
Dismissed user prompt dialog: failed to initialize keycloak: 
Build info: version: '3.14.0', revision: 'aacccce0', time: '2018-08-02T20:19:58.91Z'
System info: host: 'fv-az1030-38', ip: '10.1.0.38', os.name: 'Linux', os.arch: 'amd64', os.version: '6.2.0-1011-azure', java.version: '17.0.8.1'
Driver info: org.openqa.selenium.firefox.FirefoxDriver
...

Report flaky test

ghost
ghost reviewed Sep 14, 2023
View reviewed changes
Copy link

@ghost ghost left a comment

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

ghost
ghost reviewed Sep 14, 2023
View reviewed changes
Copy link

@ghost ghost left a comment

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

@ghost
Copy link

ghost commented Sep 14, 2023

Unreported flaky test detected

If the below 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.ui.account2.LinkedAccountsTest#unlinkAccountTest

Keycloak CI - Account Console IT (firefox)

org.awaitility.core.ConditionTimeoutException: Assertion condition defined as a org.keycloak.testsuite.util.URLAssert URL expected to begin with: https://localhost:8543/auth/realms/test/protocol/openid-connect/auth ; actual URL: https://localhost:8543/auth/realms/test/account/#/security/linked-accounts within 10 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:119)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:31)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:985)
...

Report flaky test

ghost
ghost reviewed Sep 14, 2023
View reviewed changes
Copy link

@ghost ghost left a comment

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

@ghost
Copy link

ghost commented Sep 14, 2023

Unreported flaky test detected

If the below 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.ui.account2.LinkedAccountsTest#unlinkAccountTest

Keycloak CI - Account Console IT (firefox)

org.awaitility.core.ConditionTimeoutException: Assertion condition defined as a org.keycloak.testsuite.util.URLAssert URL expected to begin with: https://localhost:8543/auth/realms/test/protocol/openid-connect/auth ; actual URL: https://localhost:8543/auth/realms/test/account/#/security/linked-accounts within 10 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:119)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:31)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:985)
...

Report flaky test

vmuzikar
vmuzikar requested changes Sep 15, 2023
View reviewed changes
Copy link
Contributor

@vmuzikar vmuzikar left a comment

Choose a reason for hiding this comment

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

@pedroigor Thanks, very nice work!

Added one comment. Apart from that LGTM but I'm not super familiar with the low-level RESTEasy stuff.

...runtime/src/main/java/org/keycloak/quarkus/runtime/integration/web/QuarkusRequestFilter.java
ctx.fail(ex);
}
}, false, null);
if (loadSheddingActive) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this PR broke the load shedding. See also #21962. We should've had probably tests for it.

CC @ahus1

Copy link
Contributor

Choose a reason for hiding this comment

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

Load shedding is not officially support as of now. If this PR breaks it, just continue with this PR and we'll fix it (and officially support it) afterwards.

@ahus1 ahus1 mentioned this pull request Sep 18, 2023
Closed
vmuzikar
vmuzikar approved these changes Sep 18, 2023
View reviewed changes
Copy link
Contributor

@vmuzikar vmuzikar left a comment

Choose a reason for hiding this comment

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

Approving based on @ahus1's feedback as my only concern is now resolved.

@pedroigor pedroigor merged commit 217a09c into keycloak:main Sep 18, 2023
71 of 72 checks passed
@pedroigor pedroigor deleted the tmp-reactive branch September 18, 2023 12:19
@ahus1 ahus1 mentioned this pull request Sep 19, 2023
Closed
@mabartos mabartos mentioned this pull request Oct 18, 2023
Closed
@Eng-Fouad Eng-Fouad mentioned this pull request Dec 3, 2023
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ahus1 ahus1 ahus1 left review comments

@geoand geoand geoand left review comments

@vmuzikar vmuzikar vmuzikar approved these changes

@mabartos mabartos mabartos approved these changes

Assignees

@vmuzikar vmuzikar

Labels
flaky-test status/hold PR should not be merged. On hold for later. team/cloud-native
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

User search returns invalid JSON if request caused error Update the server to use RESTEasy Reactive
6 participants
@pedroigor @vmuzikar @tkyjovsk @geoand @mabartos @ahus1