apolloconfig / apollo

Apollo is a reliable configuration management system suitable for microservice configuration management scenarios.
https://www.apolloconfig.com
Apache License 2.0
29.08k stars 10.2k forks source link

Fixed flaky test #5007

Closed wang3820 closed 10 months ago

wang3820 commented 11 months ago

What's the purpose of this PR

To fix flaky testing at com.ctrip.framework.apollo.configservice.service.AppNamespaceServiceWithCacheTest#testAppNamespace

Which issue(s) this PR fixes:

com.ctrip.framework.apollo.configservice.service.AppNamespaceServiceWithCacheTest#testAppNamespace is flaky. Nondex was used to check and locate the flakiness in the test. The test can be reproduced using the following command:

mvn install -pl apollo-configservice -am -DskipTests
mvn -pl apollo-configservice test -Dtest=com.ctrip.framework.apollo.configservice.service.AppNamespaceServiceWithCacheTest#testAppNamespace
mvn -pl apollo-configservice edu.illinois:nondex-maven-plugin:2.1.1:nondex -Dtest=com.ctrip.framework.apollo.configservice.service.AppNamespaceServiceWithCacheTest#testAppNamespace

Brief changelog

Implementation of appNamespaceIdCache was changed from https://github.com/wang3820/apollo/blob/5e49df687c64cf6f64f0210ba3ebc13084468757/apollo-configservice/src/main/java/com/ctrip/framework/apollo/configservice/service/AppNamespaceServiceWithCache.java#L87 to https://github.com/wang3820/apollo/blob/66b66a972552733339d154504c785907d30c5249/apollo-configservice/src/main/java/com/ctrip/framework/apollo/configservice/service/AppNamespaceServiceWithCache.java#L72

Maps.newConcurrentMap() returns a ConcurrentHashMap object and its order is not maintained. When keySet() is called, this results in returning a null object.

The nondex debug log:

TEST: com.ctrip.framework.apollo.configservice.service.AppNamespaceServiceWithCacheTest#testAppNamespace
java.base/java.lang.Thread.getStackTrace(Thread.java:1602)
java.base/edu.illinois.nondex.common.NonDex.printStackTraceIfUniqueDebugPoint(NonDex.java:165)
java.base/edu.illinois.nondex.common.NonDex.shouldExplore(NonDex.java:136)
java.base/edu.illinois.nondex.common.NonDex.getPermutation(NonDex.java:106)
java.base/edu.illinois.nondex.shuffling.ControlNondeterminism.shuffle(ControlNondeterminism.java:93)
java.base/java.util.concurrent.ConcurrentHashMap$Traverser.<init>(ConcurrentHashMap.java:3342)
java.base/java.util.concurrent.ConcurrentHashMap$BaseIterator.<init>(ConcurrentHashMap.java:3424)
java.base/java.util.concurrent.ConcurrentHashMap$KeyIterator.<init>(ConcurrentHashMap.java:3445)
java.base/java.util.concurrent.ConcurrentHashMap$KeySetView.iterator(ConcurrentHashMap.java:4625)
java.base/java.util.concurrent.ConcurrentHashMap$CollectionView.toArray(ConcurrentHashMap.java:4454)
java.base/java.util.ArrayList.<init>(ArrayList.java:179)
com.google.common.collect.Lists.newArrayList(Lists.java:146)
com.ctrip.framework.apollo.configservice.service.AppNamespaceServiceWithCache.updateAndDeleteCache(AppNamespaceServiceWithCache.java:181)
com.ctrip.framework.apollo.configservice.service.AppNamespaceServiceWithCache.lambda$afterPropertiesSet$0(AppNamespaceServiceWithCache.java:124)

Follow this checklist to help us incorporate your contribution quickly and easily:

github-actions[bot] commented 11 months ago

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

wang3820 commented 11 months ago

I have read the CLA Document and I hereby sign the CLA

wang3820 commented 11 months ago

recheck

codecov[bot] commented 11 months ago

Codecov Report

Merging #5007 (1f795cb) into master (bc55ba6) will increase coverage by 0.34%. Report is 1 commits behind head on master. The diff coverage is 91.66%.

@@             Coverage Diff              @@
##             master    #5007      +/-   ##
============================================
+ Coverage     49.27%   49.61%   +0.34%     
- Complexity     1889     1908      +19     
============================================
  Files           372      372              
  Lines         11538    11553      +15     
  Branches       1123     1127       +4     
============================================
+ Hits           5685     5732      +47     
+ Misses         5513     5479      -34     
- Partials        340      342       +2     
Files Coverage Δ
...mework/apollo/biz/service/AppNamespaceService.java 32.46% <ø> (ø)
...gservice/service/AppNamespaceServiceWithCache.java 81.61% <100.00%> (ø)
...p/framework/apollo/portal/api/AdminServiceAPI.java 13.19% <ø> (ø)
.../apollo/portal/controller/AccessKeyController.java 22.22% <ø> (ø)
...mework/apollo/portal/controller/AppController.java 29.33% <ø> (ø)
...rk/apollo/portal/controller/ClusterController.java 36.36% <ø> (ø)
...o/portal/controller/NamespaceBranchController.java 18.91% <ø> (ø)
.../apollo/portal/controller/NamespaceController.java 11.95% <ø> (ø)
...apollo/portal/controller/PermissionController.java 5.75% <ø> (ø)
...ollo/portal/controller/ServerConfigController.java 85.71% <ø> (ø)
... and 6 more

... and 2 files with indirect coverage changes

nobodyiam commented 11 months ago

There was a similar pr #4998 merged, does the issue this pr try to solve still exist?

wang3820 commented 10 months ago

There was a similar pr #4998 merged, does the issue this pr try to solve still exist?

This PR appears to be reduntant as #4998 fixes the same issue. Closing this PR.