You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@eventmesh.apache.org by GitBox <gi...@apache.org> on 2022/05/15 13:16:13 UTC

[GitHub] [incubator-eventmesh] HoffmanZheng opened a new pull request, #867: [ISSUE-856] Add test code for module [eventmesh-trace-plugin]

HoffmanZheng opened a new pull request, #867:
URL: https://github.com/apache/incubator-eventmesh/pull/867

   Fixes ISSUE #856.
   
   ### Motivation
   
   Add test code for eventmesh-trace-plugin module.
   
   ### Modifications
   
   Add unit test for `org/apache/eventmesh/trace/api/TracePluginFactory.java`.
   
   Add unit test for `org/apache/eventmesh/trace/zipkin/config/ZipkinConfiguration.java`.
   
   ### Documentation
   
   Not a new feature, thus no doc needed.
   


-- 
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: dev-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org

[GitHub] [incubator-eventmesh] codecov[bot] commented on pull request #867: [ISSUE-856] Add test code for module [eventmesh-trace-plugin]

Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #867:
URL: https://github.com/apache/incubator-eventmesh/pull/867#issuecomment-1128334913

   # [Codecov](https://codecov.io/gh/apache/incubator-eventmesh/pull/867?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#867](https://codecov.io/gh/apache/incubator-eventmesh/pull/867?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9919771) into [master](https://codecov.io/gh/apache/incubator-eventmesh/commit/836e3a5bd6a25c9831b0c6a8db30c997c68f0e91?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (836e3a5) will **increase** coverage by `0.20%`.
   > The diff coverage is `100.00%`.
   
   ```diff
   @@             Coverage Diff             @@
   ##             master    #867      +/-   ##
   ===========================================
   + Coverage      6.85%   7.05%   +0.20%     
   - Complexity      433     441       +8     
   ===========================================
     Files           345     349       +4     
     Lines         21619   21723     +104     
     Branches       2404    2413       +9     
   ===========================================
   + Hits           1481    1532      +51     
   - Misses        20025   20073      +48     
   - Partials        113     118       +5     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-eventmesh/pull/867?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...che/eventmesh/trace/zipkin/ZipkinTraceService.java](https://codecov.io/gh/apache/incubator-eventmesh/pull/867/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXZlbnRtZXNoLXRyYWNlLXBsdWdpbi9ldmVudG1lc2gtdHJhY2Utemlwa2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9ldmVudG1lc2gvdHJhY2Uvemlwa2luL1ppcGtpblRyYWNlU2VydmljZS5qYXZh) | `93.93% <100.00%> (ø)` | |
   | [...ava/org/apache/eventmesh/common/utils/IPUtils.java](https://codecov.io/gh/apache/incubator-eventmesh/pull/867/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXZlbnRtZXNoLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZXZlbnRtZXNoL2NvbW1vbi91dGlscy9JUFV0aWxzLmphdmE=) | `33.33% <0.00%> (-6.49%)` | :arrow_down: |
   | [...che/eventmesh/runtime/boot/AbstractHTTPServer.java](https://codecov.io/gh/apache/incubator-eventmesh/pull/867/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXZlbnRtZXNoLXJ1bnRpbWUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2V2ZW50bWVzaC9ydW50aW1lL2Jvb3QvQWJzdHJhY3RIVFRQU2VydmVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [...che/eventmesh/runtime/boot/EventMeshTCPServer.java](https://codecov.io/gh/apache/incubator-eventmesh/pull/867/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXZlbnRtZXNoLXJ1bnRpbWUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2V2ZW50bWVzaC9ydW50aW1lL2Jvb3QvRXZlbnRNZXNoVENQU2VydmVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [...sh/client/http/producer/EventMeshHttpProducer.java](https://codecov.io/gh/apache/incubator-eventmesh/pull/867/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXZlbnRtZXNoLXNkay1qYXZhL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9ldmVudG1lc2gvY2xpZW50L2h0dHAvcHJvZHVjZXIvRXZlbnRNZXNoSHR0cFByb2R1Y2VyLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [...core/protocol/http/consumer/EventMeshConsumer.java](https://codecov.io/gh/apache/incubator-eventmesh/pull/867/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXZlbnRtZXNoLXJ1bnRpbWUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2V2ZW50bWVzaC9ydW50aW1lL2NvcmUvcHJvdG9jb2wvaHR0cC9jb25zdW1lci9FdmVudE1lc2hDb25zdW1lci5qYXZh) | `0.00% <0.00%> (ø)` | |
   | [.../protocol/tcp/client/group/ClientGroupWrapper.java](https://codecov.io/gh/apache/incubator-eventmesh/pull/867/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXZlbnRtZXNoLXJ1bnRpbWUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2V2ZW50bWVzaC9ydW50aW1lL2NvcmUvcHJvdG9jb2wvdGNwL2NsaWVudC9ncm91cC9DbGllbnRHcm91cFdyYXBwZXIuamF2YQ==) | `0.00% <0.00%> (ø)` | |
   | [...ol/tcp/client/session/send/UpStreamMsgContext.java](https://codecov.io/gh/apache/incubator-eventmesh/pull/867/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXZlbnRtZXNoLXJ1bnRpbWUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2V2ZW50bWVzaC9ydW50aW1lL2NvcmUvcHJvdG9jb2wvdGNwL2NsaWVudC9zZXNzaW9uL3NlbmQvVXBTdHJlYW1Nc2dDb250ZXh0LmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [...pl/consumer/ConsumeMessageConcurrentlyService.java](https://codecov.io/gh/apache/incubator-eventmesh/pull/867/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXZlbnRtZXNoLWNvbm5lY3Rvci1wbHVnaW4vZXZlbnRtZXNoLWNvbm5lY3Rvci1yb2NrZXRtcS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvY2xpZW50L2ltcGwvY29uc3VtZXIvQ29uc3VtZU1lc3NhZ2VDb25jdXJyZW50bHlTZXJ2aWNlLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [...tmesh/trace/zipkin/config/ZipkinConfiguration.java](https://codecov.io/gh/apache/incubator-eventmesh/pull/867/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXZlbnRtZXNoLXRyYWNlLXBsdWdpbi9ldmVudG1lc2gtdHJhY2Utemlwa2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9ldmVudG1lc2gvdHJhY2Uvemlwa2luL2NvbmZpZy9aaXBraW5Db25maWd1cmF0aW9uLmphdmE=) | `70.58% <0.00%> (ø)` | |
   | ... and [3 more](https://codecov.io/gh/apache/incubator-eventmesh/pull/867/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-eventmesh/pull/867?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-eventmesh/pull/867?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [836e3a5...9919771](https://codecov.io/gh/apache/incubator-eventmesh/pull/867?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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: dev-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org

[GitHub] [incubator-eventmesh] ruanwenjun merged pull request #867: [ISSUE-856] Add test code for module [eventmesh-trace-plugin]

Posted by GitBox <gi...@apache.org>.
ruanwenjun merged PR #867:
URL: https://github.com/apache/incubator-eventmesh/pull/867


-- 
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: dev-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org

[GitHub] [incubator-eventmesh] xiaoyang-sde commented on pull request #867: [ISSUE-856] Add test code for module [eventmesh-trace-plugin]

Posted by GitBox <gi...@apache.org>.
xiaoyang-sde commented on PR #867:
URL: https://github.com/apache/incubator-eventmesh/pull/867#issuecomment-1127113922

   Welcome and thanks for your contribution!


-- 
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: dev-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org

[GitHub] [incubator-eventmesh] ruanwenjun commented on a diff in pull request #867: [ISSUE-856] Add test code for module [eventmesh-trace-plugin]

Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on code in PR #867:
URL: https://github.com/apache/incubator-eventmesh/pull/867#discussion_r875725621


##########
eventmesh-trace-plugin/eventmesh-trace-zipkin/src/main/java/org/apache/eventmesh/trace/zipkin/ZipkinTraceService.java:
##########
@@ -92,7 +94,8 @@ public void init() {
             .setTracerProvider(sdkTracerProvider)
             .build();
 
-        Runtime.getRuntime().addShutdownHook(new Thread(sdkTracerProvider::close));
+        shutdownHook = new Thread(sdkTracerProvider::close);
+        Runtime.getRuntime().addShutdownHook(shutdownHook);

Review Comment:
   OK, I got it.



-- 
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: dev-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org

[GitHub] [incubator-eventmesh] ruanwenjun commented on pull request #867: [ISSUE-856] Add test code for module [eventmesh-trace-plugin]

Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on PR #867:
URL: https://github.com/apache/incubator-eventmesh/pull/867#issuecomment-1128483002

   > > `eventmesh-trace-plugin/eventmesh-trace-zipkin/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker`, @HoffmanZheng hi, you need add the apache license header for this file.
   > 
   > Once I add the license head, the Mockito test failed. As I mentioned above, this file might be **single line** to enable the incubating feature of Mockito (for our situation, to mock the final class). ![1652762657839](https://user-images.githubusercontent.com/45369893/168730850-7be24126-120c-4d11-936e-4b3c5d59289d.jpg)
   
   If so, I suggest that we can use `PowerMock` to mock the final class, since we must contain the license header in our source file.


-- 
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: dev-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org

[GitHub] [incubator-eventmesh] ruanwenjun commented on a diff in pull request #867: [ISSUE-856] Add test code for module [eventmesh-trace-plugin]

Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on code in PR #867:
URL: https://github.com/apache/incubator-eventmesh/pull/867#discussion_r875065467


##########
eventmesh-trace-plugin/eventmesh-trace-zipkin/src/main/java/org/apache/eventmesh/trace/zipkin/ZipkinTraceService.java:
##########
@@ -92,7 +94,8 @@ public void init() {
             .setTracerProvider(sdkTracerProvider)
             .build();
 
-        Runtime.getRuntime().addShutdownHook(new Thread(sdkTracerProvider::close));
+        shutdownHook = new Thread(sdkTracerProvider::close);
+        Runtime.getRuntime().addShutdownHook(shutdownHook);

Review Comment:
   I don't think this change is needed.



##########
eventmesh-trace-plugin/eventmesh-trace-zipkin/src/test/java/org/apache/eventmesh/trace/zipkin/ZipkinTraceServiceTest.java:
##########
@@ -0,0 +1,70 @@
+/*
+ * 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.eventmesh.trace.zipkin;
+
+import static org.junit.Assert.assertThrows;
+
+import java.lang.reflect.Field;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import io.opentelemetry.api.OpenTelemetry;
+import io.opentelemetry.sdk.trace.SdkTracerProvider;
+
+public class ZipkinTraceServiceTest {
+
+    @Test
+    public void testInit() throws NoSuchFieldException, IllegalAccessException {
+        ZipkinTraceService zipkinTraceService = new ZipkinTraceService();
+        zipkinTraceService.init();
+
+        Field sdkTracerProviderField = ZipkinTraceService.class.getDeclaredField("sdkTracerProvider");
+        Field openTelemetryField = ZipkinTraceService.class.getDeclaredField("openTelemetry");
+        Field shutdownHookField = ZipkinTraceService.class.getDeclaredField("shutdownHook");
+        sdkTracerProviderField.setAccessible(true);
+        openTelemetryField.setAccessible(true);
+        shutdownHookField.setAccessible(true);

Review Comment:
   The better way is to change the visibility in `ZipkinTraceService` rather than use reflect.



-- 
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: dev-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org

[GitHub] [incubator-eventmesh] HoffmanZheng commented on a diff in pull request #867: [ISSUE-856] Add test code for module [eventmesh-trace-plugin]

Posted by GitBox <gi...@apache.org>.
HoffmanZheng commented on code in PR #867:
URL: https://github.com/apache/incubator-eventmesh/pull/867#discussion_r875393725


##########
eventmesh-trace-plugin/eventmesh-trace-zipkin/src/main/java/org/apache/eventmesh/trace/zipkin/ZipkinTraceService.java:
##########
@@ -92,7 +94,8 @@ public void init() {
             .setTracerProvider(sdkTracerProvider)
             .build();
 
-        Runtime.getRuntime().addShutdownHook(new Thread(sdkTracerProvider::close));
+        shutdownHook = new Thread(sdkTracerProvider::close);
+        Runtime.getRuntime().addShutdownHook(shutdownHook);

Review Comment:
   ^ to keep the reference to the shutdownHookThread, thus the behavior of `addShutdownHook` could be tested (it raise `Hook previously registered` when the same shutdownHookThread is added at second time).
   
   https://github.com/apache/incubator-eventmesh/blob/6ea8944b8c91314b296acbb9ef107902c32b084d/eventmesh-trace-plugin/eventmesh-trace-zipkin/src/test/java/org/apache/eventmesh/trace/zipkin/ZipkinTraceServiceTest.java#L47-L55
   
   the `new Thread(sdkTracerProvider::close)` is not the same shutdownHookThread as previous, it could be added successfully and doesn't raise any exception (thus the behavior of `addShutdownHook` could not be tested).
   
   https://github.com/apache/incubator-eventmesh/blob/836e3a5bd6a25c9831b0c6a8db30c997c68f0e91/eventmesh-trace-plugin/eventmesh-trace-zipkin/src/main/java/org/apache/eventmesh/trace/zipkin/ZipkinTraceService.java#L95



-- 
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: dev-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org

[GitHub] [incubator-eventmesh] ruanwenjun commented on a diff in pull request #867: [ISSUE-856] Add test code for module [eventmesh-trace-plugin]

Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on code in PR #867:
URL: https://github.com/apache/incubator-eventmesh/pull/867#discussion_r875410965


##########
eventmesh-trace-plugin/eventmesh-trace-zipkin/src/main/java/org/apache/eventmesh/trace/zipkin/ZipkinTraceService.java:
##########
@@ -92,7 +94,8 @@ public void init() {
             .setTracerProvider(sdkTracerProvider)
             .build();
 
-        Runtime.getRuntime().addShutdownHook(new Thread(sdkTracerProvider::close));
+        shutdownHook = new Thread(sdkTracerProvider::close);
+        Runtime.getRuntime().addShutdownHook(shutdownHook);

Review Comment:
   In your assert, you add a shutdown hook twice, then it throws an exception. Does this test make sense? 



-- 
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: dev-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org

[GitHub] [incubator-eventmesh] HoffmanZheng commented on pull request #867: [ISSUE-856] Add test code for module [eventmesh-trace-plugin]

Posted by GitBox <gi...@apache.org>.
HoffmanZheng commented on PR #867:
URL: https://github.com/apache/incubator-eventmesh/pull/867#issuecomment-1127119225

   There is no exposed behavior in [org/apache/eventmesh/trace/api/config/ExporterConfiguration.java](https://github.com/apache/incubator-eventmesh/blob/master/eventmesh-trace-plugin/eventmesh-trace-api/src/main/java/org/apache/eventmesh/trace/api/config/ExporterConfiguration.java).
   Not sure whether it is necessary to make `initializeConfig()` and `loadProperties()` protected (package private), and add test for them.


-- 
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: dev-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org

[GitHub] [incubator-eventmesh] xwm1992 commented on pull request #867: [ISSUE-856] Add test code for module [eventmesh-trace-plugin]

Posted by GitBox <gi...@apache.org>.
xwm1992 commented on PR #867:
URL: https://github.com/apache/incubator-eventmesh/pull/867#issuecomment-1128342575

   `eventmesh-trace-plugin/eventmesh-trace-zipkin/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker`, @HoffmanZheng hi, you need add the apache license header for this file.
   


-- 
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: dev-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org

[GitHub] [incubator-eventmesh] HoffmanZheng commented on pull request #867: [ISSUE-856] Add test code for module [eventmesh-trace-plugin]

Posted by GitBox <gi...@apache.org>.
HoffmanZheng commented on PR #867:
URL: https://github.com/apache/incubator-eventmesh/pull/867#issuecomment-1128407366

   > `eventmesh-trace-plugin/eventmesh-trace-zipkin/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker`, @HoffmanZheng hi, you need add the apache license header for this file.
   
   Once I add the license head, the Mockito test failed. As I mentioned above, this file might be single line to enable the incubating feature of Mockito (for our situation, to mock the final class).
   ![1652762657839](https://user-images.githubusercontent.com/45369893/168730850-7be24126-120c-4d11-936e-4b3c5d59289d.jpg)
   
   


-- 
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: dev-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org

[GitHub] [incubator-eventmesh] HoffmanZheng commented on pull request #867: [ISSUE-856] Add test code for module [eventmesh-trace-plugin]

Posted by GitBox <gi...@apache.org>.
HoffmanZheng commented on PR #867:
URL: https://github.com/apache/incubator-eventmesh/pull/867#issuecomment-1127442988

   It seems that mocking a final class is [still a incubating feature](https://github.com/mockito/mockito/wiki/What%27s-new-in-Mockito-2#mock-the-unmockable-opt-in-mocking-of-final-classesmethods) of Mockito, which need a [extra file with single line](https://github.com/apache/incubator-eventmesh/blob/f7c49017a349d8d4453e3fab3b1419efa84f55f8/eventmesh-trace-plugin/eventmesh-trace-zipkin/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker) to enable this feature. 
   However the additional file violate the [ci license check](https://github.com/apache/incubator-eventmesh/runs/6444747775?check_suite_focus=true), any possible to skip license check on this file ?
   Otherwise i may remove the unit test for [zipkinTrackService.shutdown()](https://github.com/apache/incubator-eventmesh/blob/f7c49017a349d8d4453e3fab3b1419efa84f55f8/eventmesh-trace-plugin/eventmesh-trace-zipkin/src/test/java/org/apache/eventmesh/trace/zipkin/ZipkinTraceServiceTest.java#L62-L73).
   
   
   


-- 
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: dev-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org

[GitHub] [incubator-eventmesh] HoffmanZheng commented on pull request #867: [ISSUE-856] Add test code for module [eventmesh-trace-plugin]

Posted by GitBox <gi...@apache.org>.
HoffmanZheng commented on PR #867:
URL: https://github.com/apache/incubator-eventmesh/pull/867#issuecomment-1128886964

   > > > `eventmesh-trace-plugin/eventmesh-trace-zipkin/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker`, @HoffmanZheng hi, you need add the apache license header for this file.
   > > 
   > > 
   > > Once I add the license head, the Mockito test failed. As I mentioned above, this file might be **single line** to enable the incubating feature of Mockito (for our situation, to mock the final class). ![1652762657839](https://user-images.githubusercontent.com/45369893/168730850-7be24126-120c-4d11-936e-4b3c5d59289d.jpg)
   > 
   > If so, I suggest that we can use `PowerMock` to mock the final class, since we must contain the license header in our source file.
   
   didn't find the solution how PowerMock mocks the final class, but found this to solve the problem.
   https://frontbackend.com/java/mocking-final-method-with-mockito-framework


-- 
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: dev-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org

[GitHub] [incubator-eventmesh] HoffmanZheng commented on a diff in pull request #867: [ISSUE-856] Add test code for module [eventmesh-trace-plugin]

Posted by GitBox <gi...@apache.org>.
HoffmanZheng commented on code in PR #867:
URL: https://github.com/apache/incubator-eventmesh/pull/867#discussion_r875452897


##########
eventmesh-trace-plugin/eventmesh-trace-zipkin/src/main/java/org/apache/eventmesh/trace/zipkin/ZipkinTraceService.java:
##########
@@ -92,7 +94,8 @@ public void init() {
             .setTracerProvider(sdkTracerProvider)
             .build();
 
-        Runtime.getRuntime().addShutdownHook(new Thread(sdkTracerProvider::close));
+        shutdownHook = new Thread(sdkTracerProvider::close);
+        Runtime.getRuntime().addShutdownHook(shutdownHook);

Review Comment:
   > add a shutdown hook twice, then it throws an exception
   
   the shutdown hook is already added in the `zipkinTraceService.init();`, thus it throws exception in test as expected.
   
   It ensures the `addShutdownHook` is called in the `init()` method, and the `Thread(sdkTracerProvider::close)` is added there. If someone change the behavior of that (e.g. remove the whole line `Runtime.getRuntime().addShutdownHook(new Thread(sdkTracerProvider::close));`), the unit test would fail.
   



-- 
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: dev-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org

[GitHub] [incubator-eventmesh] HoffmanZheng commented on a diff in pull request #867: [ISSUE-856] Add test code for module [eventmesh-trace-plugin]

Posted by GitBox <gi...@apache.org>.
HoffmanZheng commented on code in PR #867:
URL: https://github.com/apache/incubator-eventmesh/pull/867#discussion_r875829333


##########
eventmesh-trace-plugin/eventmesh-trace-zipkin/src/test/java/org/apache/eventmesh/trace/zipkin/ZipkinTraceServiceTest.java:
##########
@@ -0,0 +1,70 @@
+/*
+ * 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.eventmesh.trace.zipkin;
+
+import static org.junit.Assert.assertThrows;
+
+import java.lang.reflect.Field;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import io.opentelemetry.api.OpenTelemetry;
+import io.opentelemetry.sdk.trace.SdkTracerProvider;
+
+public class ZipkinTraceServiceTest {
+
+    @Test
+    public void testInit() throws NoSuchFieldException, IllegalAccessException {
+        ZipkinTraceService zipkinTraceService = new ZipkinTraceService();
+        zipkinTraceService.init();
+
+        Field sdkTracerProviderField = ZipkinTraceService.class.getDeclaredField("sdkTracerProvider");
+        Field openTelemetryField = ZipkinTraceService.class.getDeclaredField("openTelemetry");
+        Field shutdownHookField = ZipkinTraceService.class.getDeclaredField("shutdownHook");
+        sdkTracerProviderField.setAccessible(true);
+        openTelemetryField.setAccessible(true);
+        shutdownHookField.setAccessible(true);

Review Comment:
   Do you mean change the modifier of these member variable from private to `protected` ?
   That makes sense to me, the test code located in the same package of `ZipkinTraceService `, which makes the protected member variable also accessible.



-- 
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: dev-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org

[GitHub] [incubator-eventmesh] ruanwenjun commented on a diff in pull request #867: [ISSUE-856] Add test code for module [eventmesh-trace-plugin]

Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on code in PR #867:
URL: https://github.com/apache/incubator-eventmesh/pull/867#discussion_r877084389


##########
eventmesh-trace-plugin/eventmesh-trace-zipkin/src/test/java/org/apache/eventmesh/trace/zipkin/ZipkinTraceServiceTest.java:
##########
@@ -0,0 +1,70 @@
+/*
+ * 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.eventmesh.trace.zipkin;
+
+import static org.junit.Assert.assertThrows;
+
+import java.lang.reflect.Field;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import io.opentelemetry.api.OpenTelemetry;
+import io.opentelemetry.sdk.trace.SdkTracerProvider;
+
+public class ZipkinTraceServiceTest {
+
+    @Test
+    public void testInit() throws NoSuchFieldException, IllegalAccessException {
+        ZipkinTraceService zipkinTraceService = new ZipkinTraceService();
+        zipkinTraceService.init();
+
+        Field sdkTracerProviderField = ZipkinTraceService.class.getDeclaredField("sdkTracerProvider");
+        Field openTelemetryField = ZipkinTraceService.class.getDeclaredField("openTelemetry");
+        Field shutdownHookField = ZipkinTraceService.class.getDeclaredField("shutdownHook");
+        sdkTracerProviderField.setAccessible(true);
+        openTelemetryField.setAccessible(true);
+        shutdownHookField.setAccessible(true);

Review Comment:
   Yes.



-- 
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: dev-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org