Skip to content

Replace constant hashcode#3832

Open
mirkoalicastro wants to merge 4 commits into
mockito:mainfrom
mirkoalicastro:fix/constant-hashcode-invocation-matchers
Open

Replace constant hashcode#3832
mirkoalicastro wants to merge 4 commits into
mockito:mainfrom
mirkoalicastro:fix/constant-hashcode-invocation-matchers

Conversation

@mirkoalicastro

Copy link
Copy Markdown
Contributor

A few classes (InterceptedInvocation, SerializableMethod, Equals) returned a constant number (1) as their hashcode implementation, collapsing every instance into one bucket and degrading HashMap/HashSet lookups to linear complexity.

This PR fixes it (and removes corresponding TODO from the code) so that each hashcode derives from the same fields taken into consideration for their equals.

Checklist

  • Read the contributing guide
  • PR should be motivated, i.e. what does it fix, why, and if relevant how
  • If possible / relevant include an example in the description, that could help all readers
    including project members to get a better picture of the change
  • Avoid other runtime dependencies
  • Meaningful commit history ; intention is important please rebase your commit history so that each
    commit is meaningful and help the people that will explore a change in 2 years
  • The pull request follows coding style (run ./gradlew spotlessApply for auto-formatting)
  • Mention Fixes #<issue number> in the description if relevant
  • At least one commit should end with Fixes #<issue number> if relevant

@mirkoalicastro
mirkoalicastro force-pushed the fix/constant-hashcode-invocation-matchers branch from 249ac2d to cf722a1 Compare June 12, 2026 20:57
@codecov-commenter

codecov-commenter commented Jun 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 71.42857% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.82%. Comparing base (d6cb323) to head (0fe90cd).

Files with missing lines Patch % Lines
...ockito/internal/invocation/SerializableMethod.java 55.55% 0 Missing and 4 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #3832      +/-   ##
============================================
- Coverage     86.84%   86.82%   -0.02%     
- Complexity     3044     3048       +4     
============================================
  Files           344      344              
  Lines          9172     9183      +11     
  Branches       1135     1139       +4     
============================================
+ Hits           7965     7973       +8     
+ Misses          917      915       -2     
- Partials        290      295       +5     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@raphw

raphw commented Jun 13, 2026

Copy link
Copy Markdown
Member

If I remember correctly, the problem is hashing class identity. This will break with serialization. Serializing on strings should be fine, though, that's a thing we should certainly improve.

@mirkoalicastro

Copy link
Copy Markdown
Contributor Author

If I remember correctly, the problem is hashing class identity. This will break with serialization. Serializing on strings should be fine, though, that's a thing we should certainly improve.

You're right, and I learned something here 😄

I've changed it to hash on the class names instead of the Class objects to achieve a portable hashCode implementation. The equals contract still holds: equal methods resolve to the same classes and therefore the same names.

Thanks for the catch!

Let me know your thoughts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants