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

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
1 change: 1 addition & 0 deletions docs/release_notes.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ include::include.adoc[]
* Size of data providers is no longer calculated multiple times but only once
* Fix exception when using `@RepeatUntilFailure` with a data provider with unknown iteration amount. spockPull:2031[]
* Clarified documentation about data providers and `size()` calls spockIssue:2022[]
* Add best-effort error reporting for interactions on final methods when using the `byte-buddy` mock maker spockIssue:2039[]

== 2.4-M4 (2024-03-21)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,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 @@ -17,6 +17,7 @@
package org.spockframework.compiler;

import org.spockframework.compiler.model.*;
import org.spockframework.runtime.GroovyRuntimeUtil;
import org.spockframework.runtime.SpockException;
import org.spockframework.util.*;

Expand All @@ -26,7 +27,6 @@
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.*;
import org.objectweb.asm.Opcodes;

Expand Down Expand Up @@ -109,7 +109,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 @@ -135,7 +135,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 @@ -158,7 +158,7 @@ private void createFinalFieldGetter(Field field) {
}

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

import org.spockframework.mock.ISpockMockObject;

public class SpockNames {
public static final String VALUE_RECORDER = "$spock_valueRecorder";
public static final String ERROR_COLLECTOR = "$spock_errorCollector";
public static final String OLD_VALUE = "$spock_oldValue";
public static final String SPOCK_EX = "$spock_ex";
/**
* 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";
}
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;
}
}
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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import net.bytebuddy.TypeCache;
import net.bytebuddy.description.annotation.AnnotationDescription;
import net.bytebuddy.description.method.MethodDescription;
import net.bytebuddy.description.modifier.MethodManifestation;
import net.bytebuddy.description.modifier.SynchronizationState;
import net.bytebuddy.description.modifier.SyntheticState;
import net.bytebuddy.description.modifier.Visibility;
Expand All @@ -31,19 +32,23 @@
import net.bytebuddy.dynamic.loading.MultipleParentClassLoader;
import net.bytebuddy.dynamic.scaffold.TypeValidation;
import net.bytebuddy.implementation.FieldAccessor;
import net.bytebuddy.implementation.FixedValue;
import net.bytebuddy.implementation.MethodDelegation;
import net.bytebuddy.implementation.bind.annotation.Morph;
import org.codehaus.groovy.runtime.callsite.AbstractCallSite;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.VisibleForTesting;
import org.spockframework.compiler.SpockNames;
import org.spockframework.mock.ISpockMockObject;
import org.spockframework.mock.codegen.Target;
import org.spockframework.util.ReflectionUtil;

import java.lang.reflect.Method;
import java.util.Collection;
import java.util.concurrent.Callable;
import java.util.concurrent.ThreadLocalRandom;

import static java.util.Objects.requireNonNull;
import static net.bytebuddy.matcher.ElementMatchers.none;

class ByteBuddyMockFactory {
Expand All @@ -60,6 +65,7 @@ class ByteBuddyMockFactory {
private static final Class<?> CODEGEN_TARGET_CLASS = Target.class;
private static final String CODEGEN_PACKAGE = CODEGEN_TARGET_CLASS.getPackage().getName();
private static final AnnotationDescription INTERNAL_ANNOTATION = AnnotationDescription.Builder.ofType(Internal.class).build();
private static final Method MOCK_INTERACTION_VALIDATOR_METHOD = requireNonNull(ReflectionUtil.getMethodByName(ISpockMockObject.class, SpockNames.SPOCK_MOCK_INTERATION_VALIDATOR));

/**
* This array contains {@link TypeCachingLock} instances, which are used as java monitor locks for
Expand Down Expand Up @@ -136,6 +142,10 @@ Object createMock(IMockMaker.IMockCreationSettings settings) {
.method(m -> !isGroovyMOPMethod(type, m))
.intercept(mockInterceptor())
.transform(mockTransformer())
.method(m -> m.represents(MOCK_INTERACTION_VALIDATOR_METHOD))
// Implement the $spock_mockInteractionValidation() method which returns the static field below, so we have a validation instance for every mock class
.intercept(FixedValue.reference(new ByteBuddyMockInteractionValidator(), SpockNames.SPOCK_MOCK_INTERATION_VALIDATOR))
.transform(validateMockInteractionTransformer())
.implement(ByteBuddyInterceptorAdapter.InterceptorAccess.class)
.intercept(FieldAccessor.ofField("$spock_interceptor"))
.defineField("$spock_interceptor", IProxyBasedMockInterceptor.class, Visibility.PRIVATE, SyntheticState.SYNTHETIC)
Expand All @@ -149,6 +159,10 @@ Object createMock(IMockMaker.IMockCreationSettings settings) {
return proxy;
}

private static Transformer<MethodDescription> validateMockInteractionTransformer() {
return Transformer.ForMethod.withModifiers(SynchronizationState.PLAIN, Visibility.PUBLIC, MethodManifestation.FINAL);
}

private static Transformer<MethodDescription> mockTransformer() {
return Transformer.ForMethod.withModifiers(SynchronizationState.PLAIN, Visibility.PUBLIC); //Overridden methods should be public and non-synchronized.
}
Expand Down
Loading
Loading