You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@curator.apache.org by "MyEnthusiastic (via GitHub)" <gi...@apache.org> on 2023/11/22 16:11:41 UTC
[PR] Improve test reliability by resolving nondeterministic order by sorting [curator]
MyEnthusiastic opened a new pull request, #491:
URL: 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)
# 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]
```
--
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: commits-unsubscribe@curator.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve test reliability by resolving nondeterministic order by sorting [curator]
Posted by "eolivelli (via GitHub)" <gi...@apache.org>.
eolivelli commented on PR #491:
URL: https://github.com/apache/curator/pull/491#issuecomment-1825511972
Thank you very much.
Let me create a JIRA and attach it to this PR.
Cc @tisonkun
--
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: commits-unsubscribe@curator.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve test reliability by resolving nondeterministic order by sorting [curator]
Posted by "MyEnthusiastic (via GitHub)" <gi...@apache.org>.
MyEnthusiastic commented on PR #491:
URL: https://github.com/apache/curator/pull/491#issuecomment-1824986905
> Can you please fix the code style? See the error on CI
Fixed, thanks!
--
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: commits-unsubscribe@curator.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CURATOR-694] Improve test reliability by resolving nondeterministic order by sorting [curator]
Posted by "MyEnthusiastic (via GitHub)" <gi...@apache.org>.
MyEnthusiastic commented on PR #491:
URL: https://github.com/apache/curator/pull/491#issuecomment-1836715197
@eolivelli Hey is it OK to merge that?
--
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: commits-unsubscribe@curator.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve test reliability by resolving nondeterministic order by sorting [curator]
Posted by "MyEnthusiastic (via GitHub)" <gi...@apache.org>.
MyEnthusiastic commented on PR #491:
URL: https://github.com/apache/curator/pull/491#issuecomment-1825847544
Sorry --- why the CI crushed --- I'm confused and looks like not related with this change
--
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: commits-unsubscribe@curator.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CURATOR-694] Improve test reliability by resolving nondeterministic order by sorting [curator]
Posted by "MyEnthusiastic (via GitHub)" <gi...@apache.org>.
MyEnthusiastic commented on PR #491:
URL: https://github.com/apache/curator/pull/491#issuecomment-1837070038
@tisonkun Cool! hey the CI had passed
--
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: commits-unsubscribe@curator.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CURATOR-694] Improve test reliability by resolving nondeterministic order by sorting [curator]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun merged PR #491:
URL: https://github.com/apache/curator/pull/491
--
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: commits-unsubscribe@curator.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve test reliability by resolving nondeterministic order by sorting [curator]
Posted by "eolivelli (via GitHub)" <gi...@apache.org>.
eolivelli commented on PR #491:
URL: https://github.com/apache/curator/pull/491#issuecomment-1824895902
Can you please fix the code style?
See the error on CI
--
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: commits-unsubscribe@curator.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org