[GitHub] incubator-rocketmq pull request #145: [ROCKETMQ-264]Fix unit test cost too l...
Github user lindzh closed the pull request at: https://github.com/apache/incubator-rocketmq/pull/145 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq pull request #145: [ROCKETMQ-264]Fix unit test cost too l...
Github user lindzh commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/145#discussion_r135436908
--- Diff:
broker/src/test/java/org/apache/rocketmq/broker/filter/MessageStoreWithFilterTest.java
---
@@ -76,6 +86,24 @@
}
}
+@Before
+public void init() {
+filterManager = ConsumerFilterManagerTest.gen(topicCount,
msgPerTopic);
+try {
+master = gen(filterManager);
+} catch (Exception e) {
+e.printStackTrace();
--- End diff --
This is a DefaultMessageStore start progress that used in annotation
Before.And can't add expect Exception in before,I think there is no need to add
exception expect in before and all we want is while the messageStore is started
or not.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq pull request #145: [ROCKETMQ-264]Fix unit test cost too l...
Github user lindzh commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/145#discussion_r135436434
--- Diff:
store/src/test/java/org/apache/rocketmq/store/DefaultMessageStoreTest.java ---
@@ -43,15 +43,24 @@
private SocketAddress BornHost;
private SocketAddress StoreHost;
private byte[] MessageBody;
+private MessageStore messageStore;
@Before
public void init() throws Exception {
StoreHost = new InetSocketAddress(InetAddress.getLocalHost(),
8123);
BornHost = new
InetSocketAddress(InetAddress.getByName("127.0.0.1"), 0);
+
+messageStore = buildMessageStore();
+boolean load = messageStore.load();
+assertTrue(load);
+messageStore.start();
}
@After
public void destory() {
+messageStore.shutdown();
+messageStore.destroy();
--- End diff --
Delete created files has been did before at this file after destroy.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq pull request #145: [ROCKETMQ-264]Fix unit test cost too l...
Github user lindzh commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/145#discussion_r135431420
--- Diff:
store/src/test/java/org/apache/rocketmq/store/DefaultMessageStoreTest.java ---
@@ -97,54 +110,46 @@ public MessageExtBrokerInner buildMessage() {
@Test
public void testGroupCommit() throws Exception {
-long totalMsgs = 100;
+long totalMsgs = 10;
QUEUE_TOTAL = 1;
MessageBody = StoreMessage.getBytes();
-MessageStoreConfig messageStoreConfig = new MessageStoreConfig();
-messageStoreConfig.setMapedFileSizeCommitLog(1024 * 8);
-messageStoreConfig.setFlushDiskType(FlushDiskType.SYNC_FLUSH);
-MessageStore master = new DefaultMessageStore(messageStoreConfig,
null, new MyMessageArrivingListener(), new BrokerConfig());
-boolean load = master.load();
-assertTrue(load);
+for (long i = 0; i < totalMsgs; i++) {
+messageStore.putMessage(buildMessage());
+}
-master.start();
-verifyThatMasterIsFunctional(totalMsgs, master);
+for (long i = 0; i < totalMsgs; i++) {
+GetMessageResult result = messageStore.getMessage("GROUP_A",
"TOPIC_A", 0, i, 1024 * 1024, null);
+assertThat(result).isNotNull();
+result.release();
+}
+verifyThatMasterIsFunctional(totalMsgs, messageStore);
}
private void verifyThatMasterIsFunctional(long totalMsgs, MessageStore
master) {
-try {
-for (long i = 0; i < totalMsgs; i++) {
-master.putMessage(buildMessage());
-}
-
-for (long i = 0; i < totalMsgs; i++) {
-GetMessageResult result = master.getMessage("GROUP_A",
"TOPIC_A", 0, i, 1024 * 1024, null);
-assertThat(result).isNotNull();
-result.release();
-
-}
-} finally {
-master.shutdown();
-master.destroy();
+for (long i = 0; i < totalMsgs; i++) {
+master.putMessage(buildMessage());
+}
+
+for (long i = 0; i < totalMsgs; i++) {
+GetMessageResult result = master.getMessage("GROUP_A",
"TOPIC_A", 0, i, 1024 * 1024, null);
+assertThat(result).isNotNull();
+result.release();
+
}
}
@Test
public void testPullSize() throws Exception {
-MessageStore messageStore = buildMessageStore();
-boolean load = messageStore.load();
-assertTrue(load);
-messageStore.start();
String topic = "pullSizeTopic";
for (int i = 0; i < 32; i++) {
MessageExtBrokerInner messageExtBrokerInner = buildMessage();
messageExtBrokerInner.setTopic(topic);
messageExtBrokerInner.setQueueId(0);
-PutMessageResult putMessageResult =
messageStore.putMessage(messageExtBrokerInner);
+messageStore.putMessage(messageExtBrokerInner);
}
//wait for consume queue build
-Thread.sleep(100);
+Thread.sleep(10);
--- End diff --
In test scenario, 10ms is enough.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq pull request #145: [ROCKETMQ-264]Fix unit test cost too l...
Github user lindzh commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/145#discussion_r134975634
--- Diff:
client/src/test/java/org/apache/rocketmq/client/consumer/rebalance/AllocateMessageQueueConsitentHashTest.java
---
@@ -92,9 +92,9 @@ public void testAllocate2() {
@Test
public void testRun100RandomCase() {
-for (int i = 0; i < 100; i++) {
-int consumerSize = new Random().nextInt(200) + 1;//1-200
-int queueSize = new Random().nextInt(100) + 1;//1-100
+for (int i = 0; i < 10; i++) {
+int consumerSize = new Random().nextInt(20) + 1;//1-20
--- End diff --
We aim at making test cost less time, make iteration count less is a good
idea as unit test is only test for function.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq pull request #145: [ROCKETMQ-264]Fix unit test cost too l...
Github user lindzh commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/145#discussion_r134974719
--- Diff:
broker/src/test/java/org/apache/rocketmq/broker/BrokerControllerTest.java ---
@@ -37,16 +37,14 @@
*/
@Test
public void testBrokerRestart() throws Exception {
-for (int i = 0; i < 2; i++) {
-BrokerController brokerController = new BrokerController(
-new BrokerConfig(),
-new NettyServerConfig(),
-new NettyClientConfig(),
-new MessageStoreConfig());
-assertThat(brokerController.initialize());
-brokerController.start();
-brokerController.shutdown();
-}
+BrokerController brokerController = new BrokerController(
--- End diff --
As there is more whole startup in test case , I think there no need to do
this. Any questions?IMO
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq pull request #145: [ROCKETMQ-264]Fix unit test cost too l...
Github user lindzh commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/145#discussion_r134974332
--- Diff:
broker/src/test/java/org/apache/rocketmq/broker/filter/MessageStoreWithFilterTest.java
---
@@ -201,177 +229,143 @@ public void dispatch(DispatchRequest request) {
@Test
public void testGetMessage_withFilterBitMapAndConsumerChanged() {
-int topicCount = 10, msgPerTopic = 10;
-ConsumerFilterManager filterManager =
ConsumerFilterManagerTest.gen(topicCount, msgPerTopic);
-
-DefaultMessageStore master = null;
+List msgs = null;
try {
-master = gen(filterManager);
+msgs = putMsg(master, topicCount, msgPerTopic);
} catch (Exception e) {
e.printStackTrace();
assertThat(true).isFalse();
}
+// sleep to wait for consume queue has been constructed.
try {
-List msgs = null;
-try {
-msgs = putMsg(master, topicCount, msgPerTopic);
-} catch (Exception e) {
-e.printStackTrace();
-assertThat(true).isFalse();
-}
-
-// sleep to wait for consume queue has been constructed.
-try {
-Thread.sleep(1000);
-} catch (InterruptedException e) {
-e.printStackTrace();
-assertThat(true).isFalse();
-}
+Thread.sleep(200);
--- End diff --
At this time,there is no way to ensure CQ constructed except adding
countdownlatch to CQ,if only in test,I think there is no need to do this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq pull request #145: [ROCKETMQ-264]Fix unit test cost too l...
Github user vsair commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/145#discussion_r134968491
--- Diff:
store/src/test/java/org/apache/rocketmq/store/DefaultMessageStoreTest.java ---
@@ -97,54 +110,46 @@ public MessageExtBrokerInner buildMessage() {
@Test
public void testGroupCommit() throws Exception {
-long totalMsgs = 100;
+long totalMsgs = 10;
QUEUE_TOTAL = 1;
MessageBody = StoreMessage.getBytes();
-MessageStoreConfig messageStoreConfig = new MessageStoreConfig();
-messageStoreConfig.setMapedFileSizeCommitLog(1024 * 8);
-messageStoreConfig.setFlushDiskType(FlushDiskType.SYNC_FLUSH);
-MessageStore master = new DefaultMessageStore(messageStoreConfig,
null, new MyMessageArrivingListener(), new BrokerConfig());
-boolean load = master.load();
-assertTrue(load);
+for (long i = 0; i < totalMsgs; i++) {
+messageStore.putMessage(buildMessage());
+}
-master.start();
-verifyThatMasterIsFunctional(totalMsgs, master);
+for (long i = 0; i < totalMsgs; i++) {
+GetMessageResult result = messageStore.getMessage("GROUP_A",
"TOPIC_A", 0, i, 1024 * 1024, null);
+assertThat(result).isNotNull();
+result.release();
+}
+verifyThatMasterIsFunctional(totalMsgs, messageStore);
}
private void verifyThatMasterIsFunctional(long totalMsgs, MessageStore
master) {
-try {
-for (long i = 0; i < totalMsgs; i++) {
-master.putMessage(buildMessage());
-}
-
-for (long i = 0; i < totalMsgs; i++) {
-GetMessageResult result = master.getMessage("GROUP_A",
"TOPIC_A", 0, i, 1024 * 1024, null);
-assertThat(result).isNotNull();
-result.release();
-
-}
-} finally {
-master.shutdown();
-master.destroy();
+for (long i = 0; i < totalMsgs; i++) {
+master.putMessage(buildMessage());
+}
+
+for (long i = 0; i < totalMsgs; i++) {
+GetMessageResult result = master.getMessage("GROUP_A",
"TOPIC_A", 0, i, 1024 * 1024, null);
+assertThat(result).isNotNull();
+result.release();
+
}
}
@Test
public void testPullSize() throws Exception {
-MessageStore messageStore = buildMessageStore();
-boolean load = messageStore.load();
-assertTrue(load);
-messageStore.start();
String topic = "pullSizeTopic";
for (int i = 0; i < 32; i++) {
MessageExtBrokerInner messageExtBrokerInner = buildMessage();
messageExtBrokerInner.setTopic(topic);
messageExtBrokerInner.setQueueId(0);
-PutMessageResult putMessageResult =
messageStore.putMessage(messageExtBrokerInner);
+messageStore.putMessage(messageExtBrokerInner);
}
//wait for consume queue build
-Thread.sleep(100);
+Thread.sleep(10);
--- End diff --
Is 10ms enough to build consume queue?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq pull request #145: [ROCKETMQ-264]Fix unit test cost too l...
Github user vsair commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/145#discussion_r134969821
--- Diff:
store/src/test/java/org/apache/rocketmq/store/DefaultMessageStoreTest.java ---
@@ -43,15 +43,24 @@
private SocketAddress BornHost;
private SocketAddress StoreHost;
private byte[] MessageBody;
+private MessageStore messageStore;
@Before
public void init() throws Exception {
StoreHost = new InetSocketAddress(InetAddress.getLocalHost(),
8123);
BornHost = new
InetSocketAddress(InetAddress.getByName("127.0.0.1"), 0);
+
+messageStore = buildMessageStore();
+boolean load = messageStore.load();
+assertTrue(load);
+messageStore.start();
}
@After
public void destory() {
+messageStore.shutdown();
+messageStore.destroy();
--- End diff --
As i remember, method of destroy may not clean all files generated.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq pull request #145: [ROCKETMQ-264]Fix unit test cost too l...
Github user vsair commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/145#discussion_r134970377
--- Diff:
store/src/test/java/org/apache/rocketmq/store/DefaultMessageStoreTest.java ---
@@ -97,54 +110,46 @@ public MessageExtBrokerInner buildMessage() {
@Test
public void testGroupCommit() throws Exception {
-long totalMsgs = 100;
+long totalMsgs = 10;
QUEUE_TOTAL = 1;
MessageBody = StoreMessage.getBytes();
-MessageStoreConfig messageStoreConfig = new MessageStoreConfig();
-messageStoreConfig.setMapedFileSizeCommitLog(1024 * 8);
-messageStoreConfig.setFlushDiskType(FlushDiskType.SYNC_FLUSH);
-MessageStore master = new DefaultMessageStore(messageStoreConfig,
null, new MyMessageArrivingListener(), new BrokerConfig());
-boolean load = master.load();
-assertTrue(load);
+for (long i = 0; i < totalMsgs; i++) {
+messageStore.putMessage(buildMessage());
+}
-master.start();
-verifyThatMasterIsFunctional(totalMsgs, master);
+for (long i = 0; i < totalMsgs; i++) {
+GetMessageResult result = messageStore.getMessage("GROUP_A",
"TOPIC_A", 0, i, 1024 * 1024, null);
+assertThat(result).isNotNull();
+result.release();
+}
+verifyThatMasterIsFunctional(totalMsgs, messageStore);
}
private void verifyThatMasterIsFunctional(long totalMsgs, MessageStore
master) {
-try {
-for (long i = 0; i < totalMsgs; i++) {
-master.putMessage(buildMessage());
-}
-
-for (long i = 0; i < totalMsgs; i++) {
-GetMessageResult result = master.getMessage("GROUP_A",
"TOPIC_A", 0, i, 1024 * 1024, null);
-assertThat(result).isNotNull();
-result.release();
-
-}
-} finally {
-master.shutdown();
-master.destroy();
+for (long i = 0; i < totalMsgs; i++) {
+master.putMessage(buildMessage());
+}
+
+for (long i = 0; i < totalMsgs; i++) {
+GetMessageResult result = master.getMessage("GROUP_A",
"TOPIC_A", 0, i, 1024 * 1024, null);
+assertThat(result).isNotNull();
+result.release();
+
}
}
@Test
public void testPullSize() throws Exception {
-MessageStore messageStore = buildMessageStore();
-boolean load = messageStore.load();
-assertTrue(load);
-messageStore.start();
String topic = "pullSizeTopic";
for (int i = 0; i < 32; i++) {
MessageExtBrokerInner messageExtBrokerInner = buildMessage();
messageExtBrokerInner.setTopic(topic);
messageExtBrokerInner.setQueueId(0);
-PutMessageResult putMessageResult =
messageStore.putMessage(messageExtBrokerInner);
+messageStore.putMessage(messageExtBrokerInner);
}
//wait for consume queue build
-Thread.sleep(100);
+Thread.sleep(10);
--- End diff --
Is it enough to build consume queue ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq pull request #145: [ROCKETMQ-264]Fix unit test cost too l...
Github user vsair commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/145#discussion_r134970255
--- Diff:
client/src/test/java/org/apache/rocketmq/client/consumer/rebalance/AllocateMessageQueueConsitentHashTest.java
---
@@ -92,9 +92,9 @@ public void testAllocate2() {
@Test
public void testRun100RandomCase() {
-for (int i = 0; i < 100; i++) {
-int consumerSize = new Random().nextInt(200) + 1;//1-200
-int queueSize = new Random().nextInt(100) + 1;//1-100
+for (int i = 0; i < 10; i++) {
+int consumerSize = new Random().nextInt(20) + 1;//1-20
--- End diff --
Does it spend too much time to allocate queue ? If not, i think it does't
matter to run 10 or 100 times.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq pull request #145: [ROCKETMQ-264]Fix unit test cost too l...
Github user shroman commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/145#discussion_r134962143
--- Diff:
client/src/test/java/org/apache/rocketmq/client/consumer/rebalance/AllocateMessageQueueConsitentHashTest.java
---
@@ -92,9 +92,9 @@ public void testAllocate2() {
@Test
public void testRun100RandomCase() {
-for (int i = 0; i < 100; i++) {
-int consumerSize = new Random().nextInt(200) + 1;//1-200
-int queueSize = new Random().nextInt(100) + 1;//1-100
+for (int i = 0; i < 10; i++) {
+int consumerSize = new Random().nextInt(20) + 1;//1-20
--- End diff --
@vongosling If you have stress tests for this, I think low numbers here are
ok.
Btw, how do/will you verify it?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq pull request #145: [ROCKETMQ-264]Fix unit test cost too l...
Github user vongosling commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/145#discussion_r134941453
--- Diff:
client/src/test/java/org/apache/rocketmq/client/consumer/rebalance/AllocateMessageQueueConsitentHashTest.java
---
@@ -92,9 +92,9 @@ public void testAllocate2() {
@Test
public void testRun100RandomCase() {
-for (int i = 0; i < 100; i++) {
-int consumerSize = new Random().nextInt(200) + 1;//1-200
-int queueSize = new Random().nextInt(100) + 1;//1-100
+for (int i = 0; i < 10; i++) {
+int consumerSize = new Random().nextInt(20) + 1;//1-20
--- End diff --
@shroman I would like to verify the program correction though the specific
trick not iteration count, especially in unit-test, thoughts ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq pull request #145: [ROCKETMQ-264]Fix unit test cost too l...
Github user shroman commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/145#discussion_r133863841
--- Diff:
client/src/test/java/org/apache/rocketmq/client/consumer/rebalance/AllocateMessageQueueConsitentHashTest.java
---
@@ -92,9 +92,9 @@ public void testAllocate2() {
@Test
public void testRun100RandomCase() {
-for (int i = 0; i < 100; i++) {
-int consumerSize = new Random().nextInt(200) + 1;//1-200
-int queueSize = new Random().nextInt(100) + 1;//1-100
+for (int i = 0; i < 10; i++) {
+int consumerSize = new Random().nextInt(20) + 1;//1-20
--- End diff --
Guys, what's the point to reduce these numbers (in other tests in this PR
too)?
@vongosling @zhouxinyu
IMO, we need to have a pretty high number for messages/queues/etc. in unit
tests, because it is a large-scale messaging system. Unless it makes tests
really slow.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq pull request #145: [ROCKETMQ-264]Fix unit test cost too l...
Github user shroman commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/145#discussion_r133863133 --- Diff: namesrv/src/test/java/org/apache/rocketmq/namesrv/NamesrvControllerTest.java --- @@ -1,46 +0,0 @@ -/* --- End diff -- Why is it deleted?? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq pull request #145: [ROCKETMQ-264]Fix unit test cost too l...
Github user shroman commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/145#discussion_r133862502
--- Diff:
broker/src/test/java/org/apache/rocketmq/broker/BrokerControllerTest.java ---
@@ -37,16 +37,14 @@
*/
@Test
public void testBrokerRestart() throws Exception {
-for (int i = 0; i < 2; i++) {
-BrokerController brokerController = new BrokerController(
-new BrokerConfig(),
-new NettyServerConfig(),
-new NettyClientConfig(),
-new MessageStoreConfig());
-assertThat(brokerController.initialize());
-brokerController.start();
-brokerController.shutdown();
-}
+BrokerController brokerController = new BrokerController(
--- End diff --
By changing this you change the expected behavior -- it is a check for
proper `restart`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq pull request #145: [ROCKETMQ-264]Fix unit test cost too l...
Github user lindzh commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/145#discussion_r133344416
--- Diff:
broker/src/test/java/org/apache/rocketmq/broker/filter/MessageStoreWithFilterTest.java
---
@@ -76,6 +86,24 @@
}
}
+@Before
+public void init() {
+filterManager = ConsumerFilterManagerTest.gen(topicCount,
msgPerTopic);
+try {
+master = gen(filterManager);
+} catch (Exception e) {
+e.printStackTrace();
--- End diff --
Yep,that's a good idea and this exception will be fixed at next commit.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq pull request #145: [ROCKETMQ-264]Fix unit test cost too l...
Github user vongosling commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/145#discussion_r133153891
--- Diff:
broker/src/test/java/org/apache/rocketmq/broker/filter/MessageStoreWithFilterTest.java
---
@@ -76,6 +86,24 @@
}
}
+@Before
+public void init() {
+filterManager = ConsumerFilterManagerTest.gen(topicCount,
msgPerTopic);
+try {
+master = gen(filterManager);
+} catch (Exception e) {
+e.printStackTrace();
--- End diff --
redundant code, when you use assert in your exception scenario, right ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq pull request #145: [ROCKETMQ-264]Fix unit test cost too l...
Github user zhouxinyu commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/145#discussion_r132838396
--- Diff:
broker/src/test/java/org/apache/rocketmq/broker/BrokerControllerTest.java ---
@@ -37,16 +37,14 @@
*/
@Test
public void testBrokerRestart() throws Exception {
-for (int i = 0; i < 2; i++) {
-BrokerController brokerController = new BrokerController(//
-new BrokerConfig(), //
-new NettyServerConfig(), //
-new NettyClientConfig(), //
-new MessageStoreConfig());
-assertThat(brokerController.initialize());
-brokerController.start();
-brokerController.shutdown();
-}
+BrokerController brokerController = new BrokerController(//
+new BrokerConfig(), //
+new NettyServerConfig(), //
+new NettyClientConfig(), //
+new MessageStoreConfig());
+assertThat(brokerController.initialize());
+brokerController.start();
+brokerController.shutdown();
--- End diff --
If we only start/shutdown broker once, `testBrokerRestart` isn't a
appropriate method~
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq pull request #145: [ROCKETMQ-264]Fix unit test cost too l...
Github user zhouxinyu commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/145#discussion_r132838549
--- Diff:
client/src/test/java/org/apache/rocketmq/client/consumer/rebalance/AllocateMessageQueueConsitentHashTest.java
---
@@ -95,14 +95,15 @@ public void testAllocate2() {
@Test
-public void testRun100RandomCase(){
-for(int i=0;i<100;i++){
-int consumerSize = new Random().nextInt(200)+1;//1-200
-int queueSize = new Random().nextInt(100)+1;//1-100
-testAllocate(queueSize,consumerSize);
+public void testRun100RandomCase() {
+for (int i = 0; i < 10; i++) {
+int consumerSize = new Random().nextInt(20) + 1;//1-20
+int queueSize = new Random().nextInt(20) + 1;//1-20
+testAllocate(queueSize, consumerSize);
try {
Thread.sleep(1);
-} catch (InterruptedException e) {}
+} catch (InterruptedException e) {
--- End diff --
Add exception to method signature is better~
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq pull request #145: [ROCKETMQ-264]Fix unit test cost too l...
Github user zhouxinyu commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/145#discussion_r132838496
--- Diff:
broker/src/test/java/org/apache/rocketmq/broker/filter/MessageStoreWithFilterTest.java
---
@@ -201,177 +229,143 @@ public void dispatch(DispatchRequest request) {
@Test
public void testGetMessage_withFilterBitMapAndConsumerChanged() {
-int topicCount = 10, msgPerTopic = 10;
-ConsumerFilterManager filterManager =
ConsumerFilterManagerTest.gen(topicCount, msgPerTopic);
-
-DefaultMessageStore master = null;
+List msgs = null;
try {
-master = gen(filterManager);
+msgs = putMsg(master, topicCount, msgPerTopic);
} catch (Exception e) {
e.printStackTrace();
assertThat(true).isFalse();
}
+// sleep to wait for consume queue has been constructed.
try {
-List msgs = null;
-try {
-msgs = putMsg(master, topicCount, msgPerTopic);
-} catch (Exception e) {
-e.printStackTrace();
-assertThat(true).isFalse();
-}
-
-// sleep to wait for consume queue has been constructed.
-try {
-Thread.sleep(1000);
-} catch (InterruptedException e) {
-e.printStackTrace();
-assertThat(true).isFalse();
-}
+Thread.sleep(200);
--- End diff --
Is `200ms` enough to dispatch consume queue in a slow speed disk, or do we
have another method to ensure the CQ has been constructed?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
