Skip to content

Added Unit Tests For oauth2-core #4499

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

Closed
wants to merge 3 commits into from

Conversation

luander
Copy link
Contributor

@luander luander commented Aug 16, 2017

I added new tests for the oauth2-core according to the oauth 2 RFC 6749.
These tests are only for the "endpoint" package. Other tests may be added/required.
In this PR I also included some fixes for the classes tested.

Part of #4298

This commit contains unit tests for Oauth2-Core.
This commit fixes behavior according to unit tests.
@jgrandja jgrandja added the in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) label Aug 16, 2017
@jgrandja jgrandja self-assigned this Aug 16, 2017
@jgrandja jgrandja added this to the 5.0.0.M4 milestone Aug 23, 2017
Copy link
Contributor

@jgrandja jgrandja left a comment

Choose a reason for hiding this comment

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

@luander The tests look great. There are a couple of minor updates I'm requesting. After you update I'll merge. Thanks!

@@ -28,6 +29,7 @@
private String code;
private String clientId;
private String redirectUri;
private AuthorizationGrantType grantType;
Copy link
Contributor

Choose a reason for hiding this comment

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

The grant type member is not needed here as it's implied based on the class name -> AuthorizationCodeTokenRequestAttributes. Can you remove this please.

}

@Test(expected = IllegalArgumentException.class)
public void buildWhenClientIdIsNotCalledThenThrowIllegalArgumentException() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please rename to buildWhenClientIdNotSetThenThrowIllegalArgumentException

}

@Test(expected = IllegalArgumentException.class)
public void buildWhenRedirectUriIsNotCalledThenThrowIllegalArgumentException() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please rename to buildWhenRedirectUriNotSetThenThrowIllegalArgumentException

}

@Test
public void buildWhenGetGrantTypeIsCalledThenReturnAuthorizationCodeString() {
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be removed based on previous comment

}

@Test(expected = IllegalArgumentException.class)
public void buildWhenAuthorizeUriIsNotCalledThenThrowIllegalArgumentException() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please rename to buildWhenAuthorizeUriNotSetThenThrowIllegalArgumentException

}

@Test
public void buildWhenStateIsNotCalledThenDoesNotThrowAnyException() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please rename to buildWhenStateNotSetThenDoesNotThrowAnyException

import org.junit.Test;

/**
* Tests ${@link ErrorResponseAttributes}
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove $

public class ErrorResponseAttributesTest {

@Test(expected = IllegalArgumentException.class)
public void builderWhenCodeIsNullThenThrowIllegalArgumentException() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please rename to withErrorCodeWhenCodeIsNullThenThrowIllegalArgumentException

import java.util.Collections;

/**
* Tests ${@link TokenResponseAttributes}
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove $

}

@Test(expected = IllegalArgumentException.class)
public void buildWhenTokeTypeIsNotCalledThenThrowIllegalArgumentException() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please rename to buildWhenTokenTypeNotSetThenThrowIllegalArgumentException

@luander
Copy link
Contributor Author

luander commented Aug 24, 2017

Thanks for the feedback @jgrandja
Please let me know if any other changes are necessary.

@jgrandja jgrandja closed this in ec908bb Aug 25, 2017
@jgrandja
Copy link
Contributor

Thanks for your help in adding these tests @luander. This is now merged to master.

@luander
Copy link
Contributor Author

luander commented Aug 25, 2017

Thank you for accepting my changes. I want to help more.

@jgrandja
Copy link
Contributor

We could definitely use more help especially with writing tests. I'll keep an eye out for your contributions. Thanks again!

thomasdarimont pushed a commit to thomasdarimont/spring-security that referenced this pull request Apr 25, 2018
Fixes spring-projectsgh-4499

This commit contains unit tests for the endpoints package in oauth2-core.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants