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

Best-effort error reporting for interactions on final methods #2040

Merged
merged 15 commits into from
Dec 28, 2024
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/release_notes.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ include::include.adoc[]
* Add `IStatelessAnnotationDrivenExtension` to allow a single extension instance to be reused across all specifications spockPull:2055[]
* Add new well-known versions to `Jvm` helper to support versions up to 29 spockPull:2057[]
** Built-in extensions have been updated to use this new interface where applicable.
* Add best-effort error reporting for interactions on final methods when using the `byte-buddy` mock maker spockIssue:2039[]
* Add support for `@FailsWith` to assert exception message spockIssue:2039[]
* Improve `@Timeout` extension will now use virtual threads if available spockPull:1986[]
* Improve mock argument matching, types constraints or arguments in interactions can now handle primitive types like `_ as int` spockIssue:1974[]
* Improve `verifyEach` to accept an optional second index parameter for the assertion block closure spockPull:2043[]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
*/
public abstract class AstUtil {
private static final Pattern DATA_TABLE_SEPARATOR = Pattern.compile("_{2,}+");
private static final String GET_METHOD_NAME = "get";
private static final String GET_AT_METHOD_NAME = new IntegerArrayGetAtMetaMethod().getName();

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@
import org.codehaus.groovy.ast.*;
import org.codehaus.groovy.ast.expr.*;
import org.codehaus.groovy.ast.stmt.*;
import org.codehaus.groovy.runtime.MetaClassHelper;
import org.codehaus.groovy.syntax.Token;
import org.codehaus.groovy.syntax.Types;
import org.jetbrains.annotations.NotNull;
import org.objectweb.asm.Opcodes;
import org.spockframework.compiler.model.*;
import org.spockframework.runtime.GroovyRuntimeUtil;
import org.spockframework.runtime.SpockException;
import org.spockframework.util.InternalIdentifiers;
import org.spockframework.util.ObjectUtil;
Expand Down Expand Up @@ -112,7 +112,7 @@ private void changeFinalFieldInternalName(Field field) {
}

private void createSharedFieldGetter(Field field) {
String getterName = "get" + MetaClassHelper.capitalize(field.getName());
String getterName = GroovyRuntimeUtil.propertyToGetterMethodName(field.getName());
MethodNode getter = spec.getAst().getMethod(getterName, Parameter.EMPTY_ARRAY);
if (getter != null) {
errorReporter.error(field.getAst(),
Expand All @@ -138,7 +138,7 @@ private void createSharedFieldGetter(Field field) {
}

private void createFinalFieldGetter(Field field) {
String getterName = "get" + MetaClassHelper.capitalize(field.getName());
String getterName = GroovyRuntimeUtil.propertyToGetterMethodName(field.getName());
MethodNode getter = spec.getAst().getMethod(getterName, Parameter.EMPTY_ARRAY);
if (getter != null) {
errorReporter.error(field.getAst(),
Expand All @@ -161,8 +161,8 @@ private void createFinalFieldGetter(Field field) {
}

private void createSharedFieldSetter(Field field) {
String setterName = "set" + MetaClassHelper.capitalize(field.getName());
Parameter[] params = new Parameter[] { new Parameter(field.getAst().getType(), SpockNames.SPOCK_VALUE) };
String setterName = GroovyRuntimeUtil.propertyToSetterMethodName(field.getName());
Parameter[] params = new Parameter[]{new Parameter(field.getAst().getType(), SpockNames.SPOCK_VALUE)};
MethodNode setter = spec.getAst().getMethod(setterName, params);
if (setter != null) {
errorReporter.error(field.getAst(),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package org.spockframework.compiler;

import org.spockframework.mock.ISpockMockObject;

public class SpockNames {
public static final String ERROR_COLLECTOR = "$spock_errorCollector";
public static final String FAILED_BLOCK = "$spock_failedBlock";
Expand All @@ -9,4 +11,12 @@ public class SpockNames {
public static final String SPOCK_TMP_THROWABLE = "$spock_tmp_throwable";
public static final String SPOCK_VALUE = "$spock_value";
public static final String VALUE_RECORDER = "$spock_valueRecorder";
/**
* Name of the method {@link ISpockMockObject#$spock_get()}.
*/
public static final String SPOCK_GET = "$spock_get";
/**
* Name of the method {@link ISpockMockObject#$spock_mockInteractionValidator()}.
*/
public static final String SPOCK_MOCK_INTERATION_VALIDATOR = "$spock_mockInteractionValidator";
}
Original file line number Diff line number Diff line change
Expand Up @@ -324,8 +324,8 @@ private <T> T createMockImpl(String inferredName, Class<?> inferredType, T insta
MockImplementation implementation, Map<String, Object> options, Class<?> specifiedType, Closure<?> closure) {
Type effectiveType = specifiedType != null ? specifiedType : options.containsKey("type") ? (Type) options.get("type") : inferredType;
if (effectiveType == null) {
throw new InvalidSpecException("Mock object type cannot be inferred automatically. " +
"Please specify a type explicitly (e.g. 'Mock(Person)').");
throw new InvalidSpecException(nature + " object type cannot be inferred automatically. " +
"Please specify a type explicitly (e.g. '" + nature + "(Person)').");
}
return createMock(inferredName, instance, effectiveType, nature, implementation, options, closure);
}
Expand Down
16 changes: 16 additions & 0 deletions spock-core/src/main/java/org/spockframework/mock/IMockObject.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package org.spockframework.mock;

import org.spockframework.mock.runtime.SpecificationAttachable;
import org.spockframework.util.Beta;
import org.spockframework.util.Nullable;
import spock.lang.Specification;

Expand All @@ -29,6 +30,13 @@ public interface IMockObject extends SpecificationAttachable {
@Nullable
String getName();

/**
* Returns the {@link #getName()} of this mock object, or {@code "unnamed"} if it has no name.
*
* @return the name of this mock object, or {@code "unnamed"} if it has no name
*/
String getMockName();

/**
* Returns the declared type of this mock object.
*
Expand Down Expand Up @@ -81,4 +89,12 @@ public interface IMockObject extends SpecificationAttachable {
* @return whether this mock object matches the target of the specified interaction
*/
boolean matches(Object target, IMockInteraction interaction);

/**
* Returns the used mock configuration which created this mock.
*
* @return the mock configuration
*/
@Beta
IMockConfiguration getConfiguration();
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,28 @@

package org.spockframework.mock;

import org.spockframework.mock.runtime.IMockInteractionValidator;
import org.spockframework.util.Beta;
import org.spockframework.util.Nullable;

/**
* Marker-like interface implemented by all mock objects that allows
* {@link MockUtil} to detect mock objects. Not intended for direct use.
* MockObject interface implemented by some {@link spock.mock.MockMakers} that allows the {@link org.spockframework.mock.runtime.MockMakerRegistry}
* to detect mock objects.
*
* <p>Not intended for direct use.
*/
public interface ISpockMockObject {

IMockObject $spock_get();

/**
* @return the {@link IMockInteractionValidator} used to verify {@link IMockInteraction}
* @see IMockInteractionValidator
* @since 2.4
*/
@Nullable
@Beta
default IMockInteractionValidator $spock_mockInteractionValidator() {
return null;
}
}
15 changes: 11 additions & 4 deletions spock-core/src/main/java/org/spockframework/mock/MockNature.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,28 +27,30 @@ public enum MockNature {
* A mock object whose method calls are verified, which instantiates class-based mock objects with Objenesis,
* and whose strategy for responding to unexpected method calls is {@link ZeroOrNullResponse}.
*/
MOCK(true, true, ZeroOrNullResponse.INSTANCE),
MOCK(true, true, ZeroOrNullResponse.INSTANCE, "Mock"),

/**
* A mock object whose method calls are not verified, which instantiates class-based mock objects with Objenesis,
* and whose strategy for responding to unexpected method calls is {@link EmptyOrDummyResponse}.
*/
STUB(false, true, EmptyOrDummyResponse.INSTANCE),
STUB(false, true, EmptyOrDummyResponse.INSTANCE, "Stub"),

/**
* A mock object whose method calls are verified, which instantiates class-based mock objects by calling a
* real constructor, and whose strategy for responding to unexpected method calls is {@link CallRealMethodResponse}.
*/
SPY(true, false, CallRealMethodResponse.INSTANCE);
SPY(true, false, CallRealMethodResponse.INSTANCE, "Spy");

private final boolean verified;
private final boolean useObjenesis;
private final IDefaultResponse defaultResponse;
private final String methodName;

MockNature(boolean verified, boolean useObjenesis, IDefaultResponse defaultResponse) {
MockNature(boolean verified, boolean useObjenesis, IDefaultResponse defaultResponse, String methodName) {
this.verified = verified;
this.useObjenesis = useObjenesis;
this.defaultResponse = defaultResponse;
this.methodName = methodName;
}

/**
Expand Down Expand Up @@ -79,4 +81,9 @@ public boolean isUseObjenesis() {
public IDefaultResponse getDefaultResponse() {
return defaultResponse;
}

@Override
public String toString() {
return methodName;
}
}
13 changes: 12 additions & 1 deletion spock-core/src/main/java/org/spockframework/mock/MockUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public class MockUtil {
* @return whether the given object is a (Spock) mock object
*/
public boolean isMock(Object object) {
return getMockMakerRegistry().asMockOrNull(object) != null;
return asMockOrNull(object) != null;
}

/**
Expand Down Expand Up @@ -69,6 +69,17 @@ public IMockObject asMock(Object object) {
return mockOrNull;
}

/**
* Returns information about a mock object or {@code null}, if the object is not a mock.
*
* @param object a mock object
* @return information about the mock object or {@code null}, if the object is not a mock.
*/
@Nullable
public IMockObject asMockOrNull(Object object) {
return getMockMakerRegistry().asMockOrNull(object);
}

/**
* Attaches mock to a Specification.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ public EqualMethodNameConstraint(String methodName) {
this.methodName = methodName;
}

public String getMethodName() {
return methodName;
}

@Override
public boolean isSatisfiedBy(IMockInvocation invocation) {
return invocation.getMethod().getName().equals(methodName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ public EqualPropertyNameConstraint(String propertyName) {
this.propertyName = propertyName;
}

public String getPropertyName() {
return propertyName;
}

@Override
public boolean isSatisfiedBy(IMockInvocation invocation) {
return propertyName.equals(getPropertyName(invocation));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package org.spockframework.mock.constraint;

import org.spockframework.mock.*;
import org.spockframework.mock.runtime.IMockInteractionValidator;
import org.spockframework.runtime.Condition;
import org.spockframework.runtime.InvalidSpecException;
import org.spockframework.util.CollectionUtil;
Expand Down Expand Up @@ -50,12 +51,27 @@ public String describeMismatch(IMockInvocation invocation) {
@Override
public void setInteraction(IMockInteraction interaction) {
this.interaction = interaction;
if (interaction.isRequired() && MOCK_UTIL.isMock(target)) {
IMockObject mockObject = MOCK_UTIL.asMock(target);
IMockObject mockObject = MOCK_UTIL.asMockOrNull(target);
if (mockObject != null) {
checkRequiredInteraction(mockObject, interaction);
validateMockInteraction(mockObject, interaction);
}
}

private void checkRequiredInteraction(IMockObject mockObject, IMockInteraction interaction) {
if (interaction.isRequired()) {
if (!mockObject.isVerified()) {
String mockName = mockObject.getName() != null ? mockObject.getName() : "unnamed";
throw new InvalidSpecException("Stub '%s' matches the following required interaction:" +
"\n\n%s\n\nRemove the cardinality (e.g. '1 *'), or turn the stub into a mock.\n").withArgs(mockName, interaction);
"\n\n%s\n\nRemove the cardinality (e.g. '1 *'), or turn the stub into a mock.\n").withArgs(mockObject.getMockName(), interaction);
}
}
}

private void validateMockInteraction(IMockObject mockObject, IMockInteraction interaction) {
if (target instanceof ISpockMockObject) {
IMockInteractionValidator validation = ((ISpockMockObject) target).$spock_mockInteractionValidator();
if (validation != null) {
validation.validateMockInteraction(mockObject, interaction);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
package org.spockframework.mock.runtime;

import org.spockframework.compiler.SpockNames;
import org.spockframework.mock.IMockObject;
import org.spockframework.mock.ISpockMockObject;
import org.spockframework.runtime.GroovyRuntimeUtil;

import java.lang.reflect.Method;
Expand All @@ -8,8 +11,13 @@

import groovy.lang.*;
import org.jetbrains.annotations.Nullable;
import org.spockframework.util.ReflectionUtil;

import static java.util.Objects.requireNonNull;

public abstract class BaseMockInterceptor implements IProxyBasedMockInterceptor {


private MetaClass mockMetaClass;

BaseMockInterceptor(MetaClass mockMetaClass) {
Expand Down Expand Up @@ -39,11 +47,11 @@ protected String handleGetProperty(GroovyObject target, Object[] args) {
MetaClass metaClass = target.getMetaClass();
//First try the isXXX before getXXX, because this is the expected behavior, if it is boolean property.
MetaMethod booleanVariant = metaClass
.getMetaMethod(GroovyRuntimeUtil.propertyToMethodName("is", propertyName), GroovyRuntimeUtil.EMPTY_ARGUMENTS);
.getMetaMethod(GroovyRuntimeUtil.propertyToBooleanGetterMethodName(propertyName), GroovyRuntimeUtil.EMPTY_ARGUMENTS);
if (booleanVariant != null && booleanVariant.getReturnType() == boolean.class) {
methodName = booleanVariant.getName();
} else {
methodName = GroovyRuntimeUtil.propertyToMethodName("get", propertyName);
methodName = GroovyRuntimeUtil.propertyToGetterMethodName(propertyName);
}
}
return methodName;
Expand All @@ -52,4 +60,11 @@ protected String handleGetProperty(GroovyObject target, Object[] args) {
protected boolean isMethod(Method method, String name, Class<?>... parameterTypes) {
return method.getName().equals(name) && Arrays.equals(method.getParameterTypes(), parameterTypes);
}

public static Object handleSpockMockInterface(Method method, IMockObject mockObject) {
if (SpockNames.SPOCK_MOCK_INTERATION_VALIDATOR.equals(method.getName())) {
return null; //This should be handled in the corresponding MockMakers.
}
return mockObject;
}
}
Loading
Loading