-
Notifications
You must be signed in to change notification settings - Fork 11
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
Added tests for custom exceptions #556
base: develop
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please contact me any time if you need any help or if you have any questions.
src/test/java/com/softserveinc/dokazovi/controller/AuthControllerTest.java
Show resolved
Hide resolved
src/test/java/com/softserveinc/dokazovi/controller/AuthControllerTest.java
Show resolved
Hide resolved
UserEntity user = buildUserEntity(EMAIL, PASSWORD, true); | ||
when(authenticationManager.authenticate(any(UsernamePasswordAuthenticationToken.class))) | ||
.thenReturn(authentication); | ||
when(tokenProvider.createToken(any(Authentication.class))).thenReturn(TOKEN); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make assertion on token value after post request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
} | ||
|
||
private Authentication authenticate(LoginRequest loginRequest) { | ||
return authenticationManager.authenticate( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
authenticationManager
is mocked and couldn't provide proper response.
If I'm not mistaken, you can return
new UsernamePasswordAuthenticationToken(loginRequest.getEmail(), loginRequest.getPassword())
without calling authenticationManager
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
verify(postRepository, never()) | ||
.save(any(PostEntity.class) | ||
); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove empty line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
Timestamp publishedAt = Timestamp.valueOf( | ||
LocalDateTime.of( | ||
LocalDate.of( | ||
2002, Month.JANUARY, 14), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please move it on the same line with LocalDate.of(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
@@ -37,6 +40,8 @@ | |||
@MockitoSettings(strictness = Strictness.LENIENT) | |||
class AuthControllerTest { | |||
|
|||
|
|||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove excessive empty lines
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
@@ -1278,6 +1279,79 @@ void updatePostById_WhenExists_isOk_DoctorRole() { | |||
Assertions.assertThat(postService.updatePostById(userPrincipal, dto)).isTrue(); | |||
} | |||
|
|||
@Test | |||
void updatePostById_WhenPostStatus_NotFount_ThrowException() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NotFount
it appears to be mistyping here. It should be NotFound
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
.contentType(MediaType.APPLICATION_JSON) | ||
.accept(MediaType.APPLICATION_JSON)) | ||
.andExpect(status().isOk()); | ||
assertEquals(TOKEN, tokenProvider.createToken(authentication)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That assertion doesn't make sense because it uses mocked value result from line 81 when(tokenProvider.createToken(authentication)).thenReturn(TOKEN);
You should compare TOKEN
with value in the response after POST request. In your case, it will look like
mockMvc.perform(MockMvcRequestBuilders.post(URI) .content(asJsonString(loginRequest)) .contentType(MediaType.APPLICATION_JSON) .accept(MediaType.APPLICATION_JSON)) .andExpect(status().isOk()) .andExpect(mvcResult -> mvcResult.getResponse().getContentAsString().equals(TOKEN));
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
develop
GitHub Board
Issue link
Tests for custom exceptions #555
Code reviewers
Summary of issue
Summary of change