Add a Maven profile to remove GELF support by mabartos · Pull Request #22615 · keycloak/keycloak

mabartos · 2023-08-22T13:36:27Z

mabartos · 2023-08-24T17:07:32Z

Should be ready for review now.

ghost

Unreported flaky test detected, please review

ghost · 2023-08-24T18:31:32Z

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.SessionTest#welcomeScreenAsyncLogoutTest

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/#/personal-info 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

mabartos · 2023-08-25T10:03:25Z

I've removed the comprehensive approach for HelpCommandDistTest to be working for the product. It'd bring a lot of maintenance difficulties. You can check out what'd be necessary to make it work here: mabartos@0246380

vmuzikar

I think we should have some test coverage to check GELF is not present in the help. But I agree adding more approved help txts is a bit cumbersome.

Maybe we could create some CheckGelfRemoved test that would just assert the string gelf is not present in help?

mabartos

@vmuzikar That should be sufficient. I manually tested for GELF enabled/disabled and it works as expected.

Is it ok for you?

ghost

Unreported flaky test detected, please review

ghost · 2023-08-25T11:00:28Z

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.forms.VerifyProfileTest#testAttributeRequiredAndSelectedByScope

Keycloak CI - Forms IT (chrome)

java.lang.IllegalArgumentException: No enum constant org.keycloak.testsuite.pages.AppPage.RequestType.
	at java.base/java.lang.Enum.valueOf(Enum.java:273)
	at org.keycloak.testsuite.pages.AppPage$RequestType.valueOf(AppPage.java:56)
	at org.keycloak.testsuite.pages.AppPage.getRequestType(AppPage.java:49)
	at jdk.internal.reflect.GeneratedMethodAccessor552.invoke(Unknown Source)
...

Report flaky test

vmuzikar · 2023-08-25T15:28:57Z

We'll also need to update the docs to exclude GELF on -Dproduct.

ahus1 · 2023-08-28T10:29:29Z

@mabartos - the logging guide mentions GELF explicitly, so I assume it needs to be changed as well (either in this PR, or another PR).

mabartos · 2023-08-28T10:55:13Z

@ahus1 Yes, I'm on it.

Closes keycloak#22515 Co-authored-by: Václav Muzikář <vmuzikar@redhat.com>

mabartos · 2023-08-29T13:55:45Z

After a discussion with @pedroigor, the property persisting during the build time is not a blocker for this PR.

Documentation stuff works as expected; I've tried it with product doc.

@vmuzikar Could you please check it? (Sorry, I've already squashed all commits, but no big changes were provided since the last time)

pedroigor

@mabartos Sure. We can do it as a follow-up.

vmuzikar · 2023-08-29T14:03:15Z

@mabartos Thanks for the update! Can you please create an issue for the follow-up?

mabartos · 2023-08-29T15:13:43Z

Created follow-up general task related to the static initialization : #22799

ghost

Unreported flaky test detected, please review

ghost · 2023-08-29T17:12:50Z

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.adapter.example.cors.CorsExampleAdapterTest#angularCorsProductTest

Keycloak CI - Base IT (2)

junit.framework.AssertionFailedError
	at junit.framework.Assert.fail(Assert.java:55)
	at junit.framework.Assert.assertTrue(Assert.java:22)
	at junit.framework.Assert.assertNotNull(Assert.java:256)
	at junit.framework.Assert.assertNotNull(Assert.java:248)
...

Report flaky test

mabartos added the status/hold PR should not be merged. On hold for later. label Aug 22, 2023

mabartos self-assigned this Aug 22, 2023

vmuzikar linked an issue Aug 23, 2023 that may be closed by this pull request

Add a Maven profile to remove GELF support #22515

Closed

vmuzikar requested changes Aug 24, 2023

View reviewed changes

Comment thread quarkus/deployment/pom.xml Outdated

Comment thread quarkus/runtime/pom.xml Outdated

Comment thread quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/Environment.java Outdated

mabartos force-pushed the gelfProductRemoval branch from 346b1d4 to d0961c7 Compare August 24, 2023 16:50

mabartos commented Aug 24, 2023

View reviewed changes

Comment thread quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/HelpCommandDistTest.java Outdated

mabartos changed the title ~~WiP - Remove GELF from product distribution~~ Add a Maven profile to remove GELF support Aug 24, 2023

mabartos removed the status/hold PR should not be merged. On hold for later. label Aug 24, 2023

mabartos marked this pull request as ready for review August 24, 2023 16:56

mabartos requested review from a team as code owners August 24, 2023 16:56

ghost added team/cloud-native labels Aug 24, 2023

mabartos force-pushed the gelfProductRemoval branch 3 times, most recently from b299296 to 0246380 Compare August 24, 2023 17:03

ghost added the flaky-test label Aug 24, 2023

ghost mentioned this pull request Aug 24, 2023

Flaky test: org.keycloak.testsuite.model.ConcurrentHashMapStorageTest#testStorageSeparation #22645

Closed

ghost reviewed Aug 24, 2023

View reviewed changes

mabartos requested review from Pepo48 and shawkins August 25, 2023 08:47

mabartos force-pushed the gelfProductRemoval branch from 0246380 to 2a28be3 Compare August 25, 2023 10:00

vmuzikar requested changes Aug 25, 2023

View reviewed changes

mabartos force-pushed the gelfProductRemoval branch from 2a28be3 to 174b580 Compare August 25, 2023 10:54

mabartos commented Aug 25, 2023

View reviewed changes

Comment thread quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/GelfRemovedTest.java

This was referenced Aug 25, 2023

Flaky test: org.keycloak.testsuite.forms.BrowserFlowTest#testAlternativeNonInteractiveExecutorInSubflow #22644

Closed

Flaky test: org.keycloak.testsuite.forms.RecoveryAuthnCodesAuthenticatorTest#test03AuthenticateRecoveryAuthnCodes #22691

Closed

ghost reviewed Aug 25, 2023

View reviewed changes

vmuzikar reviewed Aug 25, 2023

View reviewed changes

Comment thread quarkus/server/pom.xml Outdated

pedroigor reviewed Aug 25, 2023

View reviewed changes

Comment thread quarkus/config-api/src/main/java/org/keycloak/config/LoggingOptions.java Outdated

mabartos marked this pull request as draft August 28, 2023 10:54

Pepo48 reviewed Aug 28, 2023

View reviewed changes

Comment thread quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/GelfRemovedTest.java Outdated

This was referenced Aug 28, 2023

Provide support for determining community/product guides #22762

Closed

Provide support for determining community/product guides #22763

Merged

Add a Maven profile to remove GELF support

f8a4b67

Closes keycloak#22515 Co-authored-by: Václav Muzikář <vmuzikar@redhat.com>

mabartos force-pushed the gelfProductRemoval branch from 5765bea to f8a4b67 Compare August 29, 2023 13:36

mabartos marked this pull request as ready for review August 29, 2023 13:53

pedroigor approved these changes Aug 29, 2023

View reviewed changes

vmuzikar approved these changes Aug 29, 2023

View reviewed changes

vmuzikar enabled auto-merge (squash) August 29, 2023 14:02

This was referenced Aug 29, 2023

Add a Maven profile to remove GELF support #22797

Merged

Analyze all static initialization during runtime #22799

Open

vmuzikar merged commit 7c013e8 into keycloak:main Aug 29, 2023

ghost reviewed Aug 29, 2023

View reviewed changes

Uh oh!

Conversation

mabartos commented Aug 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mabartos commented Aug 24, 2023

Uh oh!

ghost left a comment

Choose a reason for hiding this comment

Uh oh!

ghost commented Aug 24, 2023

Unreported flaky test detected

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

Uh oh!

mabartos commented Aug 25, 2023

Uh oh!

vmuzikar left a comment

Choose a reason for hiding this comment

Uh oh!

mabartos left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ghost left a comment

Choose a reason for hiding this comment

Uh oh!

ghost commented Aug 25, 2023

Unreported flaky test detected

org.keycloak.testsuite.forms.VerifyProfileTest#testAttributeRequiredAndSelectedByScope

Uh oh!

Uh oh!

vmuzikar commented Aug 25, 2023

Uh oh!

Uh oh!

ahus1 commented Aug 28, 2023

Uh oh!

mabartos commented Aug 28, 2023

Uh oh!

Uh oh!

mabartos commented Aug 29, 2023

Uh oh!

pedroigor left a comment

Choose a reason for hiding this comment

Uh oh!

vmuzikar commented Aug 29, 2023

Uh oh!

mabartos commented Aug 29, 2023

Uh oh!

ghost left a comment

Choose a reason for hiding this comment

Uh oh!

ghost commented Aug 29, 2023

Unreported flaky test detected

org.keycloak.testsuite.adapter.example.cors.CorsExampleAdapterTest#angularCorsProductTest

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

mabartos commented Aug 22, 2023 •

edited

Loading

mabartos left a comment •

edited

Loading