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/04 08:51:34 UTC
[GitHub] [incubator-inlong] Greedyu opened a new pull request #2902: [INLONG-2901] [Manager] fix maven tests
Greedyu opened a new pull request #2902:
URL: https://github.com/apache/incubator-inlong/pull/2902
### Title Name: [INLONG-XYZ][Manager] fix maven tests
Fixes #2901
1. Fix the exception that the manager module maven cannot run the test class, and unify the version of the maven-surefire-plugin plugin
2. Remove the skipTests attribute of the manager-web module pom.xml file, otherwise maven compilation will force the test to be executed
### Motivation
Because of the 'Replace MySQL with PostgreSQL' #2784 feature developed, Need to unit test the manager module
### Modifications
After testing, it is found that maven cannot run the test class due to the version of maven-surefire-plugin, which may conflict with other packages of the manager module. Tried multiple versions and found that only 2.21 and below can perform maven tests normally
<img width="862" alt="wecom-temp-b766bbf8b7a7bfb79ae628c9f5442460" src="https://user-images.githubusercontent.com/20356765/156730512-e957936e-becc-4b69-8b3b-b3e24b6be6f3.png">
3.0.0-M5
<img width="1111" alt="wecom-temp-6aee5bcfa56c0cd941dda6bc359f6f25" src="https://user-images.githubusercontent.com/20356765/156730498-75c38439-ad25-486d-976b-c20bb41e457a.png">
--
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
[GitHub] [incubator-inlong] healchow commented on a change in pull request #2902: [INLONG-2901] [Manager] fix maven tests
Posted by GitBox <gi...@apache.org>.
healchow commented on a change in pull request #2902:
URL: https://github.com/apache/incubator-inlong/pull/2902#discussion_r819401007
##########
File path: pom.xml
##########
@@ -117,6 +117,7 @@
<logback.version>1.2.10</logback.version>
<junit.version>4.13.2</junit.version>
<je.version>7.3.7</je.version>
+ <plugin.surefire.version>2.21.0</plugin.surefire.version>
Review comment:
Why not use 2.22.x ?
--
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
[GitHub] [incubator-inlong] Greedyu commented on a change in pull request #2902: [INLONG-2901][Manager] Fix maven tests
Posted by GitBox <gi...@apache.org>.
Greedyu commented on a change in pull request #2902:
URL: https://github.com/apache/incubator-inlong/pull/2902#discussion_r819418816
##########
File path: pom.xml
##########
@@ -117,6 +117,7 @@
<logback.version>1.2.10</logback.version>
<junit.version>4.13.2</junit.version>
<je.version>7.3.7</je.version>
+ <plugin.surefire.version>2.21.0</plugin.surefire.version>
Review comment:
The manager module above 2.22.x will not run test files
<img width="774" alt="image" src="https://user-images.githubusercontent.com/20356765/156739845-fe28c57c-6150-447d-8b31-f64e062ce09f.png">
<img width="782" alt="image" src="https://user-images.githubusercontent.com/20356765/156740110-c9b76a05-4ed1-4a54-8700-92d909045927.png">
--
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
[GitHub] [incubator-inlong] healchow commented on a change in pull request #2902: [INLONG-2901][Manager] Fix maven tests
Posted by GitBox <gi...@apache.org>.
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
[GitHub] [incubator-inlong] Greedyu commented on pull request #2902: [INLONG-2901][Manager] Fix maven tests
Posted by GitBox <gi...@apache.org>.
Greedyu commented on pull request #2902:
URL: https://github.com/apache/incubator-inlong/pull/2902#issuecomment-1059063798
There is a problem with the manager unit test, which should be due to a version problem before. There may not actually be a test file that executes the manager
<img width="741" alt="wecom-temp-1ed2e0bbf4320b8379a508b8da79683c" src="https://user-images.githubusercontent.com/20356765/156752254-938980b1-2869-4fee-83a4-64809aaa9bff.png">
--
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