spring-projects / spring-framework

Spring Framework
https://spring.io/projects/spring-framework
Apache License 2.0
56.69k stars 38.15k forks source link

Unable to mock HttpHeaders since Spring Framework 5.3 #26509

Closed ERAGON-JCV closed 3 years ago

ERAGON-JCV commented 3 years ago

In unit test with mockito and Junit 5

with version Spring Boot 2.3.4 @Mock HttpHeaders works

image

image

but in version 2.4.X this error appears, apparently Mock HttpHeaders is null or empty in this verification

image

image

with version Spring Boot 2.3.4 this verification was like this and it worked

image

please fix it in new versions, thank you very much.

mbhave commented 3 years ago

@ERAGON-JCV Can you provide a small sample that we can run to reproduce this behavior? It can be a zip or a link to a GitHub repository. I don't think there's anything in Spring Boot itself that would cause this behavior but a sample will help understand what's going on.

ERAGON-JCV commented 3 years ago

Hi, in this repository is the sample https://github.com/ERAGON-JCV/issueHttpsHeaders

is with vesion spring boot 2.4.2, with this version unit test does not pass and this behavior appears, but if you change of version to 2.3.4.RELEASE the unit test passes.

thank you.

mbhave commented 3 years ago

Thanks for the sample @ERAGON-JCV. This appears to be happening because of this change in Spring Framework. Previously a temp HttpHeaders was created, which would, in this case, return an empty MultiValueMap for headers.headers. With the change mentioned, headers.headers returns null for the mock.

Let's see what the Spring Framework team thinks.

sbrannen commented 3 years ago

@ERAGON-JCV, why are you mocking HttpHeaders?

Why not simply use a new instance of HttpHeaders?

For example, if you remove the @Mock private HttpHeaders headers; declaration in ClientServiceTest and then add the following line to your init() method ...

ReflectionTestUtils.setField(clientService, "headers", new HttpHeaders());

... then your test passes with a real, legitimate instance of HttpHeaders.

Of course, we discourage the use of @Autowired fields in production classes, since that makes it harder to test components. Introducing constructors or setter methods that accept the HttpHeaders and urlMonetaryFee would alleviate the need to use ReflectionTestUtils for setting those private fields.

sbrannen commented 3 years ago

Team Decision: The Spring Framework team does its best to ensure stability in public APIs across releases; however, that does not apply to internal implementation details. Mocking a concrete type such as HttpHeaders can lead to a mock that relies on the internal implementation (which is not part of the public API), and the behavior of such a mock can therefore be incompatible with changes to internal implementation details across releases. In light of that, I am closing this issue.

https://github.com/spring-projects/spring-framework/issues/26509#issuecomment-776616369 provides a viable solution by using a real instance of HttpHeaders.

Aside from that, having an @Autowired HttpHeaders dependency in a Spring-managed component seems questionable. Typically a new instance of HttpHeaders would be created for each request submitted via the RestTemplate. If a single, preconfigured instance of HttpHeaders is desirable, one would typically create one local to its usage and not define one as a singleton bean in the ApplicationContext. Thus, you may wish to reconsider your use of a shared HttpHeaders bean in your application.