Conversation
1) Does e2e test, i.e. goes to https://api.icndb.com 2) Does the same but acts against mocked ICNDB client
| @SpringBootTest | ||
| @RunWith(SpringRunner.class) | ||
| @AutoConfigureMockMvc | ||
| public class GraphQLIntegrationTest { |
There was a problem hiding this comment.
This is actually an e2e test, please rename accordingly
There was a problem hiding this comment.
how can I ensure that e2e tests are not executed in the same test run as unit and integration tests?
| private static final String JOKE_TEXT = "Chuck Norris can divide by zero."; | ||
|
|
||
| @Autowired | ||
| private MockMvc mockMvc; |
There was a problem hiding this comment.
By going through MockMvc we fail to test that Spring's DispatcherServlet routes to GraphQLServlet correctly in a real world scenario i.e. when an app is deployed in to a servlet container. I believe that we should do an actual POST instead
There was a problem hiding this comment.
please see this comment for a visual representation re e2e testing scope
| @SpringBootTest | ||
| @RunWith(SpringRunner.class) | ||
| @AutoConfigureMockMvc | ||
| public class GraphQLQueryTest { |
There was a problem hiding this comment.
This is actually an integration test, please rename accordingly
|
|
||
| private static final String GRAPHQL_PATH = "/graphql"; | ||
|
|
||
| private static final String QUERY = "{jokes {jokeById(id: \"77\"){id,text}}}"; |
There was a problem hiding this comment.
please extract to a .graphql file:
a) better IDE support, e.g. the IntelliJ GraphQL plugin will recognize and help out with the syntax
b) better readability for a human person
| result.andExpect(status().isOk()) | ||
| .andExpect(jsonPath("$.errors").doesNotExist()) | ||
| .andExpect(jsonPath("$.data.jokes.jokeById.id").value(JOKE_ID)) | ||
| .andExpect(jsonPath("$.data.jokes.jokeById.text").value(JOKE_TEXT)); |
There was a problem hiding this comment.
in case of an error, $.errors will be set to an array of errors. in order to fail fast, please assert that the error array is empty
| } | ||
|
|
||
| @SneakyThrows | ||
| private ResultActions graphQLPost() { |
There was a problem hiding this comment.
the fact that you actually do a HTTP POST is an implementation detail and is completely irrelevant to the reader, you may do a GET and nothing would change contract-wise.
please change the name to something like executeGraphQLQuery
| private static final String JOKE_TEXT = "Chuck Norris can divide by zero."; | ||
|
|
||
| @Autowired | ||
| private AppProperties appProperties; |
There was a problem hiding this comment.
why not auto wire required app properties directly? i.e.
@Value("icndb.url") private String icndbUrl;after all we only need a single value from the props, let's be straight about it
|
|
||
| @Getter | ||
| @Setter | ||
| public class ICNDBJoke { |
There was a problem hiding this comment.
@Getter and @Setter can be folded in to @Data
2 participants
Schema integration test #1
Two tests added.