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 2021/07/22 00:38:21 UTC
[GitHub] [helix] alirezazamani opened a new pull request #1828: Stabilize TestInstancesAccessor
alirezazamani opened a new pull request #1828:
URL: https://github.com/apache/helix/pull/1828
### Issues
- [X] My PR addresses the following Helix issues and references them in the PR description:
Fixes #1821
### Description
- [x] Here are some details about my PR, including screenshots of any UI changes:
TestInstancesAccessor is unstable because the cluster might not be stable while the test runs the stoppable check and cluster config and instance config can be missing. In this commit, the test waits until the cluster config and instance config get created and then proceeds for the stoppable checks.
### Tests
- [x] The following is the result of the "mvn test" command on the appropriate module:
Helix-rest:
```
```
### Commits
- My commits all reference appropriate Apache Helix GitHub issues in their subject lines. In addition, my commits follow the guidelines from "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)":
1. Subject is separated from body by a blank line
1. Subject is limited to 50 characters (not including Jira issue reference)
1. Subject does not end with a period
1. Subject uses the imperative mood ("add", not "adding")
1. Body wraps at 72 characters
1. Body explains "what" and "why", not "how"
### Code Quality
- My diff has been formatted using helix-style.xml
(helix-style-intellij.xml if IntelliJ IDE is used)
--
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: reviews-unsubscribe@helix.apache.org
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
[GitHub] [helix] alirezazamani merged pull request #1828: Stabilize TestInstancesAccessor
Posted by GitBox <gi...@apache.org>.
alirezazamani merged pull request #1828:
URL: https://github.com/apache/helix/pull/1828
--
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: reviews-unsubscribe@helix.apache.org
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
[GitHub] [helix] jiajunwang commented on a change in pull request #1828: Stabilize TestInstancesAccessor
Posted by GitBox <gi...@apache.org>.
jiajunwang commented on a change in pull request #1828:
URL: https://github.com/apache/helix/pull/1828#discussion_r682902962
##########
File path: helix-rest/src/test/java/org/apache/helix/rest/server/TestInstancesAccessor.java
##########
@@ -46,7 +46,7 @@
private final static String CLUSTER_NAME = "TestCluster_0";
@Test
- public void testInstancesStoppable_zoneBased() throws IOException {
+ public void testInstancesStoppable_zoneBased() throws Exception {
Review comment:
nit, No need to change this file with your new modification?
--
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: reviews-unsubscribe@helix.apache.org
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
[GitHub] [helix] alirezazamani commented on a change in pull request #1828: Stabilize TestInstancesAccessor
Posted by GitBox <gi...@apache.org>.
alirezazamani commented on a change in pull request #1828:
URL: https://github.com/apache/helix/pull/1828#discussion_r682845671
##########
File path: helix-rest/src/test/java/org/apache/helix/rest/server/TestInstancesAccessor.java
##########
@@ -46,8 +46,34 @@
private final static String CLUSTER_NAME = "TestCluster_0";
@Test
- public void testInstancesStoppable_zoneBased() throws IOException {
+ public void testInstancesStoppable_zoneBased() throws Exception {
System.out.println("Start test :" + TestHelper.getTestMethodName());
+ // Make sure that cluster config exists
Review comment:
Done.
--
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: reviews-unsubscribe@helix.apache.org
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
[GitHub] [helix] alirezazamani commented on pull request #1828: Stabilize TestInstancesAccessor
Posted by GitBox <gi...@apache.org>.
alirezazamani commented on pull request #1828:
URL: https://github.com/apache/helix/pull/1828#issuecomment-885203321
> Running the test using the test script for 1000 times to make sure the test is stable. I will update the PR description after the test.
Ran the test 1000 times and all of them passed.
--
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: reviews-unsubscribe@helix.apache.org
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
[GitHub] [helix] alirezazamani commented on a change in pull request #1828: Stabilize TestInstancesAccessor
Posted by GitBox <gi...@apache.org>.
alirezazamani commented on a change in pull request #1828:
URL: https://github.com/apache/helix/pull/1828#discussion_r682845671
##########
File path: helix-rest/src/test/java/org/apache/helix/rest/server/TestInstancesAccessor.java
##########
@@ -46,8 +46,34 @@
private final static String CLUSTER_NAME = "TestCluster_0";
@Test
- public void testInstancesStoppable_zoneBased() throws IOException {
+ public void testInstancesStoppable_zoneBased() throws Exception {
System.out.println("Start test :" + TestHelper.getTestMethodName());
+ // Make sure that cluster config exists
Review comment:
Done.
--
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: reviews-unsubscribe@helix.apache.org
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
[GitHub] [helix] alirezazamani commented on pull request #1828: Stabilize TestInstancesAccessor
Posted by GitBox <gi...@apache.org>.
alirezazamani commented on pull request #1828:
URL: https://github.com/apache/helix/pull/1828#issuecomment-893689848
This PR is ready to be merged.
Commit message:
Stabilize TestInstancesAccessor
TestInstancesAccessor is unstable because the cluster might not be
stable while the test runs the stoppable check and cluster config and
instance config can be missing. In this commit, the test waits until the
cluster config and instance config get created and then proceeds for
the stoppable checks.
--
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: reviews-unsubscribe@helix.apache.org
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
[GitHub] [helix] jiajunwang commented on a change in pull request #1828: Stabilize TestInstancesAccessor
Posted by GitBox <gi...@apache.org>.
jiajunwang commented on a change in pull request #1828:
URL: https://github.com/apache/helix/pull/1828#discussion_r682902962
##########
File path: helix-rest/src/test/java/org/apache/helix/rest/server/TestInstancesAccessor.java
##########
@@ -46,7 +46,7 @@
private final static String CLUSTER_NAME = "TestCluster_0";
@Test
- public void testInstancesStoppable_zoneBased() throws IOException {
+ public void testInstancesStoppable_zoneBased() throws Exception {
Review comment:
nit, No need to change this file with your new modification?
--
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: reviews-unsubscribe@helix.apache.org
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
[GitHub] [helix] jiajunwang commented on a change in pull request #1828: Stabilize TestInstancesAccessor
Posted by GitBox <gi...@apache.org>.
jiajunwang commented on a change in pull request #1828:
URL: https://github.com/apache/helix/pull/1828#discussion_r681977339
##########
File path: helix-rest/src/test/java/org/apache/helix/rest/server/TestInstancesAccessor.java
##########
@@ -46,8 +46,34 @@
private final static String CLUSTER_NAME = "TestCluster_0";
@Test
- public void testInstancesStoppable_zoneBased() throws IOException {
+ public void testInstancesStoppable_zoneBased() throws Exception {
System.out.println("Start test :" + TestHelper.getTestMethodName());
+ // Make sure that cluster config exists
Review comment:
Shall we put the wait to the AbstractTestClass? Since the cluster and instances were created there. This check does not make sense in this specific test class scenario.
--
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: reviews-unsubscribe@helix.apache.org
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
[GitHub] [helix] alirezazamani commented on pull request #1828: Stabilize TestInstancesAccessor
Posted by GitBox <gi...@apache.org>.
alirezazamani commented on pull request #1828:
URL: https://github.com/apache/helix/pull/1828#issuecomment-884587994
Running the test in a loop for 1000 times to make sure the test is stable. I will update the PR description after the test.
--
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: reviews-unsubscribe@helix.apache.org
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
[GitHub] [helix] alirezazamani edited a comment on pull request #1828: Stabilize TestInstancesAccessor
Posted by GitBox <gi...@apache.org>.
alirezazamani edited a comment on pull request #1828:
URL: https://github.com/apache/helix/pull/1828#issuecomment-884587994
Running the test using the test script for 1000 times to make sure the test is stable. I will update the PR description after the test.
--
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: reviews-unsubscribe@helix.apache.org
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
[GitHub] [helix] alirezazamani commented on a change in pull request #1828: Stabilize TestInstancesAccessor
Posted by GitBox <gi...@apache.org>.
alirezazamani commented on a change in pull request #1828:
URL: https://github.com/apache/helix/pull/1828#discussion_r683695900
##########
File path: helix-rest/src/test/java/org/apache/helix/rest/server/TestInstancesAccessor.java
##########
@@ -46,7 +46,7 @@
private final static String CLUSTER_NAME = "TestCluster_0";
@Test
- public void testInstancesStoppable_zoneBased() throws IOException {
+ public void testInstancesStoppable_zoneBased() throws Exception {
Review comment:
Done.
--
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: reviews-unsubscribe@helix.apache.org
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