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

[WFLY-11813] Test for empty username for Elytron's FormAuthenticationMechanism #12125

Conversation

spriadka
Copy link
Contributor

FormMechTestCase#testEmptyUsername() could be added to FormMechTestBase, however I'm not sure if it is within desired scope of other test classes extending FormMechTestBase

@wildfly-ci
Copy link

Can one of the admins verify this patch?

@spriadka spriadka changed the title added test for empty username + additional minor refactoring [WFLY-11813] Test for empty username for Elytron's FormAuthenticationMechanism Mar 6, 2019
@mchoma
Copy link
Contributor

mchoma commented Mar 6, 2019

@spriadka could you squash commits? Otherwise seems good to me.

@darranl
Copy link
Contributor

darranl commented Mar 7, 2019

The commit messages should also reference the WFLY Jira issue.

…cationMechanism + additional minor refactoring
@spriadka spriadka force-pushed the tests-for-empty-username-http-form-elytron branch from f614959 to 335a0b5 Compare March 7, 2019 12:33
@spriadka
Copy link
Contributor Author

spriadka commented Mar 7, 2019

Thank you @mchoma and @darranl. I have squashed the commits and added JIRA reference to commit message.

@jamezp
Copy link
Member

jamezp commented Mar 22, 2019

@darranl or @fjuma could one of you please review this?

import org.apache.http.util.EntityUtils;
import org.junit.Test;

import static org.apache.http.HttpStatus.SC_FORBIDDEN;
Copy link
Contributor

Choose a reason for hiding this comment

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

The import order is wrong on this one, statics should come first.

@darranl
Copy link
Contributor

darranl commented May 14, 2019

@spriadka Are you able to get the imports corrected and then we can look to get this merged?

@bstansberry
Copy link
Contributor

Resolved via #12744, which just corrected the import order here. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants