diff --git a/sentinel-core/src/test/java/com/alibaba/csp/sentinel/node/ClusterNodeTest.java b/sentinel-core/src/test/java/com/alibaba/csp/sentinel/node/ClusterNodeTest.java index e762a09f..258ac76d 100644 --- a/sentinel-core/src/test/java/com/alibaba/csp/sentinel/node/ClusterNodeTest.java +++ b/sentinel-core/src/test/java/com/alibaba/csp/sentinel/node/ClusterNodeTest.java @@ -16,15 +16,18 @@ package com.alibaba.csp.sentinel.node; import com.alibaba.csp.sentinel.slots.block.flow.FlowException; + import org.junit.Test; import java.util.ArrayList; import java.util.Arrays; -import java.util.HashSet; +import java.util.Collections; import java.util.List; import java.util.Random; import java.util.Set; import java.util.concurrent.Callable; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -34,7 +37,6 @@ import static org.junit.Assert.*; * Test cases for {@link ClusterNode}. * * @author cdfive - * @date 2019-01-11 */ public class ClusterNodeTest { @@ -64,23 +66,28 @@ public class ClusterNodeTest { @Test public void testGetOrCreateOriginNodeMultiThread() { - // note: in junit4, repeat execute a test method is not very convenient + // Note: in JUnit 4, repeat execute a test method is not very convenient // for simple, here use a loop instead // https://stackoverflow.com/questions/1492856/easy-way-of-running-the-same-junit-test-over-and-over - // in junit5, use @RepeatedTest(10) - int testTimes = 10;// execute 10 times, test will have chance to failed, if remove the lock in ClusterNode + // in JUnit 5, use @RepeatedTest(10) + + // execute 10 times, test will have chance to failed, if remove the lock in ClusterNode + final int testTimes = 10; + for (int times = 0; times < testTimes; times++) { final ClusterNode clusterNode = new ClusterNode(); - // store all distinct nodes by calling ClusterNode#getOrCreateOriginNode - final Set createdNodes = new HashSet(); + // Store all distinct nodes by calling ClusterNode#getOrCreateOriginNode. + // Here we need a thread-safe concurrent set (created from ConcurrentHashMap). + final Set createdNodes = Collections.newSetFromMap(new ConcurrentHashMap()); final Random random = new Random(); - // 10 threads, 3 origins, 20 tasks(in total, calling 20 times of ClusterNode#getOrCreateOriginNode concurrently) + // 10 threads, 3 origins, 20 tasks (calling 20 times of ClusterNode#getOrCreateOriginNode concurrently) final ExecutorService es = Executors.newFixedThreadPool(10); final List origins = Arrays.asList("origin1", "origin2", "origin3"); int taskCount = 20; + final CountDownLatch latch = new CountDownLatch(taskCount); List> tasks = new ArrayList>(taskCount); for (int i = 0; i < taskCount; i++) { @@ -90,10 +97,9 @@ public class ClusterNodeTest { // one task call one times of ClusterNode#getOrCreateOriginNode Node node = clusterNode.getOrCreateOriginNode(origins.get(random.nextInt(origins.size()))); // add the result node to the createdNodes set - // node: since HashSet is non-threadsafe, synchronized the ClusterNodeTest.class - synchronized (ClusterNodeTest.class) { - createdNodes.add(node); - } + createdNodes.add(node); + latch.countDown(); + return null; } }); @@ -101,6 +107,7 @@ public class ClusterNodeTest { try { es.invokeAll(tasks); + latch.await(); } catch (InterruptedException e) { e.printStackTrace(); } @@ -111,7 +118,8 @@ public class ClusterNodeTest { // not use `assertEquals(origins.size(), createdNodes.size());`, but a compare judgement for debug if (origins.size() != createdNodes.size()) { - // for debug, we can add a breakpoint here to see the detail info of createdNodes, if remove the lock in ClusterNode + // for debug, we can add a breakpoint here to see the detail info of createdNodes, + // if we removed the lock in ClusterNode fail("originCountMap's size should " + origins.size() + ", but actual " + createdNodes.size()); } @@ -122,9 +130,8 @@ public class ClusterNodeTest { } } - @Test - public void testTrace() { + public void testTraceException() { ClusterNode clusterNode = new ClusterNode(); Exception exception = new RuntimeException("test"); diff --git a/sentinel-core/src/test/java/com/alibaba/csp/sentinel/node/DefaultNodeBuilderTest.java b/sentinel-core/src/test/java/com/alibaba/csp/sentinel/node/DefaultNodeBuilderTest.java index 19eb927b..56872fa3 100644 --- a/sentinel-core/src/test/java/com/alibaba/csp/sentinel/node/DefaultNodeBuilderTest.java +++ b/sentinel-core/src/test/java/com/alibaba/csp/sentinel/node/DefaultNodeBuilderTest.java @@ -26,7 +26,6 @@ import static org.junit.Assert.*; * Test cases for {@link DefaultNodeBuilder}. * * @author cdfive - * @date 2019-01-15 */ public class DefaultNodeBuilderTest { @@ -51,7 +50,7 @@ public class DefaultNodeBuilderTest { } @Test - public void testBuildTreeNode_NullClusterNode() { + public void testBuildTreeNodeNullClusterNode() { DefaultNodeBuilder builder = new DefaultNodeBuilder(); ResourceWrapper id = new StringResourceWrapper("resA", EntryType.IN); @@ -59,7 +58,7 @@ public class DefaultNodeBuilderTest { assertNotNull(defaultNode); assertEquals(id, defaultNode.getId()); - assertEquals(null, defaultNode.getClusterNode()); + assertNull(defaultNode.getClusterNode()); // verify each call returns a different instance DefaultNode defaultNode2 = builder.buildTreeNode(id, null); diff --git a/sentinel-core/src/test/java/com/alibaba/csp/sentinel/node/StatisticNodeTest.java b/sentinel-core/src/test/java/com/alibaba/csp/sentinel/node/StatisticNodeTest.java index 674985f8..4520b5bc 100644 --- a/sentinel-core/src/test/java/com/alibaba/csp/sentinel/node/StatisticNodeTest.java +++ b/sentinel-core/src/test/java/com/alibaba/csp/sentinel/node/StatisticNodeTest.java @@ -36,7 +36,6 @@ import static org.junit.Assert.assertTrue; * Test cases for {@link StatisticNode}. * * @author cdfive - * @date 2019-01-08 */ public class StatisticNodeTest { @@ -53,7 +52,7 @@ public class StatisticNodeTest { * *

* 20 threads, 30 tasks, every task execute 10 times of bizMethod - * one bizMthod execute within 1 second, and within 0.5 second interval to exceute next bizMthod + * one bizMethod execute within 1 second, and within 0.5 second interval to exceute next bizMthod * so that the total time cost will be within 1 minute *

*