apache / apache/eventmesh

[Enhancement] The method of verifying no exception thrown is not appropriate.

Open
#3,887 1 comment 0 reactions 0 assignees View on GitHub
enhancement Stale
Dominant language
Java
Stars
1.8k
Forks
658
Avg merge
11h 29m
Merged PRs (30d)
52

Description

### Search before asking

- [X] I had searched in the [issues](https://github.com/apache/eventmesh/issues?q=is%3Aissue) and found no similar issues.

### Enhancement Request

**org.apache.eventmesh.common.loadbalance.RandomLoadBalanceSelectorTest**
```
{
......
// just assert success if no exception
Assert.assertTrue(true);
}
```
This method of verifying no exception thrown is not appropriate.

---
**org.apache.eventmesh.common.loadbalance.WeightRandomLoadBalanceSelectorTest**
```
// testRange = 100000
Assert.assertTrue(Math.abs(addressToNum.get("192.168.0.3") - addressToNum.get("192.168.0.2") * 2) < testRange / 20);
Assert.assertTrue(Math.abs(addressToNum.get("192.168.0.3") - addressToNum.get("192.168.0.1") * 4) < testRange / 20);
```
[bug]: Map `addressToNum` is the result of weighted random selection for 100000 times. Key is a address and value is times. The weight of "192.168.0.3":"192.168.0.2":"192.168.0.1" is 4:2:1. So `addressToNum.get("192.168.0.3")` should ≈ `addressToNum.get("192.168.0.2") * 2`.

Three cases, pairwise verification should be conducted, otherwise the verification is insufficient.

In java 9 and later version, Reflection access private field is regard as illegal reflective access operation. It cause warning, even exception according to VM options.

---
**org.apache.eventmesh.common.loadbalance.WeightRoundRobinLoadBalanceSelectorTest**
```
Assert.assertTrue(addressToNum.get("B") > addressToNum.get("A"));
```
It is not accurate for verify the weight in WeightRoundRobin.

The capacity of Map in `#testSelect()` is definite.

### Describe the solution you'd like

**org.apache.eventmesh.common.loadbalance.RandomLoadBalanceSelectorTest**

Use `Assert.fail()`

**org.apache.eventmesh.common.loadbalance.WeightRandomLoadBalanceSelectorTest**

[bug]: Set the error rate to 0.05 instead of 5000(=100000/20).

Verify the ratio of "192.168.0.2" : "192.168.0.1"

User getter access private field.

**org.apache.eventmesh.common.loadbalance.WeightRoundRobinLoadBalanceSelectorTest**

Use `Assert.assertEquals(excepted, actual, delta)`

Init map with a specified capacity.

### Are you willing to submit PR?

- [X] Yes I am willing to submit a PR!

Contributor guide

Open the contributing guide

Research direction

Start with RandomLoadBalanceSelectorTest, WeightRandomLoadBalanceSelectorTest, and WeightRoundRobinLoadBalanceSelectorTest, especially their testSelect methods and the addressToNum map checks. Review the existing assertions and private-field access, then run these selector tests. Done means the tests fail for incorrect selection behavior without relying on an unconditional success assertion, and validate the stated weighted ratios accurately.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
testing
Issue type
Refactor
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.