-
Notifications
You must be signed in to change notification settings - Fork 184
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,9 +25,11 @@ | |
import static org.hamcrest.CoreMatchers.equalTo; | ||
import static org.hamcrest.Matchers.is; | ||
import static org.hamcrest.Matchers.matchesPattern; | ||
import static org.junit.jupiter.api.Assertions.assertEquals; | ||
|
||
@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 commentThe 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 commentThe 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. |
||
|
||
@Test | ||
public void inspectConfiguration() { | ||
|
@@ -107,11 +109,6 @@ public void requestValidation() { | |
|
||
@Test | ||
public void jsonBinding() { | ||
Person person = new Person(); | ||
person.setFirstName("John"); | ||
person.setLastName("Doe"); | ||
person.setAge(64); | ||
|
||
String result = String.format( | ||
"Name: %s %s, Age: %d", | ||
person.getFirstName(), | ||
|
@@ -128,12 +125,20 @@ public void jsonBinding() { | |
} | ||
|
||
@Test | ||
public void xmlBinding() { | ||
Person person = new Person(); | ||
person.setFirstName("John"); | ||
person.setLastName("Doe"); | ||
person.setAge(64); | ||
public void jsonBindingProducer() { | ||
Person respondPerson = RestAssured.given() | ||
.queryParam("port", RestAssured.port) | ||
.get("/rest/producer/binding/mode/json") | ||
.then() | ||
.statusCode(200) | ||
.extract() | ||
.body() | ||
.as(Person.class); | ||
assertEquals(respondPerson, person); | ||
} | ||
|
||
@Test | ||
public void xmlBinding() { | ||
String result = String.format( | ||
"Name: %s %s, Age: %d", | ||
person.getFirstName(), | ||
|
@@ -149,6 +154,19 @@ public void xmlBinding() { | |
.body(equalTo(result)); | ||
} | ||
|
||
@Test | ||
public void xmlBindingProducer() { | ||
Person respondPerson = RestAssured.given() | ||
.queryParam("port", RestAssured.port) | ||
.get("/rest/producer/binding/mode/xml") | ||
.then() | ||
.statusCode(200) | ||
.extract() | ||
.body() | ||
.as(Person.class); | ||
assertEquals(respondPerson, person); | ||
} | ||
|
||
@Test | ||
public void testRestProducer() { | ||
RestAssured.given() | ||
|
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!