code changes for adding support for form parameter matching #2157
Conversation
src/test/java/com/github/tomakehurst/wiremock/matching/RequestPatternTest.java
Show resolved
Hide resolved
|
FWIW, there is #2153 for providing clear separation between really public APIs and not so ones |
|
incorporated review comment changes |
| @@ -60,6 +60,14 @@ interface Part { | |||
|
|
|||
| QueryParameter queryParameter(String key); | |||
|
|
|||
| default FormParameter formParameter(String key) { | |||
There was a problem hiding this comment.
Can we ditch these defaults and ensure we've implemented these everywhere instead?
There was a problem hiding this comment.
Done. We have implemented these everywhere. Oleg suggested adding this as it might break the implementation for those who might be using this interface as this is part of the library and exposed.
There was a problem hiding this comment.
I feel that it is very unlikely that anyone using it as we are using this interface as the base for our receiving requests, matching etc it wherever required. Consumers of Wiremock may not be using it for any of these purposes. But it is always better to add a safety net.
There was a problem hiding this comment.
Something weird has happened to the formatting here so I can't really see what's actually changed. Please can you restore the formatting and add just your change.
There was a problem hiding this comment.
I just added my change but spotless check started failing. It was very minor test so I just removed it for now. Let me know if you have any work around to fix this.
|
Apart from the couple of minor comments above, looks good. Happy to merge this once these are resolved and Oleg's comment on the doc PR is also resolved. |
1 similar comment
|
Apart from the couple of minor comments above, looks good. Happy to merge this once these are resolved and Oleg's comment on the doc PR is also resolved. |
|
Looks like you've deleted your changes to We need some serialisation/deserialisation tests, which I think belong in there. |
as discussed, first applied formatting changes followed by added new UTs. |
| actualJson, | ||
| true); | ||
| } | ||
|
|
There was a problem hiding this comment.
Please can we also have a test case showing that serialisation works correctly?
There was a problem hiding this comment.
sure, incorporated
There was a problem hiding this comment.
I believe you mean deserialization. I have now tests for both - serialization and deserialization.
|
Great stuff, thanks! |
Resolves #383
Code changes for adding support for form parameter matching.
Added tests for mentioned scenarios: