Skip to content

Commit

Permalink
Fix cache key computation for generated mock/spy classes (#2400)
Browse files Browse the repository at this point in the history
Add defaultAnswer to the `MockitoMockKey` to distinguish the mock types, i.e. to separate mocks from spies.
Otherwise, spies and mocks of the same class might accidentally rely on the same generated type class,
resulting in spurious test failures (depending on which mock/spy is created first).

Fixes #2399
  • Loading branch information
vyazelenko committed Aug 24, 2021
1 parent b04c703 commit 4f81d4f
Show file tree
Hide file tree
Showing 4 changed files with 145 additions and 3 deletions.
Expand Up @@ -5,12 +5,14 @@
package org.mockito.internal.creation.bytebuddy;

import java.lang.ref.ReferenceQueue;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.locks.ReadWriteLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;

import net.bytebuddy.TypeCache;
import org.mockito.mock.SerializableMode;
import org.mockito.stubbing.Answer;

class TypeCachingBytecodeGenerator extends ReferenceQueue<ClassLoader>
implements BytecodeGenerator {
Expand Down Expand Up @@ -43,7 +45,8 @@ public <T> Class<T> mockClass(final MockFeatures<T> params) {
params.mockedType,
params.interfaces,
params.serializableMode,
params.stripAnnotations),
params.stripAnnotations,
params.defaultAnswer),
() -> bytecodeGenerator.mockClass(params),
BOOTSTRAP_LOCK);
} catch (IllegalArgumentException exception) {
Expand Down Expand Up @@ -83,15 +86,18 @@ private static class MockitoMockKey extends TypeCache.SimpleKey {

private final SerializableMode serializableMode;
private final boolean stripAnnotations;
private final Answer defaultAnswer;

private MockitoMockKey(
Class<?> type,
Set<Class<?>> additionalType,
SerializableMode serializableMode,
boolean stripAnnotations) {
boolean stripAnnotations,
Answer defaultAnswer) {
super(type, additionalType);
this.serializableMode = serializableMode;
this.stripAnnotations = stripAnnotations;
this.defaultAnswer = defaultAnswer;
}

@Override
Expand All @@ -107,14 +113,16 @@ public boolean equals(Object object) {
}
MockitoMockKey that = (MockitoMockKey) object;
return stripAnnotations == that.stripAnnotations
&& serializableMode.equals(that.serializableMode);
&& serializableMode.equals(that.serializableMode)
&& Objects.equals(defaultAnswer, that.defaultAnswer);
}

@Override
public int hashCode() {
int result = super.hashCode();
result = 31 * result + (stripAnnotations ? 1 : 0);
result = 31 * result + serializableMode.hashCode();
result = 31 * result + Objects.hashCode(defaultAnswer);
return result;
}
}
Expand Down
Expand Up @@ -14,6 +14,8 @@
import java.lang.ref.ReferenceQueue;
import java.lang.ref.WeakReference;
import java.util.Collections;
import java.util.IdentityHashMap;
import java.util.Set;
import java.util.WeakHashMap;

import org.junit.Before;
Expand Down Expand Up @@ -142,6 +144,42 @@ public void ensure_cache_returns_different_instance_serializableMode() throws Ex
assertThat(other_mock_type).isNotSameAs(the_mock_type);
}

@Test
public void ensure_cache_returns_different_instance_defaultAnswer() throws Exception {
// given
ClassLoader classloader_with_life_shorter_than_cache =
inMemoryClassLoader()
.withClassDefinition("foo.Bar", makeMarkerInterface("foo.Bar"))
.build();

TypeCachingBytecodeGenerator cachingMockBytecodeGenerator =
new TypeCachingBytecodeGenerator(new SubclassBytecodeGenerator(), true);

Answers[] answers = Answers.values();
Set<Class<?>> classes = Collections.newSetFromMap(new IdentityHashMap<>());
classes.add(
cachingMockBytecodeGenerator.mockClass(
withMockFeatures(
classloader_with_life_shorter_than_cache.loadClass("foo.Bar"),
Collections.<Class<?>>emptySet(),
SerializableMode.NONE,
false,
null)));
for (Answers answer : answers) {
Class<?> klass =
cachingMockBytecodeGenerator.mockClass(
withMockFeatures(
classloader_with_life_shorter_than_cache.loadClass("foo.Bar"),
Collections.<Class<?>>emptySet(),
SerializableMode.NONE,
false,
answer));
assertThat(classes.add(klass)).isTrue();
}

assertThat(classes).hasSize(answers.length + 1);
}

@Test
public void
validate_simple_code_idea_where_weakhashmap_with_classloader_as_key_get_GCed_when_no_more_references()
Expand Down
@@ -0,0 +1,48 @@
/*
* Copyright (c) 2007 Mockito contributors
* This program is made available under the terms of the MIT License.
*/
package org.mockitousage.spies;

import org.junit.Test;
import org.mockitoutil.TestBase;

import static org.junit.Assert.assertEquals;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.spy;

public class MockCreationShouldNotAffectSpyTest extends TestBase {

@Test
public void test() {
TestClass instance = new TestClass(42);

TestClass mock = mock(TestClass.class);
assertEquals(mock.hashCode(), mock.hashCode());
assertEquals(mock, mock);

TestClass spy = spy(instance);
assertEquals(instance.hashCode(), spy.hashCode());
assertEquals(spy, instance);
assertEquals(instance, spy);
}

static class TestClass {
private final long value;

TestClass(final long value) {
this.value = value;
}

public boolean equals(final Object o) {
if (!(o instanceof TestClass)) {
return false;
}
return value == ((TestClass) o).value;
}

public int hashCode() {
return Long.hashCode(value);
}
}
}
@@ -0,0 +1,48 @@
/*
* Copyright (c) 2007 Mockito contributors
* This program is made available under the terms of the MIT License.
*/
package org.mockitousage.spies;

import org.junit.Test;
import org.mockitoutil.TestBase;

import static org.junit.Assert.assertEquals;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.spy;

public class SpyCreationShouldNotAffectMockTest extends TestBase {

@Test
public void test() {
TestClass instance = new TestClass(Long.MIN_VALUE);

TestClass spy = spy(instance);
assertEquals(instance.hashCode(), spy.hashCode());
assertEquals(spy, instance);
assertEquals(instance, spy);

TestClass mock = mock(TestClass.class);
assertEquals(mock.hashCode(), mock.hashCode());
assertEquals(mock, mock);
}

static class TestClass {
private final long value;

TestClass(final long value) {
this.value = value;
}

public boolean equals(final Object o) {
if (!(o instanceof TestClass)) {
return false;
}
return value == ((TestClass) o).value;
}

public int hashCode() {
return Long.hashCode(value);
}
}
}

0 comments on commit 4f81d4f

Please sign in to comment.