Support java 11 http2 client gh-689 #869
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #869 +/- ##
============================================
+ Coverage 77.06% 77.19% +0.13%
- Complexity 575 582 +7
============================================
Files 69 71 +2
Lines 2276 2298 +22
Branches 307 308 +1
============================================
+ Hits 1754 1774 +20
- Misses 355 356 +1
- Partials 167 168 +1
|
OlgaMaciaszek
added
enhancement
|
Fixes gh-689. |
There was a problem hiding this comment.
Hello @galaxy-sea, thanks for submitting the PR. In general, it looks good. I have added some comments - please address. Will still have a second look tomorrow and probably add some more things, so you might want to wait till then with any changes.
...re/src/test/java/org/springframework/cloud/openfeign/FeignHttp2ClientConfigurationTests.java
Outdated
Show resolved
Hide resolved
...eign-core/src/test/java/org/springframework/cloud/openfeign/beans/BeansFeignClientTests.java
Outdated
Show resolved
Hide resolved
.../springframework/cloud/openfeign/loadbalancer/Http2ClientFeignLoadBalancerConfiguration.java
Show resolved
Hide resolved
| Http2Client http2Client = (Http2Client) delegate; | ||
| HttpClient httpClient = getField(http2Client, "client"); | ||
| MockingDetails httpClientDetails = mockingDetails(httpClient); | ||
| assertThat(httpClientDetails.isMock()).isTrue(); |
There was a problem hiding this comment.
What is this test verifying? Why test that a mock is a mock?
There was a problem hiding this comment.
Test HttpClient bean customization
There was a problem hiding this comment.
It would probably be better to create a new HttpClient object (HttpClient.newHttpClient();) in the configuration and verify if it's the right type, rather than creating a mock and verifying that it's a mock.
|
hello @OlgaMaciaszek , Thank you for your advice |
...eign-core/src/test/java/org/springframework/cloud/openfeign/beans/BeansFeignClientTests.java
Outdated
Show resolved
Hide resolved
...eign-core/src/test/java/org/springframework/cloud/openfeign/beans/BeansFeignClientTests.java
Outdated
Show resolved
Hide resolved
.../springframework/cloud/openfeign/loadbalancer/Http2ClientFeignLoadBalancerConfiguration.java
Show resolved
Hide resolved
.../springframework/cloud/openfeign/loadbalancer/Http2ClientFeignLoadBalancerConfiguration.java
Show resolved
Hide resolved
...re/src/test/java/org/springframework/cloud/openfeign/test/Http2ClientConfigurationTests.java
Outdated
Show resolved
Hide resolved
| Http2Client http2Client = (Http2Client) delegate; | ||
| HttpClient httpClient = getField(http2Client, "client"); | ||
| MockingDetails httpClientDetails = mockingDetails(httpClient); | ||
| assertThat(httpClientDetails.isMock()).isTrue(); |
There was a problem hiding this comment.
It would probably be better to create a new HttpClient object (HttpClient.newHttpClient();) in the configuration and verify if it's the right type, rather than creating a mock and verifying that it's a mock.
...re/src/test/java/org/springframework/cloud/openfeign/test/Http2ClientConfigurationTests.java
Outdated
Show resolved
Hide resolved
...openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignAutoConfiguration.java
Outdated
Show resolved
Hide resolved
|
Thanks for the changes @galaxy-sea. This last one still needs to be addressed. |
Support java 11 http2 client gh-689