Mock: manage headers using RequestHeaders#706
Conversation
velo
left a comment
velo
left a comment
There was a problem hiding this comment.
I started reviewing it, but the format changes make it hard to me to see the full scope of the changes, could you please run mvn clean install to reformat this PR?
| this.url = url; | ||
| } | ||
|
|
||
| public Builder headers(Map<String, Collection<String>> headers) { |
There was a problem hiding this comment.
Could you please keep the original headers with @Deprecated?
| @RequestLine("POST /repos/{owner}/{repo}/contributors") | ||
| @Body("%7B\"login\":\"{login}\",\"type\":\"{type}\"%7D") | ||
| Contributor create(@Param("owner") String owner, | ||
| @Param("repo") String repo, |
There was a problem hiding this comment.
Looks like you changed the formatting for many classes.... could you please rollback?
running mvn clean install should do it.
There was a problem hiding this comment.
Yep, of course. I'll plug the eclipse formatter plugin to my IDE.
velo
left a comment
velo
left a comment
There was a problem hiding this comment.
LGTM, I just would like to see this remaining format and java 6 changes reverted.
Ping me when ready to merge
| import static org.hamcrest.Matchers.not; | ||
| import static org.hamcrest.Matchers.startsWith; | ||
| import static org.junit.Assert.assertThat; | ||
| import java.nio.charset.StandardCharsets; |
There was a problem hiding this comment.
Ok, i'll undo. I've made this to launch test from IntelliJ directly, because test and source don't get the same language level.
| if (result.size() != times) { | ||
| throw new VerificationAssertionError("Wanted: '%s' to be invoked: '%s' times but got: '%s'!", | ||
| throw new VerificationAssertionError( | ||
| "Wanted: '%s' to be invoked: '%s' times but got: '%s'!", |
There was a problem hiding this comment.
I still see a bunch of No-Op changes...
Would be nice to reduce the number of thoose
| @Before | ||
| public void setup() throws IOException { | ||
| try (InputStream input = getClass().getResourceAsStream("/fixtures/contributors.json")) { | ||
| InputStream input = getClass().getResourceAsStream("/fixtures/contributors.json"); |
|
@velo Sounds good to you ? |
|
Perfect! |
* feat(feign-mock): add RequestHeaders class to manage headers * feat(feign-mock): use google code style formatting * feat(feign-mock): remove system.out * feat(RequestKey): add deprecated headers builder + format code * feat(feign-mock): format pom correctly * feat(feign-mock): format pom correctly * fix(feign-mock): undo some typo and no-op change
* feat(feign-mock): add RequestHeaders class to manage headers * feat(feign-mock): use google code style formatting * feat(feign-mock): remove system.out * feat(RequestKey): add deprecated headers builder + format code * feat(feign-mock): format pom correctly * feat(feign-mock): format pom correctly * fix(feign-mock): undo some typo and no-op change
3 participants
Add the new class
RequestHeadersin order to improve management of request headers when testing.So instead of doing this:
we can do :
or
Moreover, when a request do not match, we will now print the expected headers instead of the numbers of headers.
Before:
Expected Request [GET /myrequest: with 2 headers and no charset], but was Request [GET /myrequest: with 0 headers and no charset]After:
Expected Request [GET /myrequest: with my-header=[val], my-header-2=[val2,val3] headers and no charset], but was Request [GET /myrequest: with no headers and no charset]