You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@inlong.apache.org by GitBox <gi...@apache.org> on 2022/03/24 04:01:06 UTC

[GitHub] [incubator-inlong] healchow commented on a change in pull request #2902: [INLONG-2901][Manager] Fix maven tests

healchow commented on a change in pull request #2902:
URL: https://github.com/apache/incubator-inlong/pull/2902#discussion_r833893267



##########
File path: inlong-manager/manager-service/src/main/java/org/apache/inlong/manager/service/thirdparty/mq/PulsarEventSelector.java
##########
@@ -38,6 +38,9 @@ public boolean accept(WorkflowContext context) {
         String mqType = form.getGroupInfo().getMiddlewareType();
         if (Constant.MIDDLEWARE_PULSAR.equals(mqType) || Constant.MIDDLEWARE_TDMQ_PULSAR.equals(mqType)) {
             InlongGroupPulsarInfo pulsarInfo = (InlongGroupPulsarInfo) form.getGroupInfo().getMqExtInfo();
+            if (pulsarInfo == null) {

Review comment:
       Should throw an exception as the pulsar info cannot be null.

##########
File path: inlong-manager/manager-service/src/test/java/org/apache/inlong/manager/service/core/impl/ConsumptionServiceTest.java
##########
@@ -55,25 +55,24 @@ private Integer saveConsumption(String inlongGroup, String consumerGroup, String
         return consumptionService.save(consumptionInfo, operator);
     }
 
-    @Test
+//    @Test

Review comment:
       Why do not use the Test?

##########
File path: inlong-manager/manager-common/src/main/java/org/apache/inlong/manager/common/enums/GroupState.java
##########
@@ -59,7 +59,8 @@
      */
     static {
         GROUP_STATE_AUTOMATON.put(DRAFT, Sets.newHashSet(DRAFT, TO_BE_SUBMIT, DELETING));
-        GROUP_STATE_AUTOMATON.put(TO_BE_SUBMIT, Sets.newHashSet(TO_BE_SUBMIT, TO_BE_APPROVAL, DELETING));
+        GROUP_STATE_AUTOMATON.put(TO_BE_SUBMIT,

Review comment:
       The status `TO_BE_SUBMIT` should not transfer to `CONFIG_ING`.

##########
File path: inlong-manager/manager-service/src/test/java/org/apache/inlong/manager/service/ServiceBaseTest.java
##########
@@ -27,6 +28,11 @@
 
     public final String globalGroupId = "b_group1";
     public final String globalStreamId = "stream1";
+    public final String globalStreamName = "streamName1";
     public final String globalOperator = "admin";
 
+    @Test
+    public void test() {
+        System.out.println("The test class cannot be empty, otherwise No runnable methods exception will be reported");

Review comment:
       Add another test, use `Assert`, not `System.out.println`.

##########
File path: inlong-manager/manager-common/src/main/java/org/apache/inlong/manager/common/enums/SourceState.java
##########
@@ -89,7 +89,7 @@
 
     static {
         // new
-        SOURCE_STATE_AUTOMATON.put(SOURCE_NEW, Sets.newHashSet(SOURCE_NEW, TO_BE_ISSUED_ADD));
+        SOURCE_STATE_AUTOMATON.put(SOURCE_NEW, Sets.newHashSet(SOURCE_NEW, TO_BE_ISSUED_ADD, TO_BE_ISSUED_DELETE));

Review comment:
       The status `SOURCE_NEW` should not transfer to `TO_BE_ISSUED_DELETE`, as the `SOURCE_NEW` means the source was not issued, so it does not need to delete.




-- 
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@inlong.apache.org

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