-
Notifications
You must be signed in to change notification settings - Fork 0
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
Setup of test environment #34
base: master
Are you sure you want to change the base?
Conversation
@@ -14,7 +16,7 @@ | |||
private final UserService userService; | |||
|
|||
@PostMapping("/sign-up") | |||
public void signUp(@RequestBody ApplicationUser user) { | |||
public void signUp(@RequestBody ApplicationUser user) throws IOException { |
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.
I thought we do not use checked exceptions? 😄
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.
As far as i remember there was a compilation problem and I made the quick fix. You are right we had this agreement, but then I ask myself why this compilation problem was on master?
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.
As far as I remember Intelij suggested to add it because of compilation problems. But why then was it necessary? I pulled from master?
I check this out
ResultActions signUp = mockMvc.perform( | ||
withJsonHeaders(post("/user/sign-up")) | ||
.content(applicationUser.toString())); | ||
ResultActions login = mockMvc.perform( | ||
withJsonHeaders(post("/user/login")) | ||
.content(applicationUser.toString())); | ||
|
||
if (print) { | ||
signUp.andDo(print()); | ||
login.andDo(print()); | ||
} | ||
|
||
this.bearerToken = login.andReturn() | ||
.getResponse() | ||
.getHeader("Authorization"); |
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.
Did you take a look at the link I sent you? Seems like there is a much more Spring-like aproach that lets us get rid of all this. Here is even a lot more about Security+Testing: https://docs.spring.io/spring-security/site/docs/5.3.2.RELEASE/reference/html5/#test
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.
I had a short look at it, but i am more satisfied with reproducing the original requests one by one. Feels more like simply scripting API usage.
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.
I had a short look.
I will check this out again
|
||
@Before | ||
public void init() throws Exception { | ||
DbCleaningHelper.truncateAllTables(dataSource.getConnection()); |
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.
I am pretty sure using @Commit
and manually trunkating the whole DB is bad style. Making unit tests depending on each other also is. Instead we should bring the DB in a useful state with proper dummy data either in a initialization function or even in a SQL file that spring loads when connecting to the h2 db (https://docs.spring.io/spring-boot/docs/current/reference/htmlsingle/#howto-database-initialization).
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.
As far as I know this initialization only apply at the over all beginning of testing when the environment is set up. There is a way to reinitialize the db in this fashion before each test, but this is also very slow, because the whole environment would be set up again.
abstract public class MockMvcHelper { | ||
|
||
public static MockHttpServletRequestBuilder withJsonHeaders(MockHttpServletRequestBuilder builder) { | ||
return builder.contentType(MediaType.APPLICATION_JSON) | ||
.characterEncoding("utf-8"); | ||
} | ||
|
||
} |
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.
There must be a more Spring style approch for doing that, probably something like:
@Component
public class ContentTypeHeaderMockMvcBuilderCustomizer implements MockMvcBuilderCustomizer {
@Override
public void customize(ConfigurableMockMvcBuilder<?> builder) {
RequestBuilder improvedBuilder = MockMvcRequestBuilders.get("any")
.contentType(MediaType.APPLICATION_JSON)
.characterEncoding(StandardCharsets.UTF_8.name());
builder.defaultRequest(improvedBuilder);
}
}
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.
Found and adapted from https://stackoverflow.com/questions/50532856/mockmvc-configure-a-header-for-all-requests
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.
You are right and there are couples of other techniques. On the other side you wrote it to me yourself. Sometimes spring config stuff is complicated and especially it is not very domain specific. The whole utils stuff i wrote isn't in the spring style but it feasible to write down shortly the things we need.
However in this special case it might be worth to adapt your solution :)
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.
test
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.
test
No description provided.