You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@streampark.apache.org by "tisonkun (via GitHub)" <gi...@apache.org> on 2023/02/22 04:54:54 UTC
[GitHub] [incubator-streampark] tisonkun opened a new pull request, #2353: [Fix] Test classes should be under sp package
tisonkun opened a new pull request, #2353:
URL: https://github.com/apache/incubator-streampark/pull/2353
## What changes were proposed in this pull request
1. Move tests under outside packages to `org.apache.streampark`.
2. Remove `RefreshCacheTest` since third party dependency should be self tested.
## Does this pull request potentially affect one of the following parts
- Dependencies (does it add or upgrade a dependency): (no)
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] tisonkun commented on a diff in pull request #2353: [Fix] Test classes should be under sp package
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on code in PR #2353:
URL: https://github.com/apache/incubator-streampark/pull/2353#discussion_r1113835790
##########
streampark-console/streampark-console-service/src/test/java/org/apache/streampark/common/util/RegexTest.java:
##########
@@ -28,6 +26,8 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
import java.util.stream.Collectors;
class RegexTest {
Review Comment:
BTW, `RegexTest#classLoader` and `RegexTest#flinkVersion` seem quite tricky and fail locally. How can we verify these tests correctly?
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] tisonkun commented on a diff in pull request #2353: [Fix] Test classes should be under sp package
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on code in PR #2353:
URL: https://github.com/apache/incubator-streampark/pull/2353#discussion_r1113836481
##########
streampark-console/streampark-console-service/src/test/java/org/apache/streampark/console/core/bean/SenderEmailTest.java:
##########
@@ -77,7 +77,7 @@ void alert() {
mail.setStatus(appState.name());
StringWriter writer = new StringWriter();
- Map<String, AlertTemplate> out = new HashMap<String, AlertTemplate>();
+ Map<String, AlertTemplate> out = new HashMap<>();
Review Comment:
ditto this test fails locally
... we just swallow the exception below.
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] wolfboys commented on a diff in pull request #2353: [Fix] Test classes should be under sp package
Posted by "wolfboys (via GitHub)" <gi...@apache.org>.
wolfboys commented on code in PR #2353:
URL: https://github.com/apache/incubator-streampark/pull/2353#discussion_r1113843746
##########
streampark-console/streampark-console-service/src/test/java/org/apache/streampark/common/util/RegexTest.java:
##########
@@ -28,6 +26,8 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
import java.util.stream.Collectors;
class RegexTest {
Review Comment:
This is really tricky to solve, and the local environment needs to meet the requirements. We can delete this testcase first.
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] wolfboys commented on a diff in pull request #2353: [Fix] Test classes should be under sp package
Posted by "wolfboys (via GitHub)" <gi...@apache.org>.
wolfboys commented on code in PR #2353:
URL: https://github.com/apache/incubator-streampark/pull/2353#discussion_r1113847404
##########
streampark-console/streampark-console-service/src/test/java/org/apache/streampark/console/core/bean/SenderEmailTest.java:
##########
@@ -77,7 +77,7 @@ void alert() {
mail.setStatus(appState.name());
StringWriter writer = new StringWriter();
- Map<String, AlertTemplate> out = new HashMap<String, AlertTemplate>();
+ Map<String, AlertTemplate> out = new HashMap<>();
Review Comment:
I suggest remove these testcase classes
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] wolfboys commented on a diff in pull request #2353: [Fix] Test classes should be under sp package
Posted by "wolfboys (via GitHub)" <gi...@apache.org>.
wolfboys commented on code in PR #2353:
URL: https://github.com/apache/incubator-streampark/pull/2353#discussion_r1113846616
##########
streampark-console/streampark-console-service/src/test/java/org/apache/streampark/console/core/bean/SenderEmailTest.java:
##########
@@ -77,7 +77,7 @@ void alert() {
mail.setStatus(appState.name());
StringWriter writer = new StringWriter();
- Map<String, AlertTemplate> out = new HashMap<String, AlertTemplate>();
+ Map<String, AlertTemplate> out = new HashMap<>();
Review Comment:
Because it is necessary to test the send email. It is necessary to manually fill in info such as sending emails, password ... These are private info and cannot be provided
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] wolfboys merged pull request #2353: [Fix] Test classes should be under sp package
Posted by "wolfboys (via GitHub)" <gi...@apache.org>.
wolfboys merged PR #2353:
URL: https://github.com/apache/incubator-streampark/pull/2353
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] tisonkun commented on a diff in pull request #2353: [Fix] Test classes should be under sp package
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on code in PR #2353:
URL: https://github.com/apache/incubator-streampark/pull/2353#discussion_r1113844983
##########
streampark-console/streampark-console-service/src/test/java/org/apache/streampark/common/util/RegexTest.java:
##########
@@ -28,6 +26,8 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
import java.util.stream.Collectors;
class RegexTest {
Review Comment:
Sounds reasonable. But I don't have time to investigate more. Perhaps open as a follow-up.
--
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: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org