You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by GitBox <gi...@apache.org> on 2020/03/11 15:37:26 UTC

[GitHub] [camel-quarkus] aldettinger opened a new pull request #876: Fixed kafka itests as @Inject is not supported in native tests

aldettinger opened a new pull request #876: Fixed kafka itests as @Inject is not supported in native tests
URL: https://github.com/apache/camel-quarkus/pull/876
 
 
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [camel-quarkus] aldettinger commented on a change in pull request #876: Fixed kafka itests as @Inject is not supported in native tests

Posted by GitBox <gi...@apache.org>.
aldettinger commented on a change in pull request #876: Fixed kafka itests as @Inject is not supported in native tests
URL: https://github.com/apache/camel-quarkus/pull/876#discussion_r391116571
 
 

 ##########
 File path: integration-tests/kafka/src/test/java/org/apache/camel/quarkus/component/kafka/it/CamelKafkaTestResource.java
 ##########
 @@ -32,14 +31,6 @@
     private static final String CONFLUENT_PLATFORM_VERSION = "5.3.1";
 
     private KafkaContainer container;
-    private CamelMain main;
-
-    @Override
-    public void inject(Object testInstance) {
-        if (testInstance instanceof CamelKafkaTest) {
-            this.main = ((CamelKafkaTest) testInstance).main;
 
 Review comment:
   For the record, question on [quarkus side](https://github.com/quarkusio/quarkus/issues/7784). ok, I'm gonna check if I can find similar thing elsewhere in the context of another ticket.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [camel-quarkus] aldettinger commented on a change in pull request #876: Fixed kafka itests as @Inject is not supported in native tests

Posted by GitBox <gi...@apache.org>.
aldettinger commented on a change in pull request #876: Fixed kafka itests as @Inject is not supported in native tests
URL: https://github.com/apache/camel-quarkus/pull/876#discussion_r391063903
 
 

 ##########
 File path: integration-tests/kafka/src/test/java/org/apache/camel/quarkus/component/kafka/it/CamelKafkaTestResource.java
 ##########
 @@ -32,14 +31,6 @@
     private static final String CONFLUENT_PLATFORM_VERSION = "5.3.1";
 
     private KafkaContainer container;
-    private CamelMain main;
-
-    @Override
-    public void inject(Object testInstance) {
-        if (testInstance instanceof CamelKafkaTest) {
-            this.main = ((CamelKafkaTest) testInstance).main;
 
 Review comment:
   Note that this line would have been invoked in native mode only due to distinct class loaders. But then @Inject is not authorized.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [camel-quarkus] lburgazzoli commented on a change in pull request #876: Fixed kafka itests as @Inject is not supported in native tests

Posted by GitBox <gi...@apache.org>.
lburgazzoli commented on a change in pull request #876: Fixed kafka itests as @Inject is not supported in native tests
URL: https://github.com/apache/camel-quarkus/pull/876#discussion_r391112104
 
 

 ##########
 File path: integration-tests/kafka/src/test/java/org/apache/camel/quarkus/component/kafka/it/CamelKafkaTestResource.java
 ##########
 @@ -32,14 +31,6 @@
     private static final String CONFLUENT_PLATFORM_VERSION = "5.3.1";
 
     private KafkaContainer container;
-    private CamelMain main;
-
-    @Override
-    public void inject(Object testInstance) {
-        if (testInstance instanceof CamelKafkaTest) {
-            this.main = ((CamelKafkaTest) testInstance).main;
 
 Review comment:
   oh now I realized what the issue, sorry for taking so much time. so this change is ok, there may ne something similar elsewhere 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [camel-quarkus] lburgazzoli commented on a change in pull request #876: Fixed kafka itests as @Inject is not supported in native tests

Posted by GitBox <gi...@apache.org>.
lburgazzoli commented on a change in pull request #876: Fixed kafka itests as @Inject is not supported in native tests
URL: https://github.com/apache/camel-quarkus/pull/876#discussion_r391064334
 
 

 ##########
 File path: integration-tests/kafka/src/test/java/org/apache/camel/quarkus/component/kafka/it/CamelKafkaTestResource.java
 ##########
 @@ -32,14 +31,6 @@
     private static final String CONFLUENT_PLATFORM_VERSION = "5.3.1";
 
     private KafkaContainer container;
-    private CamelMain main;
-
-    @Override
-    public void inject(Object testInstance) {
-        if (testInstance instanceof CamelKafkaTest) {
-            this.main = ((CamelKafkaTest) testInstance).main;
 
 Review comment:
   is that something documented on quarkus side or a bug ? 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [camel-quarkus] aldettinger commented on a change in pull request #876: Fixed kafka itests as @Inject is not supported in native tests

Posted by GitBox <gi...@apache.org>.
aldettinger commented on a change in pull request #876: Fixed kafka itests as @Inject is not supported in native tests
URL: https://github.com/apache/camel-quarkus/pull/876#discussion_r391110003
 
 

 ##########
 File path: integration-tests/kafka/src/test/java/org/apache/camel/quarkus/component/kafka/it/CamelKafkaTestResource.java
 ##########
 @@ -32,14 +31,6 @@
     private static final String CONFLUENT_PLATFORM_VERSION = "5.3.1";
 
     private KafkaContainer container;
-    private CamelMain main;
-
-    @Override
-    public void inject(Object testInstance) {
-        if (testInstance instanceof CamelKafkaTest) {
-            this.main = ((CamelKafkaTest) testInstance).main;
 
 Review comment:
   There must be 2 subjects:
   
   \@Inject not supported in native mode is documented [there](https://github.com/quarkusio/quarkus/blob/master/docs/src/main/asciidoc/getting-started-testing.adoc#9-native-executable-testing), so it could have behaved differently previously but it's no more the case.
   
   `testInstance instanceof CamelKafkaTest == false` because of distinct class loaders, I found no mention, so I would better open a question on quarkus side so that we know what behavior we can rely on.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [camel-quarkus] lburgazzoli commented on a change in pull request #876: Fixed kafka itests as @Inject is not supported in native tests

Posted by GitBox <gi...@apache.org>.
lburgazzoli commented on a change in pull request #876: Fixed kafka itests as @Inject is not supported in native tests
URL: https://github.com/apache/camel-quarkus/pull/876#discussion_r391073260
 
 

 ##########
 File path: integration-tests/kafka/src/test/java/org/apache/camel/quarkus/component/kafka/it/CamelKafkaTestResource.java
 ##########
 @@ -32,14 +31,6 @@
     private static final String CONFLUENT_PLATFORM_VERSION = "5.3.1";
 
     private KafkaContainer container;
-    private CamelMain main;
-
-    @Override
-    public void inject(Object testInstance) {
-        if (testInstance instanceof CamelKafkaTest) {
-            this.main = ((CamelKafkaTest) testInstance).main;
 
 Review comment:
   it looks strange as the pr is about the @Inject annotation

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [camel-quarkus] lburgazzoli commented on a change in pull request #876: Fixed kafka itests as @Inject is not supported in native tests

Posted by GitBox <gi...@apache.org>.
lburgazzoli commented on a change in pull request #876: Fixed kafka itests as @Inject is not supported in native tests
URL: https://github.com/apache/camel-quarkus/pull/876#discussion_r391064334
 
 

 ##########
 File path: integration-tests/kafka/src/test/java/org/apache/camel/quarkus/component/kafka/it/CamelKafkaTestResource.java
 ##########
 @@ -32,14 +31,6 @@
     private static final String CONFLUENT_PLATFORM_VERSION = "5.3.1";
 
     private KafkaContainer container;
-    private CamelMain main;
-
-    @Override
-    public void inject(Object testInstance) {
-        if (testInstance instanceof CamelKafkaTest) {
-            this.main = ((CamelKafkaTest) testInstance).main;
 
 Review comment:
   is that something documented on quarkus side ? 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [camel-quarkus] jamesnetherton commented on a change in pull request #876: Fixed kafka itests as @Inject is not supported in native tests

Posted by GitBox <gi...@apache.org>.
jamesnetherton commented on a change in pull request #876: Fixed kafka itests as @Inject is not supported in native tests
URL: https://github.com/apache/camel-quarkus/pull/876#discussion_r391071126
 
 

 ##########
 File path: integration-tests/kafka/src/test/java/org/apache/camel/quarkus/component/kafka/it/CamelKafkaTestResource.java
 ##########
 @@ -32,14 +31,6 @@
     private static final String CONFLUENT_PLATFORM_VERSION = "5.3.1";
 
     private KafkaContainer container;
-    private CamelMain main;
-
-    @Override
-    public void inject(Object testInstance) {
-        if (testInstance instanceof CamelKafkaTest) {
-            this.main = ((CamelKafkaTest) testInstance).main;
 
 Review comment:
   It was a change introduced in 1.3.0.CR1. See https://github.com/quarkusio/quarkus/commit/2cea862dbab1f9133ff5e0d7d9cdbef11e2d1e27.
   
   I had to make a similar change for the Infinispan tests.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [camel-quarkus] lburgazzoli commented on a change in pull request #876: Fixed kafka itests as @Inject is not supported in native tests

Posted by GitBox <gi...@apache.org>.
lburgazzoli commented on a change in pull request #876: Fixed kafka itests as @Inject is not supported in native tests
URL: https://github.com/apache/camel-quarkus/pull/876#discussion_r391066861
 
 

 ##########
 File path: integration-tests/kafka/src/test/java/org/apache/camel/quarkus/component/kafka/it/CamelKafkaTestResource.java
 ##########
 @@ -32,14 +31,6 @@
     private static final String CONFLUENT_PLATFORM_VERSION = "5.3.1";
 
     private KafkaContainer container;
-    private CamelMain main;
-
-    @Override
-    public void inject(Object testInstance) {
-        if (testInstance instanceof CamelKafkaTest) {
-            this.main = ((CamelKafkaTest) testInstance).main;
 
 Review comment:
   yes please as this was working before and it has been used to avoid having warnings at the end of the tests

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [camel-quarkus] aldettinger commented on a change in pull request #876: Fixed kafka itests as @Inject is not supported in native tests

Posted by GitBox <gi...@apache.org>.
aldettinger commented on a change in pull request #876: Fixed kafka itests as @Inject is not supported in native tests
URL: https://github.com/apache/camel-quarkus/pull/876#discussion_r391065432
 
 

 ##########
 File path: integration-tests/kafka/src/test/java/org/apache/camel/quarkus/component/kafka/it/CamelKafkaTestResource.java
 ##########
 @@ -32,14 +31,6 @@
     private static final String CONFLUENT_PLATFORM_VERSION = "5.3.1";
 
     private KafkaContainer container;
-    private CamelMain main;
-
-    @Override
-    public void inject(Object testInstance) {
-        if (testInstance instanceof CamelKafkaTest) {
-            this.main = ((CamelKafkaTest) testInstance).main;
 
 Review comment:
   Honestly, I don't know but I could take a look.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [camel-quarkus] aldettinger merged pull request #876: Fixed kafka itests as @Inject is not supported in native tests

Posted by GitBox <gi...@apache.org>.
aldettinger merged pull request #876: Fixed kafka itests as @Inject is not supported in native tests
URL: https://github.com/apache/camel-quarkus/pull/876
 
 
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services