You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by "avano (via GitHub)" <gi...@apache.org> on 2023/04/27 07:18:38 UTC
[GitHub] [camel-quarkus] avano opened a new pull request, #4838: [MapStruct] Add tests
avano opened a new pull request, #4838:
URL: https://github.com/apache/camel-quarkus/pull/4838
Adds missing tests for mapstruct extension
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-quarkus] avano commented on a diff in pull request #4838: [MapStruct] Add tests
Posted by "avano (via GitHub)" <gi...@apache.org>.
avano commented on code in PR #4838:
URL: https://github.com/apache/camel-quarkus/pull/4838#discussion_r1179997912
##########
integration-tests-jvm/mapstruct/pom.xml:
##########
@@ -37,17 +37,21 @@
<groupId>org.apache.camel.quarkus</groupId>
<artifactId>camel-quarkus-mapstruct</artifactId>
</dependency>
+ <dependency>
+ <groupId>org.apache.camel.quarkus</groupId>
+ <artifactId>camel-quarkus-direct</artifactId>
+ </dependency>
+
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-resteasy</artifactId>
</dependency>
-
- <!-- test dependencies -->
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-junit5</artifactId>
- <scope>test</scope>
Review Comment:
thanks, I was checking out how similar components are doing this and found [this](https://github.com/apache/camel-quarkus/blob/main/integration-tests/bindy/src/main/java/org/apache/camel/quarkus/component/bindy/it/BindyResource.java#L68) so I took the inspiration from there :grin:
I'll rework according to your comments
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-quarkus] ppalaga commented on pull request #4838: [MapStruct] Add tests
Posted by "ppalaga (via GitHub)" <gi...@apache.org>.
ppalaga commented on PR #4838:
URL: https://github.com/apache/camel-quarkus/pull/4838#issuecomment-1533118878
Thanks a lot @avano!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-quarkus] avano commented on pull request #4838: [MapStruct] Add tests
Posted by "avano (via GitHub)" <gi...@apache.org>.
avano commented on PR #4838:
URL: https://github.com/apache/camel-quarkus/pull/4838#issuecomment-1533107248
the failures are unrelated to these changes, mapstruct tests passed:
```
[INFO] Running org.apache.camel.quarkus.component.mapstruct.it.MapStructTest
2023-05-03 11:10:56,406 INFO [org.apa.cam.qua.cor.CamelBootstrapRecorder] (main) Bootstrap runtime: org.apache.camel.quarkus.main.CamelMainRuntime
2023-05-03 11:10:56,408 INFO [org.apa.cam.mai.MainSupport] (main) Apache Camel (Main) 4.0.0-M2 is starting
2023-05-03 11:10:56,518 INFO [org.apa.cam.imp.eng.AbstractCamelContext] (main) Apache Camel 4.0.0-M2 (camel-1) is starting
2023-05-03 11:10:56,522 INFO [org.apa.cam.com.map.DefaultMapStructFinder] (main) Discovered 1 MapStruct type converters from classpath scanning: org.apache.camel.quarkus.component.mapstruct.it.mapper
2023-05-03 11:10:56,530 INFO [org.apa.cam.imp.eng.AbstractCamelContext] (main) Routes startup (started:2)
2023-05-03 11:10:56,530 INFO [org.apa.cam.imp.eng.AbstractCamelContext] (main) Started route1 (direct://component)
2023-05-03 11:10:56,530 INFO [org.apa.cam.imp.eng.AbstractCamelContext] (main) Started route2 (direct://converter)
2023-05-03 11:10:56,530 INFO [org.apa.cam.imp.eng.AbstractCamelContext] (main) Apache Camel 4.0.0-M2 (camel-1) started in 51ms (build:0ms init:41ms start:10ms)
2023-05-03 11:10:56,614 INFO [io.quarkus] (main) camel-quarkus-integration-test-mapstruct 3.0.0-SNAPSHOT on JVM (powered by Quarkus 3.0.1.Final) started in 2.947s. Listening on: http://localhost:39811
2023-05-03 11:10:56,615 INFO [io.quarkus] (main) Profile test activated.
2023-05-03 11:10:56,615 INFO [io.quarkus] (main) Installed features: [camel-core, camel-direct, camel-mapstruct, cdi, resteasy, smallrye-context-propagation, vertx]
[INFO] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 7.665 s - in org.apache.camel.quarkus.component.mapstruct.it.MapStructTest
2023-05-03 11:10:58,210 INFO [org.apa.cam.imp.eng.AbstractCamelContext] (main) Apache Camel 4.0.0-M2 (camel-1) is shutting down (timeout:45s)
2023-05-03 11:10:58,217 INFO [org.apa.cam.imp.eng.AbstractCamelContext] (main) Routes stopped (stopped:2)
2023-05-03 11:10:58,218 INFO [org.apa.cam.imp.eng.AbstractCamelContext] (main) Stopped route2 (direct://converter)
2023-05-03 11:10:58,218 INFO [org.apa.cam.imp.eng.AbstractCamelContext] (main) Stopped route1 (direct://component)
2023-05-03 11:10:58,221 INFO [org.apa.cam.imp.eng.AbstractCamelContext] (main) Apache Camel 4.0.0-M2 (camel-1) shutdown in 10ms (uptime:1s)
2023-05-03 11:10:58,239 INFO [io.quarkus] (main) camel-quarkus-integration-test-mapstruct stopped in 0.043s
[INFO]
[INFO] Results:
[INFO]
[INFO] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-quarkus] ppalaga merged pull request #4838: [MapStruct] Add tests
Posted by "ppalaga (via GitHub)" <gi...@apache.org>.
ppalaga merged PR #4838:
URL: https://github.com/apache/camel-quarkus/pull/4838
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-quarkus] ppalaga commented on a diff in pull request #4838: [MapStruct] Add tests
Posted by "ppalaga (via GitHub)" <gi...@apache.org>.
ppalaga commented on code in PR #4838:
URL: https://github.com/apache/camel-quarkus/pull/4838#discussion_r1179458521
##########
integration-tests-jvm/mapstruct/pom.xml:
##########
@@ -81,4 +85,22 @@
</dependencies>
</profile>
</profiles>
+
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-compiler-plugin</artifactId>
+ <configuration>
+ <annotationProcessorPaths>
+ <path>
+ <groupId>org.mapstruct</groupId>
+ <artifactId>mapstruct-processor</artifactId>
+ <version>${mapstruct-version}</version>
Review Comment:
We stick to the convention that we always wrap Camel's `foo-version` in our own `foo.version` in the top level pom.xml. This is to have single place where one can see which versions come from where. Some come from Camel, some come from Quarkus, or from elsewhere. So if you could please create `<mapstruct.version>${mapstruct-version}</mapstruct.version>` in the top pom and then use `<version>${mapstruct.version}</version>` here.
##########
integration-tests-jvm/mapstruct/src/test/java/org/apache/camel/quarkus/component/mapstruct/it/MapStructTest.java:
##########
@@ -0,0 +1,35 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.quarkus.component.mapstruct.it;
+
+import io.quarkus.test.junit.QuarkusTest;
+import io.restassured.RestAssured;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+@QuarkusTest
+public class MapStructTest {
+
+ @ParameterizedTest
+ @ValueSource(strings = { "component", "converter" })
Review Comment:
Nice usage of `@ParameterizedTest`!
##########
integration-tests-jvm/mapstruct/pom.xml:
##########
@@ -37,17 +37,21 @@
<groupId>org.apache.camel.quarkus</groupId>
<artifactId>camel-quarkus-mapstruct</artifactId>
</dependency>
+ <dependency>
+ <groupId>org.apache.camel.quarkus</groupId>
+ <artifactId>camel-quarkus-direct</artifactId>
+ </dependency>
+
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-resteasy</artifactId>
</dependency>
-
- <!-- test dependencies -->
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-junit5</artifactId>
- <scope>test</scope>
Review Comment:
I'd vote for keeping junit in the test scope. It is primarily to keep the application under test minimal and as close to end user apps as possible. End users are unlikely to include junit in their production apps. Moreover adding more code makes the native compilation slower and may cause unecessary native compilation issues.
Just pass the data on which you want to do the assertions over HTTP/REST and do the assertions in the test classes. If you look around this is a common practice in Camel Quarkus.
##########
integration-tests-jvm/mapstruct/src/main/java/org/apache/camel/quarkus/component/mapstruct/it/MapStructResource.java:
##########
@@ -0,0 +1,56 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.quarkus.component.mapstruct.it;
+
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.inject.Inject;
+import jakarta.ws.rs.GET;
+import jakarta.ws.rs.Path;
+import org.apache.camel.ProducerTemplate;
+import org.apache.camel.quarkus.component.mapstruct.it.model.Car;
+import org.apache.camel.quarkus.component.mapstruct.it.model.Vehicle;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+@Path("/mapstruct")
+@ApplicationScoped
+public class MapStructResource {
+ private static final Vehicle VEHICLE = new Vehicle("Volvo", "XC60", "true", 2021);
+
+ @Inject
+ ProducerTemplate producerTemplate;
+
+ @Path("/component")
+ @GET
+ public void componentTest() {
+ testMapping("component");
+ }
+
+ @Path("/converter")
+ @GET
+ public void converterTest() {
+ testMapping("converter");
+ }
+
+ private void testMapping(String endpoint) {
+ Car car = producerTemplate.requestBody("direct:" + endpoint, VEHICLE, Car.class);
+ assertEquals(car.getBrand(), VEHICLE.getCompany());
+ assertEquals(car.getModel(), VEHICLE.getName());
+ assertEquals(car.getYear(), VEHICLE.getYear());
+ assertEquals(car.isElectric(), Boolean.parseBoolean(VEHICLE.getPower()));
Review Comment:
As noted above it would be better to have both these assertions and VEHICLE = new Vehicle("Volvo", "XC60", "true", 2021); in the test class. To avoid adding quarkus-resteasy-jackson or quarkus-resteasy-jsonb (because we really do not want any additional dependencies), you can use some add-hoc serialization, like you pass "Volvo,XC60,true,2021" over HTTP and create the Car out of the String.split(",") array.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-quarkus] zbendhiba commented on a diff in pull request #4838: [MapStruct] Add tests
Posted by "zbendhiba (via GitHub)" <gi...@apache.org>.
zbendhiba commented on code in PR #4838:
URL: https://github.com/apache/camel-quarkus/pull/4838#discussion_r1179403714
##########
integration-tests-jvm/mapstruct/src/main/java/org/apache/camel/quarkus/component/mapstruct/it/MapStructRoutes.java:
##########
@@ -0,0 +1,33 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.quarkus.component.mapstruct.it;
+
+import org.apache.camel.builder.RouteBuilder;
+import org.apache.camel.component.mapstruct.MapstructComponent;
+import org.apache.camel.quarkus.component.mapstruct.it.model.Car;
+
+public class MapStructRoutes extends RouteBuilder {
+
+ @Override
+ public void configure() throws Exception {
+ MapstructComponent mc = getContext().getComponent("mapstruct", MapstructComponent.class);
Review Comment:
Maybe we can create a named component, like done here : https://github.com/apache/camel-quarkus/blob/7e59a8b1a53288b9a2af79ad1bfb4d49ab556d87/integration-tests/hazelcast/src/main/java/org/apache/camel/quarkus/component/hazelcast/it/HazelcastRoutes.java#L90
Mapstruct component could be improved in the future to get this out of Component level.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org