-
Notifications
You must be signed in to change notification settings - Fork 26.6k
Fix: Stabilize ReferenceCacheTest by Disabling Metrics Initialization and Ensuring Test Isolation
#15794
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 3.3 #15794 +/- ##
============================================
- Coverage 60.76% 60.76% -0.01%
+ Complexity 11698 11694 -4
============================================
Files 1938 1938
Lines 88646 88646
Branches 13379 13379
============================================
- Hits 53868 53866 -2
- Misses 29250 29252 +2
Partials 5528 5528
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
zrlw
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
maybe you could combine similar revisions into a single PR to reduce unnecessary repetitive work. |
|
I will definitely keep that in mind for future. Just to be safe and make PR's more easily acceptable I added changes one test file per PR |
|
These PRs are very good. Thanks for your great work. I want to suggest: how about using one issue to describe these changes and some sub-issues to refer to these PRs? I think it would be more structured and let others know what is happening and why. @Anshul-creator |
|
And we will know when these fixes are done. ^^ |
RainYuY
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
What is the purpose of the change?
This PR stabilizes several tests inside
ReferenceCacheTestthat were intermittently failing under randomized execution orders (NonDex):testGetCacheSameReferencetestGetCacheDiffReferencetestGetCacheWithKeytestGetCacheDiffNametestDestroytestDestroyAllAll of these tests invoke reference creation through
ReferenceConfig.get(), which cause the metrics subsystem to be initialized. This introduces nondeterministic behavior that causes intermittent test failures unrelated to the core reference-caching logic.Root Cause
These failures are identical to previously fixed issues in:
ExporterSideConfigUrlTestby disabling metrics initialization during URL export #15778MethodConfigTestby disabling metrics initialization to avoid Micrometer nondeterminism #15782ServiceConfigTestby isolating system properties and cached state #15785Each of those PRs addressed flakiness caused by Micrometer’s
CompositeMeterRegistry, which can throw anArrayIndexOutOfBoundsExceptionwhen it iterates over internalIdentityHashMapstructures under randomized execution orders (NonDex).ReferenceCacheTestsuffered from the same underlying issue:Dubbo framework state was leaking between test methods. This meant that the behavior of reference creation in one test could affect subsequent tests, causing nondeterministic failures.
Changes Made
To fully isolate each test method and eliminate shared state, the following changes were applied:
Together, these changes ensure that every test in
ReferenceCacheTeststarts from a completely clean, predictable environment, eliminating flaky behavior caused by shared or leaked state.Verification
You can try running the following snippet of code from the dubbo repo root, on both pre-fix and post-fix code
The tests should fail intermittently on the pre-fix version, but pass consistently across all seeds on the post-fix version.
NonDex run logs will be available under the
dubbo-config/dubbo-config-api/.nondexdirectory.Checklist