Recently I had my first contact with the class TestRestTemplate and started using it for unit tests of my rest controllers.
Just to give some context, for my POST
tests I just call postForObject
, get the response as a string
and match it against a regex, like the example below:
@Test
public void shouldThrowNullUsernameException() {
CreateUserParams request = new CreateUserParams();
request.username = null;
request.password = "123456";
String response = testRestTemplate.postForObject(url(), request, String.class);
assertTrue(response.matches("(.*)" + NullUsernameException.message() + "(.*)"));
}
Everything was fine until I needed to test my PUT methods. If for POST I used postForObject
, my first thought for PUT
was to use put(URI url, Object request)
. According to HTTP specification a successfull PUT request should not return a body, so naturally the put
method should be void
. Trying to figure out how to write my tests in this case I read the method documentation and saw that it throws a RestClientException
. So I tried to write my test like this:
public void shouldThrowNullPasswordExceptionOnPUT() throws Exception {
RestClientException error = assertThrows(RestClientException.class, () -> {
repository.save(new User("kyopon", "123456"));
UpdateUserParams request = new UpdateUserParams();
request.password = null;
testRestTemplate.put(url() + "/kyopon", request);
});
assertTrue(error.getMessage().matches("(.*)" + NullPasswordException.message() + "(.*)"));
}
Naturally the test failed. So I started to google why TestRestTemplate.put
was not raising the exception. I could not find much so I started debugging, and I found out that the reason is the implementation of NoOpResponseErrorHandler
. So I googled again and I came across #7265, where I saw that this behavior is intentional and I should use the exchange
method in my tests:
@bclozel : so how to make assertions on the result of
put()
, given thatTestRestTemplate.put()
just returnsvoid
? As an example, how to distinguish a 200 from a 404 error response in tests with that API? 1@tkruse you can use the
exchange
method and specify the PUT method. 2
I rewrote my test and now is working fine. My question/discussion proposal is: In this situation, how can we enhance the documentation of the class/methods so the fact that a exception will never reach the caller can become obvious at first glance?
Points
- To be fair, this behavior is documented in the class comment: "Convenient alternative of RestTemplate that is suitable for integration tests. They are fault tolerant, and optionally can carry Basic authentication headers". If it is fault tolerant, then it is more or less implied that no exception will be thrown, however I feel like this sentence alone is not enough (Is it?).
- Should we remove the
@throws RestClientException
annotation from the methods? Strictly speaking the method itself does raise a exception, but the exception never reaches the caller. In this case I don't know whether the@throws
annotation should be used or not. - Maybe this issue is only valid for
put
anddelete
cause we cannot get the response body even if it fails? I know that this class does not extendsRestTemplate
, so the implementation of those methods is not mandatory. In this case, should we deprecate them and let a comment to useexchange
instead? Maybe my understanding is lacking but I cannot see much use for those methods as they are now. - Should we leave it as it is? Maybe this is not a issue at all and my case is only result of my own misunderstanding.
I already solved my problem, my motivation and concerns are for begginers and newcomers who are not well versed in testing HTTP requests with Spring (which is precisely my case).
I did not want to just change the documentation and make a pull request out of the blue, so I though that opening a issue to start a discussion was the way to go.
This is my POV as a someone who just started writing unit tests with spring, so I would like to read the opinions from experienced users and core members.
Comment From: wilkinsona
Thanks for raising this and sharing your experience, @crocarneiro. To address your points:
- I think we should describe in more detail what it means to be "fault tolerant", both in the javadoc and in the relevant section of the reference documentation.
- That's a really interesting suggestion. I think we probably should but I'll flag this for team attention to see what the rest of the team thinks.
- If part of a test doesn't care about the response to the
PUT
orDELETE
request, the existingput
anddelete
methods seem useful to me. That said, I don't think it would hurt to update their javadoc to suggest thatexchange
is used when the response is of interest. - I think there's definitely an issue here and plenty of room for improvement in the documentation. Thanks again for sharing your thoughts.
Comment From: wilkinsona
We're going to remove the throws as suggested in 2 above.
Comment From: crocarneiro
@wilkinsona Should I do it? Looks like a good first PR
Comment From: wilkinsona
Yes, please. That'd be great. Feel free just to tackle that or the other parts we've discussed as well. We'll pick up whatever's left prior to merging.
Comment From: crocarneiro
I created the pull request #26227 with the changes in the Javadoc of the class. If there is anything wrong let me know, I'll update the PR ASAP.
Comment From: wilkinsona
Thanks very much, @crocarneiro. I'll close this one in favour of your PR (#26227).