You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@helix.apache.org by GitBox <gi...@apache.org> on 2020/07/29 07:43:04 UTC
[GitHub] [helix] kaisun2000 commented on a change in pull request #1180: Remove race condition in TestTaskSchedulingTwoCurrentStates
kaisun2000 commented on a change in pull request #1180:
URL: https://github.com/apache/helix/pull/1180#discussion_r461796364
##########
File path: helix-core/src/test/java/org/apache/helix/integration/task/TestTaskSchedulingTwoCurrentStates.java
##########
@@ -65,7 +60,7 @@
* sent when there are two CurrentStates.
*/
public class TestTaskSchedulingTwoCurrentStates extends TaskTestBase {
- private static final String DATABASE = WorkflowGenerator.DEFAULT_TGT_DB;
Review comment:
Is it possible the old way of using WorkflowGenerator.DEFAULT_TGT_DB would have conflict, if multiple test using the same "WorkflowGenerator.DEFAULT_TGT_DB"?
##########
File path: helix-core/src/test/java/org/apache/helix/integration/task/TestTaskSchedulingTwoCurrentStates.java
##########
@@ -65,7 +60,7 @@
* sent when there are two CurrentStates.
*/
public class TestTaskSchedulingTwoCurrentStates extends TaskTestBase {
- private static final String DATABASE = WorkflowGenerator.DEFAULT_TGT_DB;
Review comment:
I think the better way to fix it is not call super.beforeClass() in line 72/77. This way, we won't create the cluster/full auto etc and participant that we need to stop immediately.
Instead, just creates cluster, controller and participant in this before test. What is your take?
##########
File path: helix-core/src/test/java/org/apache/helix/integration/task/TestTaskSchedulingTwoCurrentStates.java
##########
@@ -65,7 +60,7 @@
* sent when there are two CurrentStates.
*/
public class TestTaskSchedulingTwoCurrentStates extends TaskTestBase {
- private static final String DATABASE = WorkflowGenerator.DEFAULT_TGT_DB;
+ private static final String DATABASE = "TestDB_" + TestHelper.getTestClassName();
Review comment:
Another thing is that for all the resources created in
```
@BeforeClass
public void beforeClass() throws Exception {
```
We should tear them down. The participant threads, the controller thread should be stopped with syncStop. Maybe we should also clean up the cluster path etc in Zookeeper.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org