8000 Migrate ConditionalCredentialAuthenticatorTest by vaceksimon · Pull Request #52072 · keycloak/keycloak · GitHub
[go: up one dir, main page]

Skip to content

Migrate ConditionalCredentialAuthenticatorTest - #52072

Merged
rmartinc merged 2 commits into
keycloak:mainfrom
vaceksimon:52070-ConditionalCredentialAuthenticatorTest
Sep 30, 2026
Merged

rmartinc merged 2 commits into
keycloak:mainfrom
vaceksimon:52070-ConditionalCredentialAuthenticatorTest

Conversation

@vaceksimon
Copy link
Copy Markdown
Contributor

Closes: #52070

Copilot AI balanced review requested due to automatic review settings August 26, 2026 10:50
@vaceksimon
vaceksimon requested review from a team as code owners August 26, 2026 10:50
Copilot AI 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.

Pull request overview

Migrates ConditionalCredentialAuthenticatorTest from Arquillian to the new test framework.

Changes:

  • Reworks realm, OAuth, events, and UI test setup.
  • Adds OTP page objects and WebDriver support.
  • Extends RealmBuilder with OTP configuration methods.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
LoginTotpPage.java (legacy) Removes migrated helpers.
ConditionalCredentialAuthenticatorTest.java Migrates the integration test.
ManagedWebDriver.java Adds multi-element lookup.
OneTimeCode.java Adds an OTP page object.
LoginTotpPage.java (framework) Adds the framework TOTP page object.
RealmBuilder.java Adds OTP policy setters.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot AI review requested due to automatic review settings August 26, 2026 11:45
@vaceksimon
vaceksimon force-pushed the 52070-ConditionalCredentialAuthenticatorTest branch 2 times, most recently from 3dbd213 to 20e7825 Compare August 26, 2026 11:46
Copilot AI 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.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

@vaceksimon
Copy link
Copy Markdown
Contributor Author

Current WebAuthn failure related to:

Copilot AI review requested due to automatic review settings September 23, 2026 11:47
@vaceksimon
vaceksimon force-pushed the 52070-ConditionalCredentialAuthenticatorTest branch from 20e7825 to 04b097e Compare September 23, 2026 11:47
Copilot AI 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.

Copilot review overview

🟢 Approval recommended

The reviewed changes have no unresolved approval-blocking issues.

Review effort: Balanced
Findings: None

Resolved since last review (1)

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

Thanks @vaceksimon! LGTM! Just a nit-pick to not delete and create the realm in each method. Do the logout or move the lifecycle=Method to the user.

Closes: keycloak#52070

Signed-off-by: Simon Vacek <vaceksimon-github@pm.me>
Signed-off-by: Simon Vacek <vaceksimon-github@pm.me>
Copilot AI review requested due to automatic review settings September 29, 2026 12:33
@vaceksimon
vaceksimon force-pushed the 52070-ConditionalCredentialAuthenticatorTest branch from 04b097e to 60fd616 Compare September 29, 2026 12:33
@vaceksimon
vaceksimon requested a review from rmartinc September 29, 2026 12:34
Copilot AI 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.

Copilot review overview

🔵 Needs a closer look

Test teardown and flow cleanup can produce cascading or misleading failures when setup or login fails.

Review effort: Balanced
Findings: None

@vaceksimon
Copy link
Copy Markdown
Contributor Author

Hey @rmartinc, I implemented the suggestion

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

Thanks for the changes @vaceksimon!

@rmartinc
rmartinc merged commit 9e75c7d into keycloak:main Sep 30, 2026
94 checks passed
@vaceksimon
vaceksimon deleted the 52070-ConditionalCredentialAuthenticatorTest branch September 30, 2026 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Migrate ConditionalCredentialAuthenticatorTest to test framework

3 participants

0