Copilot commented on code in PR #6093:
URL: https://github.com/apache/shenyu/pull/6093#discussion_r2268677779
##########
shenyu-loadbalancer/src/test/java/org/apache/shenyu/loadbalancer/spi/RoundRobinLoadBalanceTest.java:
##########
@@ -115,7 +117,28 @@ public void roundRobinLoadBalanceTest() {
.collect(Collectors.toList());
RoundRobinLoadBalancer roundRobinLoadBalancer = new
RoundRobinLoadBalancer();
- roundRobinLoadBalancer.select(upstreamList, "");
- roundRobinLoadBalancer.select(upstreamList2, "");
+
+ // Test with weighted upstream list
+ Upstream result1 = roundRobinLoadBalancer.select(upstreamList, "");
+ assertNotNull(result1, "Selected upstream should not be null");
+ assertTrue(upstreamList.contains(result1), "Selected upstream should
be from the provided list");
+
+ // Test with equal weight upstream list
+ Upstream result2 = roundRobinLoadBalancer.select(upstreamList2, "");
+ assertNotNull(result2, "Selected upstream should not be null");
+ assertTrue(upstreamList2.contains(result2), "Selected upstream should
be from the provided list");
+
+ // Test multiple selections to verify round-robin behavior
+ Map<String, Integer> countMap = new HashMap<>();
+ IntStream.range(0, 30).forEach(i -> {
+ Upstream result = roundRobinLoadBalancer.select(upstreamList2, "");
+ int count = countMap.getOrDefault(result.getUrl(), 0);
+ countMap.put(result.getUrl(), ++count);
+ });
+
+ // With equal weights, distribution should be roughly equal
+ assertEquals(3, countMap.size(), "All three upstreams should be
selected");
+ countMap.values().forEach(count ->
+ assertTrue(count >= 8 && count <= 12, "Distribution should be
roughly equal for equal weights"));
Review Comment:
The magic numbers 8 and 12 should be calculated based on the total
iterations and expected distribution rather than hardcoded. Consider using
variables like `minExpected = SELECTION_ITERATIONS / upstreamList2.size() -
tolerance` and `maxExpected = SELECTION_ITERATIONS / upstreamList2.size() +
tolerance`.
##########
shenyu-loadbalancer/src/test/java/org/apache/shenyu/loadbalancer/spi/RoundRobinLoadBalanceTest.java:
##########
@@ -115,7 +117,28 @@ public void roundRobinLoadBalanceTest() {
.collect(Collectors.toList());
RoundRobinLoadBalancer roundRobinLoadBalancer = new
RoundRobinLoadBalancer();
- roundRobinLoadBalancer.select(upstreamList, "");
- roundRobinLoadBalancer.select(upstreamList2, "");
+
+ // Test with weighted upstream list
+ Upstream result1 = roundRobinLoadBalancer.select(upstreamList, "");
+ assertNotNull(result1, "Selected upstream should not be null");
+ assertTrue(upstreamList.contains(result1), "Selected upstream should
be from the provided list");
+
+ // Test with equal weight upstream list
+ Upstream result2 = roundRobinLoadBalancer.select(upstreamList2, "");
+ assertNotNull(result2, "Selected upstream should not be null");
+ assertTrue(upstreamList2.contains(result2), "Selected upstream should
be from the provided list");
+
+ // Test multiple selections to verify round-robin behavior
+ Map<String, Integer> countMap = new HashMap<>();
+ IntStream.range(0, 30).forEach(i -> {
Review Comment:
The magic number 30 should be extracted to a constant or variable with a
descriptive name like `SELECTION_ITERATIONS` to improve code readability and
maintainability.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
