[ 
https://issues.apache.org/jira/browse/CURATOR-694?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

huiyang chi updated CURATOR-694:
--------------------------------
    Description: 
h2. {color:#172b4d}The related PR has been opened :{color} 
[https://github.com/apache/curator/pull/491]

 
 # Problem

For these tests:

[INFO] org.apache.curator.framework.imps.TestReconfiguration#testAdd
[INFO] 
org.apache.curator.framework.imps.TestReconfiguration#testNewMembersWithEmptyList
[INFO] org.apache.curator.framework.imps.TestReconfiguration#testAddAsync
[INFO] org.apache.curator.framework.imps.TestReconfiguration#testAddAndRemove
[INFO] 
org.apache.curator.framework.imps.TestReconfiguration#testAddAndRemoveWithEmptyList
[INFO] org.apache.curator.framework.imps.TestReconfiguration#testBasicGetConfig
[INFO] 
org.apache.curator.framework.imps.TestReconfiguration#testRemoveWithChroot

They assume the order of the QuorumServer was same, but this is not necessary 
the case because QuorumMaj aggressively use Map to store it's data

```
    private Map<Long, QuorumPeer.QuorumServer> allMembers = new HashMap();
    private HashMap<Long, QuorumPeer.QuorumServer> votingMembers = new 
HashMap();
    private HashMap<Long, QuorumPeer.QuorumServer> observingMembers = new 
HashMap();
```
so because the order of the Map is indeterministic according to the [Oracle's 
official 
document]([https://docs.oracle.com/javase/8/docs/api/java/util/HashMap.html#:~:text=This%20class%20makes%20no%20guarantees%20as%20to%20the%20order%20of%20the%20map]),
 so this might trigger error message in different environments:

```
[ERROR] org.apache.curator.framework.imps.TestReconfiguration.testAdd  Time 
elapsed: 0.641 s  <<< FAILURE!
org.opentest4j.AssertionFailedError: expected: 
<127.0.0.1:55254,127.0.0.1:55251,127.0.0.1:55248,0.0.0.0:55268> but was: 
<0.0.0.0:55268,127.0.0.1:55251,127.0.0.1:55248,127.0.0.1:55254>
        at 
org.apache.curator.framework.imps.TestReconfiguration.testAdd(TestReconfiguration.java:281)

```
 # Solution

So the most precise way to deal with that would be change the 
configToConnectionString method to make sure when we serialize the 
configurations, we always get the same result, so that the tests will be safe 
in different environment (because the order of the map is undefined)

  was:
h2. {color:#ff0000}{color:#172b4d}The related PR has been opened :{color} 
[https://github.com/apache/curator/pull/491]{color}

 
 # Problem

For these tests:

[INFO] org.apache.curator.framework.imps.TestReconfiguration#testAdd
[INFO] 
org.apache.curator.framework.imps.TestReconfiguration#testNewMembersWithEmptyList
[INFO] org.apache.curator.framework.imps.TestReconfiguration#testAddAsync
[INFO] org.apache.curator.framework.imps.TestReconfiguration#testAddAndRemove
[INFO] 
org.apache.curator.framework.imps.TestReconfiguration#testAddAndRemoveWithEmptyList
[INFO] org.apache.curator.framework.imps.TestReconfiguration#testBasicGetConfig
[INFO] 
org.apache.curator.framework.imps.TestReconfiguration#testRemoveWithChroot

They assume the order of the QuorumServer was same, but this is not necessary 
the case because QuorumMaj aggressively use Map to store it's data

```
    private Map<Long, QuorumPeer.QuorumServer> allMembers = new HashMap();
    private HashMap<Long, QuorumPeer.QuorumServer> votingMembers = new 
HashMap();
    private HashMap<Long, QuorumPeer.QuorumServer> observingMembers = new 
HashMap();
```
so because the order of the Map is indeterministic according to the [Oracle's 
official 
document]([https://docs.oracle.com/javase/8/docs/api/java/util/HashMap.html#:~:text=This%20class%20makes%20no%20guarantees%20as%20to%20the%20order%20of%20the%20map]),
 so this might trigger error message in different environments:

```
[ERROR] org.apache.curator.framework.imps.TestReconfiguration.testAdd  Time 
elapsed: 0.641 s  <<< FAILURE!
org.opentest4j.AssertionFailedError: expected: 
<127.0.0.1:55254,127.0.0.1:55251,127.0.0.1:55248,0.0.0.0:55268> but was: 
<0.0.0.0:55268,127.0.0.1:55251,127.0.0.1:55248,127.0.0.1:55254>
        at 
org.apache.curator.framework.imps.TestReconfiguration.testAdd(TestReconfiguration.java:281)

```
 # Solution

So the most precise way to deal with that would be change the 
configToConnectionString method to make sure when we serialize the 
configurations, we always get the same result, so that the tests will be safe 
in different environment (because the order of the map is undefined)


> Improve test reliability by resolving nondeterministic order by sorting
> -----------------------------------------------------------------------
>
>                 Key: CURATOR-694
>                 URL: https://issues.apache.org/jira/browse/CURATOR-694
>             Project: Apache Curator
>          Issue Type: Improvement
>          Components: Framework
>    Affects Versions: 5.5.0
>         Environment: # Reproduce
> This error can be reproduced with the 
> [NondexTool](https://github.com/TestingResearchIllinois/NonDex) with the 
> following command (you can try that without modifying the pom, but feel free 
> to use the plugin to protect your project and make it safer 😊 ), and this 
> kind problem is widely documented in [International Dataset of Flaky Tests 
> (IDoFT)](https://github.com/TestingResearchIllinois/idoft)
> ```
> mvn edu.illinois:nondex-maven-plugin:2.1.7-SNAPSHOT:nondex -pl 
> /curator-framework -Dsurefire.useFile=false 
> -Dtest=org.apache.curator.framework.imps.TestReconfiguration#[methodName]
> ```
> Let's make the curator more reliable together 👩🏻‍🔧
>            Reporter: huiyang chi
>            Priority: Major
>              Labels: fixed
>             Fix For: awaiting-response
>
>
> h2. {color:#172b4d}The related PR has been opened :{color} 
> [https://github.com/apache/curator/pull/491]
>  
>  # Problem
> For these tests:
> [INFO] org.apache.curator.framework.imps.TestReconfiguration#testAdd
> [INFO] 
> org.apache.curator.framework.imps.TestReconfiguration#testNewMembersWithEmptyList
> [INFO] org.apache.curator.framework.imps.TestReconfiguration#testAddAsync
> [INFO] org.apache.curator.framework.imps.TestReconfiguration#testAddAndRemove
> [INFO] 
> org.apache.curator.framework.imps.TestReconfiguration#testAddAndRemoveWithEmptyList
> [INFO] 
> org.apache.curator.framework.imps.TestReconfiguration#testBasicGetConfig
> [INFO] 
> org.apache.curator.framework.imps.TestReconfiguration#testRemoveWithChroot
> They assume the order of the QuorumServer was same, but this is not necessary 
> the case because QuorumMaj aggressively use Map to store it's data
> ```
>     private Map<Long, QuorumPeer.QuorumServer> allMembers = new HashMap();
>     private HashMap<Long, QuorumPeer.QuorumServer> votingMembers = new 
> HashMap();
>     private HashMap<Long, QuorumPeer.QuorumServer> observingMembers = new 
> HashMap();
> ```
> so because the order of the Map is indeterministic according to the [Oracle's 
> official 
> document]([https://docs.oracle.com/javase/8/docs/api/java/util/HashMap.html#:~:text=This%20class%20makes%20no%20guarantees%20as%20to%20the%20order%20of%20the%20map]),
>  so this might trigger error message in different environments:
> ```
> [ERROR] org.apache.curator.framework.imps.TestReconfiguration.testAdd  Time 
> elapsed: 0.641 s  <<< FAILURE!
> org.opentest4j.AssertionFailedError: expected: 
> <127.0.0.1:55254,127.0.0.1:55251,127.0.0.1:55248,0.0.0.0:55268> but was: 
> <0.0.0.0:55268,127.0.0.1:55251,127.0.0.1:55248,127.0.0.1:55254>
>         at 
> org.apache.curator.framework.imps.TestReconfiguration.testAdd(TestReconfiguration.java:281)
> ```
>  # Solution
> So the most precise way to deal with that would be change the 
> configToConnectionString method to make sure when we serialize the 
> configurations, we always get the same result, so that the tests will be safe 
> in different environment (because the order of the map is undefined)



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to