When using RestClient it's possible to submit a request that's expected to succeed via
restClient.post()
.uri("/foo/bar")
.body(requestData)
.retrieve();
With a RestTestClient the equivalent is
restTestClient.post()
.uri("/foo/bar")
.body(requestData)
.exchange()
.expectStatus().is2xxSuccessful();
Could we add a correspondingretrieve() method to the RestTestClient API? Not only would it make the code a bit more concise for the very common case of submitting a request that's expected to succeed, but it would simplify migrating from RestClient to RestTestClient
Comment From: rstoyanchev
I get what you're asking but I don't think adding retrieve is a good fit. retrieve vs exchange is about working directly with the response vs using a workflow that also has to check the status because you're not working directly with the response.
The testing case doesn't fit into that. If we added retrieve it would be reasonable to expect that we also call retrieve on the internal RestClient, and therefore that configured status handlers would apply. However, we need to always call exchange to handle the response in a custom way for testing purposes.
What we could do is accept a Consumer<StatusAssertions> at build time that should be applied on every request:
RestTestClient.bindToController(..)
.alwaysExpectStatus(statusAssertions -> ...)
.build();
Comment From: donalmurtagh
The testing case doesn't fit into that. If we added
retrieveit would be reasonable to expect that we also callretrieveon the internalRestClient, and therefore that configured status handlers would apply. However, we need to always callexchangeto handle the response in a custom way for testing purposes.
That's not how I would expect retrieve to be implemented. I would expect it to look like this:
interface RequestHeadersSpec<S extends RequestHeadersSpec<S>> {
/////////////////////////////////////////////////////////////////
// other methods of this interface omitted
/////////////////////////////////////////////////////////////////
/**
* Perform the exchange without a request body.
*
* @return a spec for decoding the response
*/
ResponseSpec exchange();
/**
* Perform the exchange with an expectation that it will succeed. An exception will be thrown
* if a HTTP status code outside the 2xx range is returned.
*
* @return a spec for decoding the response
*/
default ResponseSpec retrieve() {
return exchange().expectStatus().is2xxSuccessful();
}
}
Perhaps the choice of a default interface method is not the best, but that's a side issue.
What we could do is accept a
Consumer<StatusAssertions>at build time that should be applied on every request:RestTestClient.bindToController(..) .alwaysExpectStatus(statusAssertions -> ...) .build();
I wouldn't find this useful at all. In my case I build a single RestTestClient that is shared throughout my test suite. Some tests expect success and some expect failure, so it doesn't make any sense (for me) to specify the expected outcome at build-time.
Comment From: rstoyanchev
retrieve in RestClient works differently, and that can create different expectations from yours.
Essentially we're talking about a shortcut for exchange().expectStatus().is2xxSuccessful(). Given that a majority of tests are for successful responses, we could add an overload of exchange() called exchangeSuccessfully(). Longer than retrieve, but with code completion it's no different, and it's very clear what it does.
Comment From: donalmurtagh
Essentially we're talking about a shortcut for
exchange().expectStatus().is2xxSuccessful(). Given that a majority of tests are for successful responses, we could add an overload ofexchange()calledexchangeSuccessfully(). Longer thanretrieve, but with code completion it's no different, and it's very clear what it does.
Sounds good to me. I agree that exchangeSuccessfully is a better name than retrieve.
Comment From: darth-raijin
Would this be a good first issue for new contributors?
Comment From: donalmurtagh
Would this be a good first issue for new contributors?
As @rstoyanchev self-assigned this issue, I assume this means he intends to implement the changes