Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #2648 : Add support for customising strictness via @Mock annotation and MockSettings #2650

Merged
merged 5 commits into from May 26, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/main/java/org/mockito/Mock.java
Expand Up @@ -13,6 +13,7 @@
import java.lang.annotation.Target;

import org.mockito.junit.MockitoJUnitRunner;
import org.mockito.quality.Strictness;
import org.mockito.stubbing.Answer;

/**
Expand Down Expand Up @@ -105,10 +106,21 @@
boolean serializable() default false;

/**
* @deprecated Use {@link Mock#strictness()} instead.
*
* Mock will be lenient, see {@link MockSettings#lenient()}.
* For examples how to use 'Mock' annotation and parameters see {@link Mock}.
*
* @since 2.23.3
*/
@Deprecated
boolean lenient() default false;
TimvdLippe marked this conversation as resolved.
Show resolved Hide resolved

/**
* Mock will have custom strictness, see {@link MockSettings#strictness(Strictness)}.
* For examples how to use 'Mock' annotation and parameters see {@link Mock}.
*
* @since 4.6.0
*/
Strictness strictness() default Strictness.STRICT_STUBS;
}
20 changes: 20 additions & 0 deletions src/main/java/org/mockito/MockSettings.java
Expand Up @@ -350,6 +350,8 @@ public interface MockSettings extends Serializable {
<T> MockCreationSettings<T> buildStatic(Class<T> classToMock);

/**
* @deprecated Use {@link MockSettings#strictness(Strictness)} instead.
*
* Lenient mocks bypass "strict stubbing" validation (see {@link Strictness#STRICT_STUBS}).
* When mock is declared as lenient none of its stubbings will be checked for potential stubbing problems such as
* 'unnecessary stubbing' ({@link UnnecessaryStubbingException}) or for 'stubbing argument mismatch' {@link PotentialStubbingProblem}.
Expand All @@ -360,5 +362,23 @@ public interface MockSettings extends Serializable {
*
* For more information and an elaborate example, see {@link Mockito#lenient()}.
*/
@Deprecated
MockSettings lenient();
TimvdLippe marked this conversation as resolved.
Show resolved Hide resolved

/**
* Specifies strictness level for the mock.
* The default strictness level is determined by the rule/runner used.
* If you are using no rule/runner, the default strictness level is LENIENT
*
* <pre class="code"><code class="java">
* Foo defaultStrictMock = mock(Foo.class);
* Foo explicitStrictMock = mock(Foo.class, withSettings().strictness(Strictness.STRICT_STUBS));
* Foo lenientMock = mock(Foo.class, withSettings().strictness(Strictness.LENIENT));
* </code></pre>
*
* @param strictness the strictness level to set on mock
* @return settings instance so that you can fluently specify other settings
* @since 4.6.0
*/
MockSettings strictness(Strictness strictness);
}
16 changes: 16 additions & 0 deletions src/main/java/org/mockito/Mockito.java
Expand Up @@ -105,6 +105,7 @@
* <a href="#49">49. New API for mocking object construction (Since 3.5.0)</a><br/>
* <a href="#50">50. Avoiding code generation when restricting mocks to interfaces (Since 3.12.2)</a><br/>
* <a href="#51">51. New API for marking classes as unmockable (Since 4.1.0)</a><br/>
* <a href="#51">52. New strictness attribute for @Mock annotation and <code>MockSettings.strictness()</code> methods (Since 4.6.0)</a><br/>
* </b>
*
* <h3 id="0">0. <a class="meaningful_link" href="#mockito2" name="mockito2">Migrating to Mockito 2</a></h3>
Expand Down Expand Up @@ -1606,6 +1607,21 @@
* For any class/interface you own that is problematic to mock, you can now mark the class with {@link org.mockito.DoNotMock @DoNotMock}. For usage
* of the annotation and how to ship your own (to avoid a compile time dependency on a test artifact), please see its JavaDoc.
* <p>
*
* <h3 id="52">52. <a class="meaningful_link" href="#mockito_strictness" name="mockito_strictness">
* New strictness attribute for @Mock annotation and <code>MockSettings.strictness()</code> methods (Since 4.6.0)</a></h3>
*
* You can now customize the strictness level for a single mock, either using `@Mock` annotation strictness attribute or
* using `MockSettings.strictness()`. This can be useful if you want all of your mocks to be strict,
* but one of the mocks to be lenient.
*
* <pre class="code"><code class="java">
* &#064;Mock(strictness = Strictness.LENIENT)
* Foo mock;
* // using MockSettings.withSettings()
* Foo mock = Mockito.mock(Foo.class, withSettings().strictness(Strictness.WARN));
* </code></pre>
*
*/
@CheckReturnValue
@SuppressWarnings("unchecked")
Expand Down
Expand Up @@ -45,6 +45,7 @@ public static Object processAnnotationForMock(
if (annotation.stubOnly()) {
mockSettings.stubOnly();
}
mockSettings.strictness(annotation.strictness());
if (annotation.lenient()) {
mockSettings.lenient();
}
Expand Down
Expand Up @@ -12,6 +12,7 @@
import static org.mockito.internal.exceptions.Reporter.extraInterfacesRequiresAtLeastOneInterface;
import static org.mockito.internal.exceptions.Reporter.methodDoesNotAcceptParameter;
import static org.mockito.internal.exceptions.Reporter.requiresAtLeastOneListener;
import static org.mockito.internal.exceptions.Reporter.strictnessDoesNotAcceptNullParameter;
import static org.mockito.internal.util.collections.Sets.newSet;

import java.io.Serializable;
Expand All @@ -33,6 +34,7 @@
import org.mockito.mock.MockCreationSettings;
import org.mockito.mock.MockName;
import org.mockito.mock.SerializableMode;
import org.mockito.quality.Strictness;
import org.mockito.stubbing.Answer;

@SuppressWarnings("unchecked")
Expand Down Expand Up @@ -239,7 +241,16 @@ public <T2> MockCreationSettings<T2> buildStatic(Class<T2> classToMock) {

@Override
public MockSettings lenient() {
this.lenient = true;
this.strictness = Strictness.LENIENT;
return this;
}

@Override
public MockSettings strictness(Strictness strictness) {
this.strictness = strictness;
if (strictness == null) {
throw strictnessDoesNotAcceptNullParameter();
TimvdLippe marked this conversation as resolved.
Show resolved Hide resolved
}
return this;
}

Expand Down
Expand Up @@ -18,6 +18,7 @@
import org.mockito.mock.MockCreationSettings;
import org.mockito.mock.MockName;
import org.mockito.mock.SerializableMode;
import org.mockito.quality.Strictness;
import org.mockito.stubbing.Answer;

public class CreationSettings<T> implements MockCreationSettings<T>, Serializable {
Expand All @@ -44,7 +45,7 @@ public class CreationSettings<T> implements MockCreationSettings<T>, Serializabl
private boolean useConstructor;
private Object outerClassInstance;
private Object[] constructorArgs;
protected boolean lenient;
protected Strictness strictness = Strictness.STRICT_STUBS;

public CreationSettings() {}

Expand All @@ -65,7 +66,7 @@ public CreationSettings(CreationSettings copy) {
this.useConstructor = copy.isUsingConstructor();
this.outerClassInstance = copy.getOuterClassInstance();
this.constructorArgs = copy.getConstructorArgs();
this.lenient = copy.lenient;
this.strictness = copy.strictness;
this.stripAnnotations = copy.stripAnnotations;
}

Expand Down Expand Up @@ -170,6 +171,11 @@ public boolean isStubOnly() {

@Override
public boolean isLenient() {
TimvdLippe marked this conversation as resolved.
Show resolved Hide resolved
return lenient;
return strictness == Strictness.LENIENT;
}

@Override
public Strictness getStrictness() {
return strictness;
}
}
4 changes: 4 additions & 0 deletions src/main/java/org/mockito/internal/exceptions/Reporter.java
Expand Up @@ -964,6 +964,10 @@ public static MockitoException defaultAnswerDoesNotAcceptNullParameter() {
return new MockitoException("defaultAnswer() does not accept null parameter");
}

public static MockitoException strictnessDoesNotAcceptNullParameter() {
return new MockitoException("strictness() does not accept null parameter");
}

public static MockitoException serializableWontWorkForObjectsThatDontImplementSerializable(
Class<?> classToMock) {
return new MockitoException(
Expand Down
Expand Up @@ -38,7 +38,7 @@ public class InvocationContainerImpl implements InvocationContainer, Serializabl

public InvocationContainerImpl(MockCreationSettings mockSettings) {
this.registeredInvocations = createRegisteredInvocations(mockSettings);
this.mockStrictness = mockSettings.isLenient() ? Strictness.LENIENT : null;
this.mockStrictness = mockSettings.getStrictness();
this.doAnswerStyleStubbing = new DoAnswerStyleStubbing();
}

Expand Down
Expand Up @@ -31,8 +31,8 @@ public static Strictness determineStrictness(
return stubbing.getStrictness();
}

if (mockSettings.isLenient()) {
return Strictness.LENIENT;
if (mockSettings.getStrictness() != null) {
return mockSettings.getStrictness();
}

return testLevelStrictness;
Expand Down
11 changes: 11 additions & 0 deletions src/main/java/org/mockito/mock/MockCreationSettings.java
Expand Up @@ -118,10 +118,21 @@ public interface MockCreationSettings<T> {
Object getOuterClassInstance();

/**
* @deprecated Use {@link MockCreationSettings#getStrictness()} instead.
*
* Informs if the mock was created with "lenient" strictness, e.g. having {@link Strictness#LENIENT} characteristic.
* For more information about using mocks with lenient strictness, see {@link MockSettings#lenient()}.
*
* @since 2.20.0
*/
@Deprecated
boolean isLenient();

/**
* Sets strictness level for the mock, e.g. having {@link Strictness#STRICT_STUBS} characteristic.
* For more information about using mocks with custom strictness, see {@link MockSettings#strictness(Strictness)}.
*
* @since 4.6.0
*/
Strictness getStrictness();
}
Expand Up @@ -260,4 +260,10 @@ public void addListeners_canAddDuplicateMockObjectListeners_ItsNotOurBusinessThe
assertThat(mockSettingsImpl.getStubbingLookupListeners())
.containsSequence(stubbingLookupListener, stubbingLookupListener);
}

@Test
public void validates_strictness() {
assertThatThrownBy(() -> mockSettingsImpl.strictness(null))
.hasMessageContaining("strictness() does not accept null parameter");
}
}
@@ -0,0 +1,43 @@
/*
* Copyright (c) 2022 Mockito contributors
* This program is made available under the terms of the MIT License.
*/
package org.mockitousage.strictness;

import org.assertj.core.api.Assertions;
import org.junit.Rule;
import org.junit.Test;
import org.mockito.Mock;
import org.mockito.exceptions.misusing.PotentialStubbingProblem;
import org.mockito.junit.MockitoJUnit;
import org.mockito.junit.MockitoRule;
import org.mockito.quality.Strictness;
import org.mockitousage.IMethods;

import static org.mockito.Mockito.when;

public class StrictnessMockAnnotationTest {

public @Rule MockitoRule rule = MockitoJUnit.rule().strictness(Strictness.STRICT_STUBS);

@Mock(strictness = Strictness.LENIENT)
IMethods lenientMock;

@Mock IMethods regularMock;

@Test
public void mock_is_lenient() {
when(lenientMock.simpleMethod("1")).thenReturn("1");

// then lenient mock does not throw:
TimvdLippe marked this conversation as resolved.
Show resolved Hide resolved
ProductionCode.simpleMethod(lenientMock, "3");
}

@Test
public void mock_is_strict() {
when(regularMock.simpleMethod("2")).thenReturn("2");

Assertions.assertThatThrownBy(() -> ProductionCode.simpleMethod(regularMock, "4"))
.isInstanceOf(PotentialStubbingProblem.class);
}
}
@@ -0,0 +1,57 @@
/*
* Copyright (c) 2022 Mockito contributors
* This program is made available under the terms of the MIT License.
*/
package org.mockitousage.strictness;

import org.assertj.core.api.Assertions;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.mockito.exceptions.misusing.PotentialStubbingProblem;
import org.mockito.junit.MockitoJUnit;
import org.mockito.junit.MockitoRule;
import org.mockito.quality.Strictness;
import org.mockitousage.IMethods;

import static org.mockito.Mockito.*;

public class StrictnessWithSettingsTest {

public @Rule MockitoRule rule = MockitoJUnit.rule().strictness(Strictness.STRICT_STUBS);

IMethods lenientMock;
IMethods regularMock;
IMethods strictMock;

@Before
public void before() {
lenientMock = mock(IMethods.class, withSettings().strictness(Strictness.LENIENT));
regularMock = mock(IMethods.class);
strictMock = mock(IMethods.class, withSettings().strictness(Strictness.STRICT_STUBS));
}

@Test
public void mock_is_lenient() {
when(lenientMock.simpleMethod("1")).thenReturn("1");

// lenient mock does not throw
ProductionCode.simpleMethod(lenientMock, "3");
}

@Test
public void mock_is_strict_with_default_settings() {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick Q - was it intentional for this to change mocks defined by mock(Blah.class) i.e with no rule/Junit MockitoSettings to strict by default?

I believe the previous behaviour was lenient by default?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have not changed the behavior here. This test is using a Strict rule explicitly (https://github.com/mockito/mockito/pull/2650/files/d0907808e9fce24f6dcddc94d1ea2c3ebfa15676#diff-3040268f6b18ef562dd495ac7fc13465723311889d5f326bfb615cc3aed37415R21) to verify that we handle that case correctly.

In case you are hitting any regressions, please file a new issue. You are most likely running into a different problem.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, thanks. We definitely have regressions (previously lenient mocks now behaving as strict) but I may have misidentified the possible cause. Will need to dig further if I get time.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another person has beat me to raising it at #2656 but this seems to be the same issue we are having.

when(regularMock.simpleMethod("3")).thenReturn("3");

Assertions.assertThatThrownBy(() -> ProductionCode.simpleMethod(regularMock, "4"))
.isInstanceOf(PotentialStubbingProblem.class);
}

@Test
public void mock_is_strict_with_explicit_settings() {
when(strictMock.simpleMethod("2")).thenReturn("2");

Assertions.assertThatThrownBy(() -> ProductionCode.simpleMethod(strictMock, "5"))
.isInstanceOf(PotentialStubbingProblem.class);
}
}