Repository navigation
Fixes a bug in WindmillSet.addIfAbsent. - #40748
Conversation
There was a problem hiding this comment.
📋 Review Summary
This PR fixes a bug in WindmillSet.addIfAbsent where the inverted null check on windmillMap.putIfAbsent's result caused the method to always return false. It also introduces a comprehensive unit test suite in WindmillSetTest covering addIfAbsent as well as core SetState operations.
🔍 General Feedback
- The fix accurately aligns
WindmillSet.addIfAbsentwith theSetState.addIfAbsentandMapState.putIfAbsentcontracts (putIfAbsentreturnsnullwhen the key was absent, meaning the element was newly added to the set). - The newly added unit tests are clean, well-structured, and thoroughly test the expected behavior.
💡 Tip: Comment /gemini review on this pull request to trigger a new Gemini code review.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #40748 +/- ##
=============================================
+ Coverage 56.16% 75.97% +19.80%
+ Complexity 2288 2273 -15
=============================================
Files 1124 259 -865
Lines 178336 14377 -163959
Branches 1489 1479 -10
=============================================
- Hits 100170 10923 -89247
+ Misses 75645 2763 -72882
+ Partials 2521 691 -1830
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Assigning reviewers: R: @chamikaramj added as fallback since no labels match configuration Note: If you would like to opt out of this review, comment Available commands:
The PR bot will only process comments in the main thread (not review comments). |
|
In case this helps:
The implementation calls |
putState.read() returns
nullif the element was newly added, so this method was always returningfalse.Added a unit test demonstrating this bug and since there were no basic unit tests for this class, added some as well.