Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 3.3 #15662 +/- ##
============================================
- Coverage 61.06% 61.01% -0.05%
- Complexity 11704 11724 +20
============================================
Files 1923 1923
Lines 87069 87069
Branches 13112 13112
============================================
- Hits 53165 53128 -37
- Misses 28465 28499 +34
- Partials 5439 5442 +3
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:
|
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a race condition in the Mutiny test case that was causing intermittent NullPointerException failures in CI environments. The issue occurred when data emission started before the downstream subscriber was properly established.
Key changes:
- Implemented proper synchronization using
CountDownLatchto coordinate subscription and data emission timing - Replaced direct collection with
AssertSubscriberfor better reactive stream testing - Added timeout protection and proper error handling for interrupted threads
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| public void request(int n) { | ||
| /* no-op */ |
There was a problem hiding this comment.
[nitpick] The comment '/* no-op */' should be more descriptive to explain why this method is intentionally empty in the test context.
| public void request(int n) { | |
| /* no-op */ | |
| // Intentionally left empty: flow control is not required for this test's mock observer. |
Problem Description
Ref: #15660
The
MutinyClientCallsTest.testOneToManyReturnsMultiAndEmitsItems()test case encounters aNullPointerExceptionin certain environments:This error occurs when the test attempts to call
publisher.onNext()before the downstream subscriber is properly established, resulting in a nulldownstreamreference.Root Cause Analysis
Why Local Tests Pass
In local development environments, tests typically pass due to:
Why GitHub CI Fails Intermittently
GitHub CI environments are prone to this race condition because of:
The core issue is a race condition where the data-emitting thread starts before the
Multisubscription is fully established.Solution Comparison
Approach 1: Original Code (Problematic)
Related issues:
Approach 2: Thread.sleep() (Workaround)
Characteristics:
Approach 3: AssertSubscriber with CountDownLatch (Recommended)
Advantages:
Final Solution Implementation
Here's the complete fixed test method:
Key Benefits of This Solution
AssertSubscriberfor proper reactive stream testingThis solution addresses the fundamental race condition while maintaining the test's original intent and providing robust execution guarantees across all environments.
Checklist