8000 fix: remove erroneous spi warnings by shawkins · Pull Request #33648 · keycloak/keycloak · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix: remove erroneous spi warnings #33648

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
Merged
merged 1 commit into from
Oct 21, 2024
Merged

fix: remove erroneous spi warnings #33648

merged 1 commit into from
Oct 21, 2024

Conversation

shawkins
Copy link
Contributor
@shawkins shawkins commented Oct 7, 2024 •
edited
Loading

closes: #34057

Follow up to #33638 to make all sys out unit testable. There are a couple of changes for this:

  • switch from using a logger to the picocli out for warnings. Usage of the logger was a little problematic in any case, and this makes sure that warnings are emitted the same way as other cli time output, including help. Some coloring was added for the WARNING: prefix, but can be tweaked / removed if anyone objects.
  • rather than using a boolean flag, fully encapsulate quarkus callouts on the Picocli class and inject the instance when commands are created.

The changes to capture the out output means we could do the help tests in this manner as well.

@shawkins shawkins requested review from a team as code owners October 7, 2024 14:46
@shawkins shawkins requested review from vmuzikar and mabartos October 7, 2024 14:46
@shawkins shawkins force-pushed the iss33638 branch 2 times, most recently from 7d6e138 to 8dc4bb0 Compare October 8, 2024 14:23
@keycloak-github-bot keycloak-github-bot bot mentioned this pull request Oct 8, 2024
Closed
@shawkins shawkins mentioned this pull request Oct 15, 2024
Closed
@shawkins shawkins marked this pull request as draft October 16, 2024 00:58
@shawkins shawkins mentioned this pull request Oct 16, 2024
Merged
@shawkins
Copy link
Contributor Author

Opened #33977 to expedite the fix.

@shawkins shawkins marked this pull request as ready for review October 16, 2024 16:04
@shawkins
fix: remove erroneous spi warnings
dbae3da 
closes: keycloak#34057

Signed-off-by: Steve Hawkins <shawkins@redhat.com>
keycloak-github-bot[bot]
keycloak-github-bot bot reviewed Oct 17, 2024
View reviewed changes
Copy link
@keycloak-github-bot keycloak-github-bot bot 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

@keycloak-github-bot
Copy link

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.broker.KcOidcBrokerTest#testPostBrokerLoginFlowWithOTP_bruteForceEnabled

Keycloak CI - Java Distribution IT (windows-latest - temurin - 17)

java.lang.AssertionError: Did not find the event of expected type USER_DISABLED_BY_TEMPORARY_LOCKOUT. Events present: [IDENTITY_PROVIDER_POST_LOGIN_ERROR, IDENTITY_PROVIDER_POST_LOGIN_ERROR, IDENTITY_PROVIDER_POST_LOGIN_ERROR, IDENTITY_PROVIDER_POST_LOGIN_ERROR]
	at org.junit.Assert.fail(Assert.java:89)
	at org.keycloak.testsuite.AssertEvents$ExpectedEvent.assertEvent(AssertEvents.java:410)
	at org.keycloak.testsuite.broker.AbstractAdvancedBrokerTest.testPostBrokerLoginFlowWithOTP_bruteForceEnabled(AbstractAdvancedBrokerTest.java:586)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
...

Report flaky test

vmuzikar
vmuzikar approved these changes Oct 21, 2024
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.

LGTM, thanks.

@vmuzikar vmuzikar merged commit 1d38fa8 into keycloak:main Oct 21, 2024
74 checks passed
Petar-CV pushed a commit to Petar-CV/keycloak that referenced this pull request Oct 25, 2024
@shawkins @Petar-CV
fix: remove erroneous spi warnings (keycloak#33648)
6dbddf5 
closes: keycloak#34057

Signed-off-by: Steve Hawkins <shawkins@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

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

@vmuzikar vmuzikar vmuzikar approved these changes

@mabartos mabartos Awaiting requested review from mabartos

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.

PicoCli unit testing should support sys out capture
2 participants
@shawkins @vmuzikar
0