New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[#3028] Increase test coverage of a binding mode of camel-rest component #3052
Conversation
766b5c9
to
78acc9b
Compare
@ppalaga @jamesnetherton Can someone of you take a look? |
|
||
@QuarkusTest | ||
class RestTest { | ||
private static final Person person = new Person("John", "Doe", 64); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it a requirement to use the same object for json/xml consumer/producer binding ? Otherwise, I would use distinct objects, and maybe with different value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's definitely not a requirement. I just thought it would be a little better to create the object only once. I'll change it as you suggest.
String query = "rest:get:/rest/binding/json/producer" + | ||
"?bindingMode=json" + | ||
"&outType=org.apache.camel.quarkus.component.rest.it.Person" + | ||
"&host=localhost:" + port; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at this, I am wondering why we need a JAX-RS endpoint to test the rest component? This is perhaps a way to pass bindingMode=json
, but it it can also be passed via REST DSL, which we already did before this PR, see https://github.com/apache/camel-quarkus/pull/3052/files#diff-58cbc36531a538cc13ef975a1889ac4ec3b28c54786b5896118916846558a592R64-R71 for JSON and https://github.com/apache/camel-quarkus/pull/3052/files#diff-58cbc36531a538cc13ef975a1889ac4ec3b28c54786b5896118916846558a592R78-R85 for XML.
Is this PR adding anything new given the above two exist?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @ppalaga,
the above two test only Consumer while addition in this PR covers Producer. As you can see within this test coverage result https://fuse-next-jenkins-csb-fuse-qe.apps.ocp4.prod.psi.redhat.com/job/Integration.next/job/camel-quarkus/job/code-coverage/43/Camel_20JaCoCo_20report/org.apache.camel.component.rest/RestProducer.java.html#L272
method createBindingProcessor()
is almost entirely untested. With those changes the coverage increase greatly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for explaining, @vhais!
Resolves #3028