You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rocketmq.apache.org by GitBox <gi...@apache.org> on 2022/06/22 11:02:07 UTC
[GitHub] [rocketmq] dongeforever opened a new pull request, #4500: Fix static topic test
dongeforever opened a new pull request, #4500:
URL: https://github.com/apache/rocketmq/pull/4500
**Make sure set the target branch to `develop`**
## What is the purpose of the change
Fix the unstable test StaticTopicIT
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] dongeforever merged pull request #4500: Fix static topic test, await the client metadata to be refreshed
Posted by GitBox <gi...@apache.org>.
dongeforever merged PR #4500:
URL: https://github.com/apache/rocketmq/pull/4500
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] dongeforever commented on pull request #4500: [Need to find root cause, do not merge now] Fix static topic test, await the client metadata to be refreshed
Posted by GitBox <gi...@apache.org>.
dongeforever commented on PR #4500:
URL: https://github.com/apache/rocketmq/pull/4500#issuecomment-1168385016
The root cause seems to be that the client does not refresh the metadata in time.
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] lizhanhui commented on a diff in pull request #4500: Fix static topic test
Posted by GitBox <gi...@apache.org>.
lizhanhui commented on code in PR #4500:
URL: https://github.com/apache/rocketmq/pull/4500#discussion_r905654947
##########
test/src/test/java/org/apache/rocketmq/test/statictopic/StaticTopicIT.java:
##########
@@ -117,7 +116,7 @@ public void testCommandsWithBrokers() throws Exception {
{
Set<String> brokers = ImmutableSet.of(broker2Name);
MQAdminTestUtils.remappingStaticTopicWithCommand(topic, brokers, null, nsAddr);
- Thread.sleep(500);
Review Comment:
Fix the magic number, please.
##########
test/src/test/java/org/apache/rocketmq/test/statictopic/StaticTopicIT.java:
##########
@@ -340,7 +340,7 @@ public void testDoubleReadCheckConsumerOffset() throws Exception {
sendMessagesAndCheck(producer, targetBrokers, topic, queueNum, msgEachQueue, (i + 1) * TopicQueueMappingUtils.DEFAULT_BLOCK_SEQ_SIZE);
}
- TestUtils.waitForSeconds(20);
Review Comment:
Same here
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] dongeforever commented on pull request #4500: [Need to find root cause, do not merge now] Fix static topic test
Posted by GitBox <gi...@apache.org>.
dongeforever commented on PR #4500:
URL: https://github.com/apache/rocketmq/pull/4500#issuecomment-1165308716
@lizhanhui I agree with you, await is better than sleeping.
This test is stable on local mac, but it is not stable on the Travis-ci cloud. And I have not found the root cause yet. Could you help to find it, and show the await condition in detail? I am struggling with it for days.
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] dongeforever closed pull request #4500: Static topic test
Posted by GitBox <gi...@apache.org>.
dongeforever closed pull request #4500: Static topic test
URL: https://github.com/apache/rocketmq/pull/4500
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] lizhanhui commented on pull request #4500: Fix static topic test
Posted by GitBox <gi...@apache.org>.
lizhanhui commented on PR #4500:
URL: https://github.com/apache/rocketmq/pull/4500#issuecomment-1164293386
This pull request is terrible... Magic amount of sleep duration is the root cause of random test failures. Should get rid of them.
[Stackoverflow has a thread discussing similar issues.](https://stackoverflow.com/questions/36283334/how-to-avoid-thread-sleep-in-unit-tests) Maybe we should turn to [awaitility](https://github.com/awaitility/awaitility) as recommended.
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] coveralls commented on pull request #4500: Fix static topic test
Posted by GitBox <gi...@apache.org>.
coveralls commented on PR #4500:
URL: https://github.com/apache/rocketmq/pull/4500#issuecomment-1164178804
[](https://coveralls.io/builds/50289205)
Coverage increased (+0.07%) to 47.234% when pulling **5cc93a61924d437164d82279cdffc289505d930b on dongeforever:static_topic_test** into **388cc211183e64ad2dc5cca8f911ccf92a0c064e on apache:5.0.0-beta**.
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] dongeforever commented on a diff in pull request #4500: Fix static topic test, await the client metadata to be refreshed
Posted by GitBox <gi...@apache.org>.
dongeforever commented on code in PR #4500:
URL: https://github.com/apache/rocketmq/pull/4500#discussion_r912726655
##########
test/src/test/java/org/apache/rocketmq/test/statictopic/StaticTopicIT.java:
##########
@@ -340,7 +340,7 @@ public void testDoubleReadCheckConsumerOffset() throws Exception {
sendMessagesAndCheck(producer, targetBrokers, topic, queueNum, msgEachQueue, (i + 1) * TopicQueueMappingUtils.DEFAULT_BLOCK_SEQ_SIZE);
}
- TestUtils.waitForSeconds(20);
Review Comment:
done
##########
test/src/test/java/org/apache/rocketmq/test/statictopic/StaticTopicIT.java:
##########
@@ -117,7 +116,7 @@ public void testCommandsWithBrokers() throws Exception {
{
Set<String> brokers = ImmutableSet.of(broker2Name);
MQAdminTestUtils.remappingStaticTopicWithCommand(topic, brokers, null, nsAddr);
- Thread.sleep(500);
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org