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.

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

[KEYCLOAK-11746] More unit tests: browser flow #18

Closed
wants to merge 1 commit into from

Conversation

fperot74
Copy link

No description provided.

Copy link

@AlistairDoswald AlistairDoswald left a comment

Choose a reason for hiding this comment

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

Code is good, but I think that with the refactoring of the manner in which flows are built for the tests, we can create a class in org.keycloak.testsuite.util that will be of more general use.

}

private static void addAuthenticatorExecutionSubFlow(RealmModel realm, Requirement requirement, int priority, AuthenticationFlowModel subFlow, AuthenticationFlowModel parentFlow) {
AuthenticationExecutionModel execution = new AuthenticationExecutionModel();

Choose a reason for hiding this comment

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

I think we can do a bit better here: have the method create the flow and the executor.

}

private static void addAuthenticatorExecution(RealmModel realm, Requirement requirement, String providerId, int priority, AuthenticationFlowModel parentFlow) {

Choose a reason for hiding this comment

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

These methods are really useful, and I think that we would benefit from moving them to a util class rather than just keeping them here. For example, they could also be used to simplify the code in KcOidcFirstBrokerLoginNewAuthTest

Copy link

@AlistairDoswald AlistairDoswald left a comment

Choose a reason for hiding this comment

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

The activate flow shouldn't be restricted to browser, but otherwise it's OK.

return this;
}

public void activateFlow() {

Choose a reason for hiding this comment

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

This will be used for more than just browser flows. So either the tool should know which flow to replace from a previous step, or it should be defined here.

@AlistairDoswald
Copy link

Replaced by #19

lagess pushed a commit to lagess/keycloak that referenced this pull request Mar 3, 2023
lagess pushed a commit to lagess/keycloak that referenced this pull request Mar 3, 2023
…loak#17380)

Co-authored-by: Pedro Igor <pigor.craveiro@gmail.com>
(cherry picked from commit ec81091)
lagess pushed a commit that referenced this pull request Mar 6, 2023
lagess pushed a commit to lagess/keycloak that referenced this pull request Apr 17, 2023
lagess pushed a commit to lagess/keycloak that referenced this pull request Apr 17, 2023
* KEYCLOAK-15527 Adding context to master.adoc and index.adoc to allow reuse

* KEYCLOAK-15614 modularisation of 'Creating an Open ID Connect Client' (cloudtrust#14)

* More changes based on feedback

* Puts back 2 out of 3 screens

* KEYCLOAK-15614 updates to anchors

* Minor spelling correction in Getting Started

* Keycloak 15786 (cloudtrust#16)

* rebase corrections

* post feedback changes

* post review changes (cloudtrust#18)

* KEYCLOAK-15616 initial commit

Co-authored-by: Andy Munro <amunro@redhat.com>
lagess pushed a commit to lagess/keycloak that referenced this pull request Apr 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants