-
Notifications
You must be signed in to change notification settings - Fork 5
Schema integration test #1 #37
base: master
Are you sure you want to change the base?
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is actually an e2e test, please rename accordingly
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
import java.util.List; | ||
|
||
@Getter | ||
@Setter | ||
public class ICNDBJoke { |
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.
@Getter
and @Setter
can be folded in to @Data
Schema integration test #1
Two tests added.