Avoid cache breakage #2402
Avoid cache breakage #2402
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2402 +/- ##
============================================
- Coverage 84.69% 84.68% -0.01%
+ Complexity 2797 2796 -1
============================================
Files 329 329
Lines 8518 8508 -10
Branches 1033 1031 -2
============================================
- Hits 7214 7205 -9
Misses 1021 1021
+ Partials 283 282 -1
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
My bad for not fully understanding the domain space. I mostly based my reviews on the passing tests and did not consider the fact that the cache would be broken altogether. It would be great if we can have some sort of test that verifies the cache is working as expected, but I suppose that is quite difficult to do?
In any case, thanks for catching this!
| import static org.mockito.Mockito.mock; | ||
| import static org.mockito.Mockito.spy; | ||
|
|
||
| public class MockCreationShouldNotAffectSpyTest extends TestBase { |
There was a problem hiding this comment.
Can we still keep these tests? I think they are valuable, especially when we attempt to reland the fix at a later point in time.
| import static org.mockito.Mockito.mock; | ||
| import static org.mockito.Mockito.spy; | ||
|
|
||
| public class SpyCreationShouldNotAffectMockTest extends TestBase { |
There was a problem hiding this comment.
I do not think these tests make sense as things currently are in Mockito. They basically test for the feature that is removed. I think there's a better way for test for this once we choose to enable hashCode/equals enablement.
| assertEquals(mock, mock); | ||
|
|
||
| TestClass spy = spy(instance); | ||
| assertEquals(instance.hashCode(), spy.hashCode()); |
There was a problem hiding this comment.
I guess we can't reinstate the test as-is, but I think we can if we remove this particular line?
Removes a bad patch that introduces special treatment for equals/hashCode based on a particular default answer. This would require a more general overhaul as it is a major behavioral change and breaks the type cache.