From 5d5a3e2a11185839e83cc16cd8e8ab4b953f8ba7 Mon Sep 17 00:00:00 2001 From: AndreasTu Date: Thu, 7 Nov 2024 13:01:29 +0100 Subject: [PATCH 01/11] Best-effort error reporting for interactions on final methods Add error reporting code to the byte-buddy` mock maker to report interaction on final methods, which can't be handled. The IMockInteractionValidator interface allows IMockMakers to implement different kinds of validations on mock interactions. Fixes #2039 --- docs/release_notes.adoc | 1 + .../org/spockframework/compiler/AstUtil.java | 1 - .../spockframework/compiler/SpecRewriter.java | 8 +- .../spockframework/compiler/SpockNames.java | 10 + .../org/spockframework/mock/IMockObject.java | 16 ++ .../spockframework/mock/ISpockMockObject.java | 22 +- .../org/spockframework/mock/MockUtil.java | 13 +- .../constraint/EqualMethodNameConstraint.java | 4 + .../EqualPropertyNameConstraint.java | 4 + .../mock/constraint/TargetConstraint.java | 24 ++- .../mock/runtime/BaseMockInterceptor.java | 19 +- .../mock/runtime/ByteBuddyMockFactory.java | 14 ++ .../ByteBuddyMockInteractionValidator.java | 134 +++++++++++++ .../mock/runtime/GroovyMockInterceptor.java | 7 +- .../mock/runtime/GroovyMockMetaClass.java | 9 +- .../runtime/IMockInteractionValidator.java | 47 +++++ .../mock/runtime/JavaMockInterceptor.java | 7 +- .../mock/runtime/MockInteraction.java | 4 + .../mock/runtime/MockObject.java | 52 ++--- .../runtime/mockito/MockitoMockMakerImpl.java | 3 +- .../runtime/GroovyRuntimeUtil.java | 20 +- .../runtime/GroovyRuntimeUtilSpec.groovy | 22 +- .../smoke/mock/JavaMocks.groovy | 189 ++++++++++++++++++ .../smoke/mock/JavaSpies.groovy | 39 +++- .../smoke/mock/JavaStubs.groovy | 94 ++++++++- .../smoke/mock/MockDefaultResponses.groovy | 12 ++ .../spring/mock/DelegatingInterceptor.java | 13 +- 27 files changed, 725 insertions(+), 63 deletions(-) create mode 100644 spock-core/src/main/java/org/spockframework/mock/runtime/ByteBuddyMockInteractionValidator.java create mode 100644 spock-core/src/main/java/org/spockframework/mock/runtime/IMockInteractionValidator.java create mode 100644 spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaMocks.groovy diff --git a/docs/release_notes.adoc b/docs/release_notes.adoc index d52a9c7324..5722942ff4 100644 --- a/docs/release_notes.adoc +++ b/docs/release_notes.adoc @@ -26,6 +26,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) diff --git a/spock-core/src/main/java/org/spockframework/compiler/AstUtil.java b/spock-core/src/main/java/org/spockframework/compiler/AstUtil.java index 61c1287c40..9ada98adba 100644 --- a/spock-core/src/main/java/org/spockframework/compiler/AstUtil.java +++ b/spock-core/src/main/java/org/spockframework/compiler/AstUtil.java @@ -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(); /** diff --git a/spock-core/src/main/java/org/spockframework/compiler/SpecRewriter.java b/spock-core/src/main/java/org/spockframework/compiler/SpecRewriter.java index 3b02908707..9fb14f8b81 100644 --- a/spock-core/src/main/java/org/spockframework/compiler/SpecRewriter.java +++ b/spock-core/src/main/java/org/spockframework/compiler/SpecRewriter.java @@ -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.*; @@ -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; @@ -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(), @@ -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(), @@ -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) { diff --git a/spock-core/src/main/java/org/spockframework/compiler/SpockNames.java b/spock-core/src/main/java/org/spockframework/compiler/SpockNames.java index aa4edf9024..181aeb4b8b 100644 --- a/spock-core/src/main/java/org/spockframework/compiler/SpockNames.java +++ b/spock-core/src/main/java/org/spockframework/compiler/SpockNames.java @@ -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"; } diff --git a/spock-core/src/main/java/org/spockframework/mock/IMockObject.java b/spock-core/src/main/java/org/spockframework/mock/IMockObject.java index 44407da2e8..4157ec5c29 100644 --- a/spock-core/src/main/java/org/spockframework/mock/IMockObject.java +++ b/spock-core/src/main/java/org/spockframework/mock/IMockObject.java @@ -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; @@ -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. * @@ -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(); } diff --git a/spock-core/src/main/java/org/spockframework/mock/ISpockMockObject.java b/spock-core/src/main/java/org/spockframework/mock/ISpockMockObject.java index c9e1595960..4ddd72002a 100644 --- a/spock-core/src/main/java/org/spockframework/mock/ISpockMockObject.java +++ b/spock-core/src/main/java/org/spockframework/mock/ISpockMockObject.java @@ -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. + * + *

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; + } } diff --git a/spock-core/src/main/java/org/spockframework/mock/MockUtil.java b/spock-core/src/main/java/org/spockframework/mock/MockUtil.java index 745525cd80..aa2e943fd1 100644 --- a/spock-core/src/main/java/org/spockframework/mock/MockUtil.java +++ b/spock-core/src/main/java/org/spockframework/mock/MockUtil.java @@ -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; } /** @@ -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. * diff --git a/spock-core/src/main/java/org/spockframework/mock/constraint/EqualMethodNameConstraint.java b/spock-core/src/main/java/org/spockframework/mock/constraint/EqualMethodNameConstraint.java index 020120e600..82196a538f 100644 --- a/spock-core/src/main/java/org/spockframework/mock/constraint/EqualMethodNameConstraint.java +++ b/spock-core/src/main/java/org/spockframework/mock/constraint/EqualMethodNameConstraint.java @@ -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); diff --git a/spock-core/src/main/java/org/spockframework/mock/constraint/EqualPropertyNameConstraint.java b/spock-core/src/main/java/org/spockframework/mock/constraint/EqualPropertyNameConstraint.java index 737916baef..eb21660dff 100644 --- a/spock-core/src/main/java/org/spockframework/mock/constraint/EqualPropertyNameConstraint.java +++ b/spock-core/src/main/java/org/spockframework/mock/constraint/EqualPropertyNameConstraint.java @@ -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)); diff --git a/spock-core/src/main/java/org/spockframework/mock/constraint/TargetConstraint.java b/spock-core/src/main/java/org/spockframework/mock/constraint/TargetConstraint.java index 5df131f027..b6e3753ac4 100644 --- a/spock-core/src/main/java/org/spockframework/mock/constraint/TargetConstraint.java +++ b/spock-core/src/main/java/org/spockframework/mock/constraint/TargetConstraint.java @@ -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; @@ -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); } } } diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/BaseMockInterceptor.java b/spock-core/src/main/java/org/spockframework/mock/runtime/BaseMockInterceptor.java index 3af8948528..a4e0bf5510 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/BaseMockInterceptor.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/BaseMockInterceptor.java @@ -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; @@ -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) { @@ -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; @@ -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 (method.getName().equals(SpockNames.SPOCK_MOCK_INTERATION_VALIDATOR)) { + return null; //This should be handled in the corresponding MockMakers. + } + return mockObject; + } } diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/ByteBuddyMockFactory.java b/spock-core/src/main/java/org/spockframework/mock/runtime/ByteBuddyMockFactory.java index 0e4086aa5f..1d174489ad 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/ByteBuddyMockFactory.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/ByteBuddyMockFactory.java @@ -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; @@ -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 { @@ -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 @@ -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) @@ -149,6 +159,10 @@ Object createMock(IMockMaker.IMockCreationSettings settings) { return proxy; } + private static Transformer validateMockInteractionTransformer() { + return Transformer.ForMethod.withModifiers(SynchronizationState.PLAIN, Visibility.PUBLIC, MethodManifestation.FINAL); + } + private static Transformer mockTransformer() { return Transformer.ForMethod.withModifiers(SynchronizationState.PLAIN, Visibility.PUBLIC); //Overridden methods should be public and non-synchronized. } diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/ByteBuddyMockInteractionValidator.java b/spock-core/src/main/java/org/spockframework/mock/runtime/ByteBuddyMockInteractionValidator.java new file mode 100644 index 0000000000..8af3a96868 --- /dev/null +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/ByteBuddyMockInteractionValidator.java @@ -0,0 +1,134 @@ +/* + * Copyright 2024 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.spockframework.mock.runtime; + +import org.junit.platform.commons.support.ModifierSupport; +import org.spockframework.mock.IInvocationConstraint; +import org.spockframework.mock.IMockInteraction; +import org.spockframework.mock.IMockObject; +import org.spockframework.mock.MockImplementation; +import org.spockframework.mock.constraint.EqualMethodNameConstraint; +import org.spockframework.mock.constraint.EqualPropertyNameConstraint; +import org.spockframework.runtime.InvalidSpecException; +import org.spockframework.util.ThreadSafe; + +import java.lang.reflect.Method; +import java.util.Arrays; +import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; + +import static java.util.Objects.requireNonNull; +import static org.spockframework.runtime.GroovyRuntimeUtil.propertyToBooleanGetterMethodName; +import static org.spockframework.runtime.GroovyRuntimeUtil.propertyToGetterMethodName; + +/** + * {@link ByteBuddyMockInteractionValidator} validates the {@link IMockInteraction} for {@link ByteBuddyMockMaker} mocks. + * + *

+ * + *

Implementation note: The {@link ByteBuddyMockFactory} create a single instance for each mocked class + * and the same validation is used for multiple mocks of the same class. + * + * @author Andreas Turban + */ +@ThreadSafe +final class ByteBuddyMockInteractionValidator implements IMockInteractionValidator { + + private volatile Class mockClass; + private volatile Set finalMethods; + + ByteBuddyMockInteractionValidator() { + } + + @Override + public void validateMockInteraction(IMockObject mockObject, IMockInteraction mockInteractionParam) { + requireNonNull(mockObject); + requireNonNull(mockInteractionParam); + Object instance = requireNonNull(mockObject.getInstance()); + + if (mockObject.getConfiguration().getImplementation() == MockImplementation.GROOVY) { + //We do not validate final methods for Groovy mocks, because final mocking can be done with the Groovy MOP. + return; + } + + initializeClassData(instance); + + validate(mockObject, (MockInteraction) mockInteractionParam); + } + + private void validate(IMockObject mockObject, MockInteraction mockInteraction) { + for (IInvocationConstraint constraint : mockInteraction.getConstraints()) { + if (constraint instanceof EqualMethodNameConstraint) { + EqualMethodNameConstraint methodConstraint = (EqualMethodNameConstraint) constraint; + String methodName = methodConstraint.getMethodName(); + validateNonFinalMethod(mockObject, methodName); + } + if (constraint instanceof EqualPropertyNameConstraint) { + EqualPropertyNameConstraint propNameConstraint = (EqualPropertyNameConstraint) constraint; + validateProperty(mockObject, propNameConstraint); + } + } + } + + private void validateProperty(IMockObject mockObject, EqualPropertyNameConstraint propNameConstraint) { + String propName = propNameConstraint.getPropertyName(); + //We do not need to check for setters, because a property access like x.prop = value has not mock interaction syntax + validateNonFinalMethod(mockObject, propertyToGetterMethodName(propName)); + validateNonFinalMethod(mockObject, propertyToBooleanGetterMethodName(propName)); + } + + private void validateNonFinalMethod(IMockObject mockObject, String methodName) { + if (finalMethods.contains(methodName)) { + throw new InvalidSpecException("The final method '" + + methodName + "' of '" + + mockObject.getMockName() + + "' can't be mocked by the '" + + ByteBuddyMockMaker.ID + + "' mock maker. Please use another mock maker supporting final methods."); + } + } + + private void initializeClassData(Object mock) { + Class mockClassOfMockObj = mock.getClass(); + if (mockClass == null) { + synchronized (this) { + if (mockClass == null) { + finalMethods = Arrays.stream(mockClassOfMockObj.getMethods()) + .filter(m -> !ModifierSupport.isStatic(m)) + .collect(Collectors.groupingBy(Method::getName)) + .entrySet() + .stream() + .filter(e -> e.getValue() + .stream() + .allMatch(ModifierSupport::isFinal)) + .map(Map.Entry::getKey) + .collect(Collectors.toSet()); + + mockClass = mockClassOfMockObj; + } + } + } + + if (mockClass != mockClassOfMockObj) { + throw new IllegalStateException(); + } + } +} diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/GroovyMockInterceptor.java b/spock-core/src/main/java/org/spockframework/mock/runtime/GroovyMockInterceptor.java index 48d46fd64b..9e1fcfdd45 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/GroovyMockInterceptor.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/GroovyMockInterceptor.java @@ -36,11 +36,10 @@ public GroovyMockInterceptor(IMockConfiguration mockConfiguration, Specification @Override public Object intercept(Object target, Method method, Object[] arguments, IResponseGenerator realMethodInvoker) { - IMockObject mockObject = new MockObject(mockConfiguration.getName(), mockConfiguration.getExactType(), target, - mockConfiguration.isVerified(), mockConfiguration.isGlobal(), mockConfiguration.getDefaultResponse(), specification, this); + IMockObject mockObject = new MockObject(mockConfiguration, target, specification, this); if (method.getDeclaringClass() == ISpockMockObject.class) { - return mockObject; + return handleSpockMockInterface(method, mockObject); } // we do not need the cast information from the wrappers here, the method selection @@ -61,7 +60,7 @@ public Object intercept(Object target, Method method, Object[] arguments, IRespo } } if (isMethod(method, "setProperty", String.class, Object.class)) { - String methodName = GroovyRuntimeUtil.propertyToMethodName("set", (String) args[0]); + String methodName = GroovyRuntimeUtil.propertyToSetterMethodName((String) args[0]); return GroovyRuntimeUtil.invokeMethod(target, methodName, args[1]); } if (isMethod(method, "methodMissing", String.class, Object.class)) { diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/GroovyMockMetaClass.java b/spock-core/src/main/java/org/spockframework/mock/runtime/GroovyMockMetaClass.java index b8ef479d79..1af4f4e1a7 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/GroovyMockMetaClass.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/GroovyMockMetaClass.java @@ -53,17 +53,17 @@ public Object invokeConstructor(Object[] arguments) { @Override public Object getProperty(Object target, String property) { - String methodName = GroovyRuntimeUtil.propertyToMethodName("is", property); + String methodName = GroovyRuntimeUtil.propertyToBooleanGetterMethodName(property); MetaMethod metaMethod = delegate.getMetaMethod(methodName, GroovyRuntimeUtil.EMPTY_ARGUMENTS); if (metaMethod == null || metaMethod.getReturnType() != boolean.class) { - methodName = GroovyRuntimeUtil.propertyToMethodName("get", property); + methodName = GroovyRuntimeUtil.propertyToGetterMethodName(property); } return invokeMethod(target, methodName, GroovyRuntimeUtil.EMPTY_ARGUMENTS); } @Override public void setProperty(Object target, String property, Object newValue) { - String methodName = GroovyRuntimeUtil.propertyToMethodName("set", property); + String methodName = GroovyRuntimeUtil.propertyToSetterMethodName(property); invokeMethod(target, methodName, new Object[] {newValue}); } @@ -121,8 +121,7 @@ private boolean isGetMetaClassCallOnGroovyObject(Object target, String method, O private IMockInvocation createMockInvocation(MetaMethod metaMethod, Object target, String methodName, Object[] arguments, boolean isStatic) { - IMockObject mockObject = new MockObject(configuration.getName(), configuration.getExactType(), target, - configuration.isVerified(), configuration.isGlobal(), configuration.getDefaultResponse(), specification, this); + IMockObject mockObject = new MockObject(configuration, target, specification, this); IMockMethod mockMethod; if (metaMethod != null) { List parameterTypes = asList(metaMethod.getNativeParameterTypes()); diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/IMockInteractionValidator.java b/spock-core/src/main/java/org/spockframework/mock/runtime/IMockInteractionValidator.java new file mode 100644 index 0000000000..474d08a907 --- /dev/null +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/IMockInteractionValidator.java @@ -0,0 +1,47 @@ +/* + * Copyright 2024 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.spockframework.mock.runtime; + +import org.spockframework.mock.IMockInteraction; +import org.spockframework.mock.IMockObject; +import org.spockframework.mock.ISpockMockObject; +import org.spockframework.util.Beta; +import org.spockframework.util.ThreadSafe; + +/** + * The {@link IMockInteractionValidator} interface allows {@link IMockMaker} implementations + * to implement different kinds of validations on mock interactions. + * + *

The instance is registered and the {@link ISpockMockObject#$spock_mockInteractionValidator()} method. The {@code IMockMaker} shall return the validation instance to use.

+ * + * @author Andreas Turban + * @since 2.4 + */ +@ThreadSafe +@Beta +public interface IMockInteractionValidator { + /** + * Is called by the {@link IMockInteraction} on creation to allow the underlying {@link IMockMaker} to validate the interaction. + * + * @param mockObject the spock mock object + * @param interaction the interaction to validate + * @throws org.spockframework.runtime.InvalidSpecException if the interface is invalid + * @since 2.4 + */ + @Beta + void validateMockInteraction(IMockObject mockObject, IMockInteraction interaction); +} diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/JavaMockInterceptor.java b/spock-core/src/main/java/org/spockframework/mock/runtime/JavaMockInterceptor.java index a90eacb7dc..125b66e5e9 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/JavaMockInterceptor.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/JavaMockInterceptor.java @@ -37,11 +37,10 @@ public JavaMockInterceptor(IMockConfiguration mockConfiguration, Specification s @Override public Object intercept(Object target, Method method, Object[] arguments, IResponseGenerator realMethodInvoker) { - IMockObject mockObject = new MockObject(mockConfiguration.getName(), mockConfiguration.getExactType(), - target, mockConfiguration.isVerified(), false, mockConfiguration.getDefaultResponse(), specification, this); + IMockObject mockObject = new MockObject(mockConfiguration, target, specification, this); if (method.getDeclaringClass() == ISpockMockObject.class) { - return mockObject; + return handleSpockMockInterface(method, mockObject); } // here no instances of org.codehaus.groovy.runtime.wrappers.Wrapper subclasses @@ -64,7 +63,7 @@ public Object intercept(Object target, Method method, Object[] arguments, IRespo // HACK: for some reason, runtime dispatches direct property access on mock classes via ScriptBytecodeAdapter // delegate to the corresponding setter method // for abstract groovy classes and interfaces it uses InvokerHelper - String methodName = GroovyRuntimeUtil.propertyToMethodName("set", (String)args[0]); + String methodName = GroovyRuntimeUtil.propertyToSetterMethodName((String) args[0]); return GroovyRuntimeUtil.invokeMethod(target, methodName, GroovyRuntimeUtil.asArgumentArray(args[1])); } } diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/MockInteraction.java b/spock-core/src/main/java/org/spockframework/mock/runtime/MockInteraction.java index 4971e59033..c197e86b43 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/MockInteraction.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/MockInteraction.java @@ -71,6 +71,10 @@ public boolean matches(IMockInvocation invocation) { return true; } + List getConstraints() { + return constraints; + } + @Override public Supplier accept(IMockInvocation invocation) { acceptedInvocations.add(invocation); diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/MockObject.java b/spock-core/src/main/java/org/spockframework/mock/runtime/MockObject.java index ab98ebc0a8..adbb621d51 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/MockObject.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/MockObject.java @@ -23,25 +23,18 @@ import java.lang.reflect.Type; +import static java.util.Objects.*; + public class MockObject implements IMockObject { - private final String name; - private final Type type; + private final IMockConfiguration configuration; private final Object instance; - private final boolean verified; - private final boolean global; - private final IDefaultResponse defaultResponse; private final SpecificationAttachable mockInterceptor; - + @Nullable private Specification specification; - public MockObject(@Nullable String name, Type type, Object instance, boolean verified, boolean global, - IDefaultResponse defaultResponse, Specification specification, SpecificationAttachable mockInterceptor) { - this.name = name; - this.type = type; + public MockObject(IMockConfiguration configuration, Object instance, Specification specification, SpecificationAttachable mockInterceptor) { + this.configuration = requireNonNull(configuration); this.instance = instance; - this.verified = verified; - this.global = global; - this.defaultResponse = defaultResponse; this.specification = specification; this.mockInterceptor = mockInterceptor; } @@ -49,17 +42,22 @@ public MockObject(@Nullable String name, Type type, Object instance, boolean ver @Override @Nullable public String getName() { - return name; + return configuration.getName(); + } + + @Override + public String getMockName() { + return getName() != null ? getName() : "unnamed"; } @Override public Class getType() { - return GenericTypeReflectorUtil.erase(type); + return GenericTypeReflectorUtil.erase(getExactType()); } @Override public Type getExactType() { - return type; + return configuration.getExactType(); } @Override @@ -69,12 +67,16 @@ public Object getInstance() { @Override public boolean isVerified() { - return verified; + return configuration.isVerified(); + } + + private boolean isGlobal() { + return configuration.isGlobal(); } @Override public IDefaultResponse getDefaultResponse() { - return defaultResponse; + return configuration.getDefaultResponse(); } @Override @@ -84,9 +86,9 @@ public Specification getSpecification() { @Override public boolean matches(Object target, IMockInteraction interaction) { - if (target instanceof Wildcard) return verified || !interaction.isRequired(); + if (target instanceof Wildcard) return isVerified() || !interaction.isRequired(); - boolean match = global ? matchGlobal(target) : instance == target; + boolean match = isGlobal() ? matchGlobal(target) : instance == target; if (match) { checkRequiredInteractionAllowed(interaction); } @@ -102,10 +104,9 @@ private boolean isMockOfClass() { } private void checkRequiredInteractionAllowed(IMockInteraction interaction) { - if (!verified && interaction.isRequired()) { - String mockName = name != null ? name : "unnamed"; + if (!isVerified() && interaction.isRequired()) { 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(getMockName(), interaction); } } @@ -120,4 +121,9 @@ public void detach() { this.specification = null; this.mockInterceptor.detach(); } + + @Override + public IMockConfiguration getConfiguration() { + return configuration; + } } diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/mockito/MockitoMockMakerImpl.java b/spock-core/src/main/java/org/spockframework/mock/runtime/mockito/MockitoMockMakerImpl.java index dea6f9b283..fdb7027c58 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/mockito/MockitoMockMakerImpl.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/mockito/MockitoMockMakerImpl.java @@ -29,6 +29,7 @@ import org.mockito.mock.MockCreationSettings; import org.mockito.plugins.MockMaker; import org.mockito.plugins.MockitoPlugins; +import org.spockframework.compiler.SpockNames; import org.spockframework.mock.*; import org.spockframework.mock.runtime.IMockMaker; import org.spockframework.mock.runtime.IProxyBasedMockInterceptor; @@ -53,7 +54,7 @@ class MockitoMockMakerImpl { MockitoMockMakerImpl() { inlineMockMaker = resolveInlineMockMaker(); - spockMockMethod = ReflectionUtil.getMethodByName(ISpockMockObject.class, "$spock_get"); + spockMockMethod = ReflectionUtil.getMethodByName(ISpockMockObject.class, SpockNames.SPOCK_GET); } /** diff --git a/spock-core/src/main/java/org/spockframework/runtime/GroovyRuntimeUtil.java b/spock-core/src/main/java/org/spockframework/runtime/GroovyRuntimeUtil.java index c9a7cd34fa..3686e1eb30 100644 --- a/spock-core/src/main/java/org/spockframework/runtime/GroovyRuntimeUtil.java +++ b/spock-core/src/main/java/org/spockframework/runtime/GroovyRuntimeUtil.java @@ -37,6 +37,10 @@ * @author Peter Niederwieser */ public abstract class GroovyRuntimeUtil { + private static final String SET = "set"; + private static final String GET = "get"; + private static final String IS = "is"; + public static Object[] EMPTY_ARGUMENTS = new Object[0]; public static boolean isTruthy(Object obj) { @@ -101,6 +105,18 @@ public static String propertyToMethodName(String prefix, String propertyName) { return prefix + MetaClassHelper.capitalize(propertyName); } + public static String propertyToGetterMethodName(String propertyName) { + return propertyToMethodName(GET, propertyName); + } + + public static String propertyToSetterMethodName(String propertyName) { + return propertyToMethodName(SET, propertyName); + } + + public static String propertyToBooleanGetterMethodName(String propertyName) { + return propertyToMethodName(IS, propertyName); + } + /** * Checks if the given method is a getter method according * to Groovy rules. If yes, the corresponding property name @@ -115,10 +131,10 @@ public static String getterMethodToPropertyName(String methodName, List if (!parameterTypes.isEmpty()) return null; if (returnType == void.class) return null; // Void.class is allowed - if (methodName.startsWith("get")) + if (methodName.startsWith(GET)) return getPropertyName(methodName, 3); - if (methodName.startsWith("is") + if (methodName.startsWith(IS) && returnType == boolean.class) // Boolean.class is not allowed return getPropertyName(methodName, 2); diff --git a/spock-specs/src/test/groovy/org/spockframework/runtime/GroovyRuntimeUtilSpec.groovy b/spock-specs/src/test/groovy/org/spockframework/runtime/GroovyRuntimeUtilSpec.groovy index 6b7b2a148f..59f25844de 100644 --- a/spock-specs/src/test/groovy/org/spockframework/runtime/GroovyRuntimeUtilSpec.groovy +++ b/spock-specs/src/test/groovy/org/spockframework/runtime/GroovyRuntimeUtilSpec.groovy @@ -15,7 +15,11 @@ package org.spockframework.runtime import org.codehaus.groovy.runtime.typehandling.GroovyCastException -import spock.lang.* +import spock.lang.Specification + +import static org.spockframework.runtime.GroovyRuntimeUtil.propertyToBooleanGetterMethodName +import static org.spockframework.runtime.GroovyRuntimeUtil.propertyToGetterMethodName +import static org.spockframework.runtime.GroovyRuntimeUtil.propertyToSetterMethodName class GroovyRuntimeUtilSpec extends Specification { def "getterMethodToPropertyName"() { @@ -39,10 +43,26 @@ class GroovyRuntimeUtilSpec extends Specification { "get" | Integer | null "is" | boolean | null "foo" | String | null + "isfoo" | String | null "setFoo" | void | null } + def "propertyToGetterMethodName"() { + expect: + propertyToGetterMethodName("prop") == "getProp" + } + + def "propertyToSetterMethodName"() { + expect: + propertyToSetterMethodName("prop") == "setProp" + } + + def "propertyToBooleanGetterMethodName"() { + expect: + propertyToBooleanGetterMethodName("prop") == "isProp" + } + def "coerce"() { expect: GroovyRuntimeUtil.coerce("x", Character) == "x" as Character diff --git a/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaMocks.groovy b/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaMocks.groovy new file mode 100644 index 0000000000..22e77192ca --- /dev/null +++ b/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaMocks.groovy @@ -0,0 +1,189 @@ +/* + * Copyright 2024 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * https://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.spockframework.smoke.mock + +import org.spockframework.runtime.InvalidSpecException +import spock.lang.Issue +import spock.lang.Specification +import spock.mock.MockMakers + +class JavaMocks extends Specification { + + def "can mock final classes"() { + when: + def person = Mock(FinalPerson) + person.phoneNumber >> 6789 + + then: + person.phoneNumber == "6789" + } + + def "can mock final methods as property with mockito"() { + FinalMethodPerson person = Mock(mockMaker: MockMakers.mockito) + person.phoneNumber >> 6789 + + expect: + person.phoneNumber == "6789" + } + + def "can mock final methods with mockito"() { + given: + FinalMethodPerson person = Mock(mockMaker: MockMakers.mockito) + person.getPhoneNumber() >> 6789 + + expect: + person.getPhoneNumber() == "6789" + } + + def "can mock final methods with mockito with closure"() { + given: + FinalMethodPerson person = Mock(mockMaker: MockMakers.mockito) { + phoneNumber >> 6789 + } + + expect: + person.phoneNumber == "6789" + } + + def "can mock final methods with mockito with closure and specified type"() { + FinalMethodPerson person = Mock(FinalMethodPerson, mockMaker: MockMakers.mockito) { + phoneNumber >> 6789 + } + + expect: + person.phoneNumber == "6789" + } + + @Issue("https://github.com/spockframework/spock/issues/2039") + def "cannot mock final methods with byteBuddy"() { + given: + FinalMethodPerson person = Mock(mockMaker: MockMakers.byteBuddy) + + when: + person.getPhoneNumber() >> 6789 + + then: + InvalidSpecException ex = thrown() + ex.message == "The final method 'getPhoneNumber' of 'person' can't be mocked by the 'byte-buddy' mock maker. Please use another mock maker supporting final methods." + + expect: + person.getPhoneNumber() == "12345" + } + + @Issue("https://github.com/spockframework/spock/issues/2039") + def "cannot mock final methods with byteBuddy without error message when one overload is non final"() { + given: + FinalMethodPerson person = Mock(mockMaker: MockMakers.byteBuddy) + + person.finalAndNonFinalOverload() >> "B" + + expect: + person.finalAndNonFinalOverload() == "final" + } + + @Issue("https://github.com/spockframework/spock/issues/2039") + def "non final method overload shall be mockable"() { + given: + FinalMethodPerson person = Mock(mockMaker: MockMakers.byteBuddy) + + person.finalAndNonFinalOverload("A") >> "B" + + expect: + person.finalAndNonFinalOverload("A") == "B" + } + + def "cannot mock final method as property with byteBuddy"() { + given: + FinalMethodPerson person = Mock(mockMaker: MockMakers.byteBuddy) + + when: + person.phoneNumber >> 6789 + + then: + InvalidSpecException ex = thrown() + ex.message == "The final method 'getPhoneNumber' of 'person' can't be mocked by the 'byte-buddy' mock maker. Please use another mock maker supporting final methods." + + expect: + person.phoneNumber == "12345" + } + + def "cannot mock final is getter as property with byteBuddy"() { + given: + FinalMethodPerson person = Mock(mockMaker: MockMakers.byteBuddy) + + when: + person.finalPerson >> false + + then: + InvalidSpecException ex = thrown() + ex.message == "The final method 'isFinalPerson' of 'person' can't be mocked by the 'byte-buddy' mock maker. Please use another mock maker supporting final methods." + + expect: + person.finalPerson + } + + def "cannot mock final methods without specifying mockMaker"() { + given: + FinalMethodPerson person = Mock() + + when: + person.getPhoneNumber() >> 6789 + + then: + InvalidSpecException ex = thrown() + ex.message == "The final method 'getPhoneNumber' of 'person' can't be mocked by the 'byte-buddy' mock maker. Please use another mock maker supporting final methods." + } + + def "no static type specified"() { + when: + Mock() + + then: + InvalidSpecException ex = thrown() + ex.message == "Mock object type cannot be inferred automatically. Please specify a type explicitly (e.g. 'Mock(Person)')." + } + + interface IPerson { + String getName() + + int getAge() + } + + static class Person implements IPerson { + String name = "default" + int age + } + + @SuppressWarnings('GrMethodMayBeStatic') + static final class FinalPerson extends Person { + String getPhoneNumber() { "12345" } + } + + @SuppressWarnings('GrMethodMayBeStatic') + static class FinalMethodPerson extends Person { + + final String getPhoneNumber() { "12345" } + + final boolean isFinalPerson() { return true } + + final String finalAndNonFinalOverload() { + return "final" + } + + String finalAndNonFinalOverload(String arg) { + return arg + } + } +} diff --git a/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaSpies.groovy b/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaSpies.groovy index 30f451a896..135d5b7350 100644 --- a/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaSpies.groovy +++ b/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaSpies.groovy @@ -185,7 +185,7 @@ class JavaSpies extends Specification { person.phoneNumber == "6789" } - def "can spy final methods with mockito"() { + def "can spy final methods as property with mockito"() { FinalMethodPerson person = Spy(mockMaker: MockMakers.mockito) person.phoneNumber >> 6789 @@ -193,6 +193,14 @@ class JavaSpies extends Specification { person.phoneNumber == "6789" } + def "can spy final methods with mockito"() { + FinalMethodPerson person = Spy(mockMaker: MockMakers.mockito) + person.getPhoneNumber() >> 6789 + + expect: + person.getPhoneNumber() == "6789" + } + def "can spy final methods with mockito with closure"() { FinalMethodPerson person = Spy(mockMaker: MockMakers.mockito) { phoneNumber >> 6789 @@ -202,10 +210,32 @@ class JavaSpies extends Specification { person.phoneNumber == "6789" } - def "cannot spy final methods with byteBuddy"() { + @Issue("https://github.com/spockframework/spock/issues/2039") + def "cannot spy on final methods with byteBuddy"() { + FinalMethodPerson person = Spy(mockMaker: MockMakers.byteBuddy) + + when: + person.getPhoneNumber() >> 6789 + + then: + InvalidSpecException ex = thrown() + ex.message == "The final method 'getPhoneNumber' of 'person' can't be mocked by the 'byte-buddy' mock maker. Please use another mock maker supporting final methods." + + expect: + person.getPhoneNumber() == "12345" + } + + @Issue("https://github.com/spockframework/spock/issues/2039") + def "cannot spy on final methods as property with byteBuddy"() { FinalMethodPerson person = Spy(mockMaker: MockMakers.byteBuddy) + + when: person.phoneNumber >> 6789 + then: + InvalidSpecException ex = thrown() + ex.message == "The final method 'getPhoneNumber' of 'person' can't be mocked by the 'byte-buddy' mock maker. Please use another mock maker supporting final methods." + expect: person.phoneNumber == "12345" } @@ -214,10 +244,11 @@ class JavaSpies extends Specification { FinalMethodPerson person = Spy() when: - person.phoneNumber + person.phoneNumber >> 6789 then: - 0 * person.phoneNumber + InvalidSpecException ex = thrown() + ex.message == "The final method 'getPhoneNumber' of 'person' can't be mocked by the 'byte-buddy' mock maker. Please use another mock maker supporting final methods." } def "cannot spy globally"() { diff --git a/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaStubs.groovy b/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaStubs.groovy index 9e70d29cd9..28eb933736 100644 --- a/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaStubs.groovy +++ b/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaStubs.groovy @@ -133,7 +133,7 @@ class JavaStubs extends Specification { person.phoneNumber == "6789" } - def "can stub final methods with mockito"() { + def "can stub final methods as property with mockito"() { FinalMethodPerson person = Stub(mockMaker: MockMakers.mockito) person.phoneNumber >> 6789 @@ -141,6 +141,15 @@ class JavaStubs extends Specification { person.phoneNumber == "6789" } + def "can stub final methods with mockito"() { + given: + FinalMethodPerson person = Mock(mockMaker: MockMakers.mockito) + person.getPhoneNumber() >> 6789 + + expect: + person.getPhoneNumber() == "6789" + } + def "can stub final methods with mockito with closure"() { given: FinalMethodPerson person = Stub(mockMaker: MockMakers.mockito) { @@ -160,20 +169,84 @@ class JavaStubs extends Specification { person.phoneNumber == "6789" } + @Issue("https://github.com/spockframework/spock/issues/2039") def "cannot stub final methods with byteBuddy"() { + given: + FinalMethodPerson person = Stub(mockMaker: MockMakers.byteBuddy) + + when: + person.getPhoneNumber() >> 6789 + + then: + InvalidSpecException ex = thrown() + ex.message == "The final method 'getPhoneNumber' of 'person' can't be mocked by the 'byte-buddy' mock maker. Please use another mock maker supporting final methods." + + expect: + person.getPhoneNumber() == "12345" + } + + @Issue("https://github.com/spockframework/spock/issues/2039") + def "cannot stub final methods with byteBuddy without error message when one overload is non final"() { + given: + FinalMethodPerson person = Stub(mockMaker: MockMakers.byteBuddy) + + person.finalAndNonFinalOverload() >> "B" + + expect: + person.finalAndNonFinalOverload() == "final" + } + + @Issue("https://github.com/spockframework/spock/issues/2039") + def "non final method overload shall be stubable"() { + given: FinalMethodPerson person = Stub(mockMaker: MockMakers.byteBuddy) + + person.finalAndNonFinalOverload("A") >> "B" + + expect: + person.finalAndNonFinalOverload("A") == "B" + } + + def "cannot stub final method as property with byteBuddy"() { + given: + FinalMethodPerson person = Stub(mockMaker: MockMakers.byteBuddy) + + when: person.phoneNumber >> 6789 + then: + InvalidSpecException ex = thrown() + ex.message == "The final method 'getPhoneNumber' of 'person' can't be mocked by the 'byte-buddy' mock maker. Please use another mock maker supporting final methods." + expect: person.phoneNumber == "12345" } + def "cannot stub final is getter as property with byteBuddy"() { + given: + FinalMethodPerson person = Stub(mockMaker: MockMakers.byteBuddy) + + when: + person.finalPerson >> false + + then: + InvalidSpecException ex = thrown() + ex.message == "The final method 'isFinalPerson' of 'person' can't be mocked by the 'byte-buddy' mock maker. Please use another mock maker supporting final methods." + + expect: + person.finalPerson + } + def "cannot stub final methods without specifying mockMaker"() { + given: FinalMethodPerson person = Stub() - person.phoneNumber >> 6789 - expect: - person.phoneNumber == "12345" + when: + person.getPhoneNumber() >> 6789 + + then: + InvalidSpecException ex = thrown() + ex.message == "The final method 'getPhoneNumber' of 'person' can't be mocked by the 'byte-buddy' mock maker. Please use another mock maker supporting final methods." } def "cannot stub globally"() { @@ -208,11 +281,24 @@ class JavaStubs extends Specification { List children } + @SuppressWarnings('GrMethodMayBeStatic') static final class FinalPerson extends Person { String getPhoneNumber() { "12345" } } + @SuppressWarnings('GrMethodMayBeStatic') static class FinalMethodPerson extends Person { + final String getPhoneNumber() { "12345" } + + final boolean isFinalPerson() { return true } + + final String finalAndNonFinalOverload() { + return "final" + } + + String finalAndNonFinalOverload(String arg) { + return arg + } } } diff --git a/spock-specs/src/test/groovy/org/spockframework/smoke/mock/MockDefaultResponses.groovy b/spock-specs/src/test/groovy/org/spockframework/smoke/mock/MockDefaultResponses.groovy index 4a075816bc..434e87cab5 100644 --- a/spock-specs/src/test/groovy/org/spockframework/smoke/mock/MockDefaultResponses.groovy +++ b/spock-specs/src/test/groovy/org/spockframework/smoke/mock/MockDefaultResponses.groovy @@ -16,6 +16,8 @@ package org.spockframework.smoke.mock +import org.spockframework.mock.IMockObject +import org.spockframework.mock.ISpockMockObject import spock.lang.Specification class MockDefaultResponses extends Specification { @@ -72,6 +74,16 @@ class MockDefaultResponses extends Specification { cmock.getDouble().getClass() == Double.class } + def "default impl of ISpockMockObject.spock_mockInteractionValidator"() { + given: + def obj = new ISpockMockObject() { + @Override + IMockObject $spock_get() { return null } + } + expect: + obj.$spock_mockInteractionValidator() == null + } + interface IMockable { boolean getBoolean() byte getByte() diff --git a/spock-spring/src/main/java/org/spockframework/spring/mock/DelegatingInterceptor.java b/spock-spring/src/main/java/org/spockframework/spring/mock/DelegatingInterceptor.java index 29eae58721..ef5739fbec 100644 --- a/spock-spring/src/main/java/org/spockframework/spring/mock/DelegatingInterceptor.java +++ b/spock-spring/src/main/java/org/spockframework/spring/mock/DelegatingInterceptor.java @@ -17,6 +17,7 @@ package org.spockframework.spring.mock; import org.spockframework.mock.*; +import org.spockframework.mock.runtime.BaseMockInterceptor; import org.spockframework.mock.runtime.IProxyBasedMockInterceptor; import org.spockframework.runtime.model.FieldInfo; import org.spockframework.util.*; @@ -35,7 +36,7 @@ public DelegatingInterceptor(FieldInfo fieldInfo) { @Override public Object intercept(Object target, Method method, Object[] arguments, IResponseGenerator realMethodInvoker) { if (method.getDeclaringClass() == ISpockMockObject.class) { - return new MockObjectAdapter(this); + return BaseMockInterceptor.handleSpockMockInterface(method, new MockObjectAdapter(this)); } if (delegate == null) { @@ -91,6 +92,11 @@ public String getName() { return null; } + @Override + public String getMockName() { + return null; + } + @Override public Class getType() { return null; @@ -125,5 +131,10 @@ public Specification getSpecification() { public boolean matches(Object target, IMockInteraction interaction) { return false; } + + @Override + public IMockConfiguration getConfiguration() { + throw new UnsupportedOperationException(); + } } } From 1d796fe5d80e920904f3384cfd431bbe577ec23e Mon Sep 17 00:00:00 2001 From: Andreas Turban Date: Thu, 19 Dec 2024 18:30:53 +0100 Subject: [PATCH 02/11] Update spock-core/src/main/java/org/spockframework/mock/runtime/BaseMockInterceptor.java MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Leonard Brünings --- .../org/spockframework/mock/runtime/BaseMockInterceptor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/BaseMockInterceptor.java b/spock-core/src/main/java/org/spockframework/mock/runtime/BaseMockInterceptor.java index a4e0bf5510..e8112d8199 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/BaseMockInterceptor.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/BaseMockInterceptor.java @@ -62,7 +62,7 @@ protected boolean isMethod(Method method, String name, Class... parameterType } public static Object handleSpockMockInterface(Method method, IMockObject mockObject) { - if (method.getName().equals(SpockNames.SPOCK_MOCK_INTERATION_VALIDATOR)) { + if (SpockNames.SPOCK_MOCK_INTERATION_VALIDATOR.equals(method.getName())) { return null; //This should be handled in the corresponding MockMakers. } return mockObject; From 3ea9e4bc5a9d6a5945da2b11ce83c36dd9e990e9 Mon Sep 17 00:00:00 2001 From: Andreas Turban Date: Thu, 19 Dec 2024 18:31:07 +0100 Subject: [PATCH 03/11] Update spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaMocks.groovy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Leonard Brünings --- .../test/groovy/org/spockframework/smoke/mock/JavaMocks.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaMocks.groovy b/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaMocks.groovy index 22e77192ca..7cef6f18bd 100644 --- a/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaMocks.groovy +++ b/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaMocks.groovy @@ -115,7 +115,7 @@ class JavaMocks extends Specification { InvalidSpecException ex = thrown() ex.message == "The final method 'getPhoneNumber' of 'person' can't be mocked by the 'byte-buddy' mock maker. Please use another mock maker supporting final methods." - expect: + and: person.phoneNumber == "12345" } From c5c08d0a1a4689e62ce8573d4e5a683d5b1717b5 Mon Sep 17 00:00:00 2001 From: Andreas Turban Date: Thu, 19 Dec 2024 18:31:16 +0100 Subject: [PATCH 04/11] Update spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaMocks.groovy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Leonard Brünings --- .../test/groovy/org/spockframework/smoke/mock/JavaMocks.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaMocks.groovy b/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaMocks.groovy index 7cef6f18bd..04eea06e0c 100644 --- a/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaMocks.groovy +++ b/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaMocks.groovy @@ -130,7 +130,7 @@ class JavaMocks extends Specification { InvalidSpecException ex = thrown() ex.message == "The final method 'isFinalPerson' of 'person' can't be mocked by the 'byte-buddy' mock maker. Please use another mock maker supporting final methods." - expect: + and: person.finalPerson } From 92396d4a3c398c44517fb147a52e7f915a65f308 Mon Sep 17 00:00:00 2001 From: Andreas Turban Date: Thu, 19 Dec 2024 18:31:26 +0100 Subject: [PATCH 05/11] Update spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaMocks.groovy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Leonard Brünings --- .../test/groovy/org/spockframework/smoke/mock/JavaMocks.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaMocks.groovy b/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaMocks.groovy index 04eea06e0c..cd01ed5181 100644 --- a/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaMocks.groovy +++ b/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaMocks.groovy @@ -78,7 +78,7 @@ class JavaMocks extends Specification { InvalidSpecException ex = thrown() ex.message == "The final method 'getPhoneNumber' of 'person' can't be mocked by the 'byte-buddy' mock maker. Please use another mock maker supporting final methods." - expect: + and: person.getPhoneNumber() == "12345" } From 8ad5c52e59ef758de1062e57dc41c16b91366fb2 Mon Sep 17 00:00:00 2001 From: AndreasTu Date: Thu, 19 Dec 2024 18:43:16 +0100 Subject: [PATCH 06/11] Fix review finding. --- .../groovy/org/spockframework/smoke/mock/JavaStubs.groovy | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaStubs.groovy b/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaStubs.groovy index 28eb933736..4f5e0ea072 100644 --- a/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaStubs.groovy +++ b/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaStubs.groovy @@ -185,14 +185,18 @@ class JavaStubs extends Specification { person.getPhoneNumber() == "12345" } + /** + * We expect that a stub of a final method with and non-final overload will not issue an error message + * and it will not stub the final method with byte-buddy. + */ @Issue("https://github.com/spockframework/spock/issues/2039") def "cannot stub final methods with byteBuddy without error message when one overload is non final"() { - given: + given: "Try to stub a final method, where there is a non-final overload" FinalMethodPerson person = Stub(mockMaker: MockMakers.byteBuddy) person.finalAndNonFinalOverload() >> "B" - expect: + expect: "It calls the real final method without any error message" person.finalAndNonFinalOverload() == "final" } From 2995d22d77b6af11d7ccb65803d1bb8e53584c8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leonard=20Br=C3=BCnings?= Date: Sat, 28 Dec 2024 14:13:40 +0100 Subject: [PATCH 07/11] Align indentation --- .../java/org/spockframework/compiler/SpecRewriter.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/spock-core/src/main/java/org/spockframework/compiler/SpecRewriter.java b/spock-core/src/main/java/org/spockframework/compiler/SpecRewriter.java index 7472f48f14..6118534648 100644 --- a/spock-core/src/main/java/org/spockframework/compiler/SpecRewriter.java +++ b/spock-core/src/main/java/org/spockframework/compiler/SpecRewriter.java @@ -183,7 +183,7 @@ private void createSharedFieldSetter(Field field) { // use internal name new ConstantExpression(field.getAst().getName())), Token.newSymbol(Types.ASSIGN, -1, -1), - new VariableExpression(SpockNames.SPOCK_VALUE)))); + new VariableExpression(SpockNames.SPOCK_VALUE)))); setter.setSourcePosition(field.getAst()); spec.getAst().addMethod(setter); @@ -552,7 +552,7 @@ public void visitCleanupBlock(CleanupBlock block) { } VariableExpression featureThrowableVar = - new VariableExpression(SpockNames.SPOCK_FEATURE_THROWABLE, nodeCache.Throwable); + new VariableExpression(SpockNames.SPOCK_FEATURE_THROWABLE, nodeCache.Throwable); method.getStatements().add(createVariableDeclarationStatement(featureThrowableVar)); List featureStats = new ArrayList<>(); @@ -564,8 +564,8 @@ public void visitCleanupBlock(CleanupBlock block) { CatchStatement featureCatchStat = createThrowableAssignmentAndRethrowCatchStatement(featureThrowableVar); VariableExpression failedBlock = new VariableExpression(SpockNames.FAILED_BLOCK, nodeCache.BlockInfo); List cleanupStats = asList( - new ExpressionStatement(new DeclarationExpression(failedBlock, Token.newSymbol(Types.ASSIGN, -1, -1), ConstantExpression.NULL)), - createCleanupTryCatch(block, featureThrowableVar, failedBlock)); + new ExpressionStatement(new DeclarationExpression(failedBlock, Token.newSymbol(Types.ASSIGN, -1, -1), ConstantExpression.NULL)), + createCleanupTryCatch(block, featureThrowableVar, failedBlock)); TryCatchStatement tryFinally = new TryCatchStatement( @@ -596,7 +596,7 @@ private TryCatchStatement createCleanupTryCatch(CleanupBlock block, VariableExpr TryCatchStatement tryCatchStat = new TryCatchStatement( new BlockStatement(cleanupStats, null), - ifThrowableIsNotNull(restoreFailedBlock(failedBlock)) + ifThrowableIsNotNull(restoreFailedBlock(failedBlock)) ); tryCatchStat.addCatch(createHandleSuppressedThrowableStatement(featureThrowableVar)); From b2529074887fa6d3740c4310e08941fb751e6e7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leonard=20Br=C3=BCnings?= Date: Sat, 28 Dec 2024 14:14:03 +0100 Subject: [PATCH 08/11] Fix release notes order --- docs/release_notes.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/release_notes.adoc b/docs/release_notes.adoc index 22b5c8f245..4e29a9c0ad 100644 --- a/docs/release_notes.adoc +++ b/docs/release_notes.adoc @@ -19,6 +19,7 @@ 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[] * 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[] @@ -32,7 +33,6 @@ include::include.adoc[] * 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[] * Fix compile error with single explicit assert in switch expression branch spockIssue:1845[] -* 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) From c42e6028d5808a4973123d0ef6845f0b8c440c5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leonard=20Br=C3=BCnings?= Date: Sat, 28 Dec 2024 15:01:06 +0100 Subject: [PATCH 09/11] Add support for @FailsWith to assert errorMessage --- docs/release_notes.adoc | 1 + .../builtin/FailsWithInterceptor.java | 19 ++++++++++++++++++- .../src/main/java/spock/lang/FailsWith.java | 9 +++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/docs/release_notes.adoc b/docs/release_notes.adoc index 4e29a9c0ad..0b1aaf2c6d 100644 --- a/docs/release_notes.adoc +++ b/docs/release_notes.adoc @@ -20,6 +20,7 @@ include::include.adoc[] * 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[] diff --git a/spock-core/src/main/java/org/spockframework/runtime/extension/builtin/FailsWithInterceptor.java b/spock-core/src/main/java/org/spockframework/runtime/extension/builtin/FailsWithInterceptor.java index 0a309e37c5..78931bb018 100644 --- a/spock-core/src/main/java/org/spockframework/runtime/extension/builtin/FailsWithInterceptor.java +++ b/spock-core/src/main/java/org/spockframework/runtime/extension/builtin/FailsWithInterceptor.java @@ -18,8 +18,12 @@ import org.spockframework.runtime.*; import org.spockframework.runtime.extension.*; +import org.spockframework.runtime.model.TextPosition; + import spock.lang.FailsWith; +import java.util.Arrays; + /** * * @author Peter Niederwieser @@ -44,7 +48,20 @@ public void intercept(IMethodInvocation invocation) throws Throwable { } private boolean checkException(Throwable t) { + if(matchesException(t)) { + if (!failsWith.expectedMessage().isEmpty() && !failsWith.expectedMessage().equals(t.getMessage())) { + throw new ConditionNotSatisfiedError(new Condition( + Arrays.asList(t, t.getMessage(), failsWith.expectedMessage()), + "e.value == expectedMessage", + TextPosition.create(-1, -1), null, null, null)); + } + return true; + } + return false; + } + + private boolean matchesException(Throwable t) { return failsWith.value().isInstance(t) - || (t instanceof ConditionFailedWithExceptionError && failsWith.value().isInstance(t.getCause())); + || (t instanceof ConditionFailedWithExceptionError && failsWith.value().isInstance(t.getCause())); } } diff --git a/spock-core/src/main/java/spock/lang/FailsWith.java b/spock-core/src/main/java/spock/lang/FailsWith.java index fac3e08e15..2e57a53ce3 100644 --- a/spock-core/src/main/java/spock/lang/FailsWith.java +++ b/spock-core/src/main/java/spock/lang/FailsWith.java @@ -50,4 +50,13 @@ * @return the reason for the failure */ String reason() default "unknown"; + + /** + * The expected message of the exception. + *

+ * If the value is empty, the message is not checked. + * + * @since 2.4 + */ + String expectedMessage() default ""; } From 7d2f94f5484ef11ac9e867eb1fe9ac8cdd0c4a14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leonard=20Br=C3=BCnings?= Date: Sat, 28 Dec 2024 14:32:41 +0100 Subject: [PATCH 10/11] Align specification to what it does and improve... mock creation message --- .../spockframework/lang/SpecInternals.java | 4 +- .../org/spockframework/mock/MockNature.java | 15 ++- ...JavaMocks.groovy => JavaFinalMocks.groovy} | 105 +++++++++++------- 3 files changed, 76 insertions(+), 48 deletions(-) rename spock-specs/src/test/groovy/org/spockframework/smoke/mock/{JavaMocks.groovy => JavaFinalMocks.groovy} (68%) diff --git a/spock-core/src/main/java/org/spockframework/lang/SpecInternals.java b/spock-core/src/main/java/org/spockframework/lang/SpecInternals.java index 378fd0ad70..68263a7e9f 100644 --- a/spock-core/src/main/java/org/spockframework/lang/SpecInternals.java +++ b/spock-core/src/main/java/org/spockframework/lang/SpecInternals.java @@ -324,8 +324,8 @@ private T createMockImpl(String inferredName, Class inferredType, T insta MockImplementation implementation, Map 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); } diff --git a/spock-core/src/main/java/org/spockframework/mock/MockNature.java b/spock-core/src/main/java/org/spockframework/mock/MockNature.java index 2712b3d1d4..768d84f1a0 100644 --- a/spock-core/src/main/java/org/spockframework/mock/MockNature.java +++ b/spock-core/src/main/java/org/spockframework/mock/MockNature.java @@ -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; } /** @@ -79,4 +81,9 @@ public boolean isUseObjenesis() { public IDefaultResponse getDefaultResponse() { return defaultResponse; } + + @Override + public String toString() { + return methodName; + } } diff --git a/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaMocks.groovy b/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaFinalMocks.groovy similarity index 68% rename from spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaMocks.groovy rename to spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaFinalMocks.groovy index cd01ed5181..bc3f61ae4c 100644 --- a/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaMocks.groovy +++ b/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaFinalMocks.groovy @@ -14,28 +14,37 @@ package org.spockframework.smoke.mock +import spock.lang.FailsWith + import org.spockframework.runtime.InvalidSpecException import spock.lang.Issue import spock.lang.Specification import spock.mock.MockMakers -class JavaMocks extends Specification { +class JavaFinalMocks extends Specification { def "can mock final classes"() { - when: + given: def person = Mock(FinalPerson) - person.phoneNumber >> 6789 + + when: + def result = person.phoneNumber then: - person.phoneNumber == "6789" + 1 * person.phoneNumber >> 6789 + result == "6789" } def "can mock final methods as property with mockito"() { + given: FinalMethodPerson person = Mock(mockMaker: MockMakers.mockito) - person.phoneNumber >> 6789 - expect: - person.phoneNumber == "6789" + when: + def result = person.phoneNumber + + then: + 1 * person.phoneNumber >> 6789 + result == "6789" } def "can mock final methods with mockito"() { @@ -43,14 +52,18 @@ class JavaMocks extends Specification { FinalMethodPerson person = Mock(mockMaker: MockMakers.mockito) person.getPhoneNumber() >> 6789 - expect: - person.getPhoneNumber() == "6789" + when: + def result = person.getPhoneNumber() + + then: + 1 * person.getPhoneNumber() >> 6789 + result == "6789" } def "can mock final methods with mockito with closure"() { given: FinalMethodPerson person = Mock(mockMaker: MockMakers.mockito) { - phoneNumber >> 6789 + 1 * phoneNumber >> 6789 } expect: @@ -58,8 +71,9 @@ class JavaMocks extends Specification { } def "can mock final methods with mockito with closure and specified type"() { + given: FinalMethodPerson person = Mock(FinalMethodPerson, mockMaker: MockMakers.mockito) { - phoneNumber >> 6789 + 1 * phoneNumber >> 6789 } expect: @@ -67,71 +81,78 @@ class JavaMocks extends Specification { } @Issue("https://github.com/spockframework/spock/issues/2039") + @FailsWith( + value = InvalidSpecException, + expectedMessage = "The final method 'getPhoneNumber' of 'person' can't be mocked by the 'byte-buddy' mock maker. Please use another mock maker supporting final methods." + ) def "cannot mock final methods with byteBuddy"() { given: FinalMethodPerson person = Mock(mockMaker: MockMakers.byteBuddy) when: - person.getPhoneNumber() >> 6789 + person.getPhoneNumber() then: - InvalidSpecException ex = thrown() - ex.message == "The final method 'getPhoneNumber' of 'person' can't be mocked by the 'byte-buddy' mock maker. Please use another mock maker supporting final methods." - - and: - person.getPhoneNumber() == "12345" + 1 * person.getPhoneNumber() >> 6789 } - @Issue("https://github.com/spockframework/spock/issues/2039") - def "cannot mock final methods with byteBuddy without error message when one overload is non final"() { + + @FailsWith( + value = InvalidSpecException, + expectedMessage = "The final method 'getPhoneNumber' of 'person' can't be mocked by the 'byte-buddy' mock maker. Please use another mock maker supporting final methods." + ) + def "cannot mock final method as property with byteBuddy"() { given: FinalMethodPerson person = Mock(mockMaker: MockMakers.byteBuddy) - person.finalAndNonFinalOverload() >> "B" + when: + person.phoneNumber - expect: - person.finalAndNonFinalOverload() == "final" + then: + 1 * person.phoneNumber >> 6789 } - @Issue("https://github.com/spockframework/spock/issues/2039") - def "non final method overload shall be mockable"() { + @FailsWith( + value = InvalidSpecException, + expectedMessage = "The final method 'isFinalPerson' of 'person' can't be mocked by the 'byte-buddy' mock maker. Please use another mock maker supporting final methods." + ) + def "cannot mock final is getter as property with byteBuddy"() { given: FinalMethodPerson person = Mock(mockMaker: MockMakers.byteBuddy) - person.finalAndNonFinalOverload("A") >> "B" + when: + person.finalPerson - expect: - person.finalAndNonFinalOverload("A") == "B" + then: + 1 * person.finalPerson >> false } - def "cannot mock final method as property with byteBuddy"() { + @Issue("https://github.com/spockframework/spock/issues/2039") + def "cannot mock final methods with byteBuddy when one overload is non final no error message is produced"() { given: FinalMethodPerson person = Mock(mockMaker: MockMakers.byteBuddy) - when: - person.phoneNumber >> 6789 + when: "calling the method that has both a final and non final overload" + def result = person.finalAndNonFinalOverload() - then: - InvalidSpecException ex = thrown() - ex.message == "The final method 'getPhoneNumber' of 'person' can't be mocked by the 'byte-buddy' mock maker. Please use another mock maker supporting final methods." + then: "the mocking does not work" + 0 * person.finalAndNonFinalOverload() >> "B" - and: - person.phoneNumber == "12345" + and: "the result is not stubbed" + result == "final" } - def "cannot mock final is getter as property with byteBuddy"() { + @Issue("https://github.com/spockframework/spock/issues/2039") + def "non final method overload shall be mockable"() { given: FinalMethodPerson person = Mock(mockMaker: MockMakers.byteBuddy) when: - person.finalPerson >> false + def result = person.finalAndNonFinalOverload("A") then: - InvalidSpecException ex = thrown() - ex.message == "The final method 'isFinalPerson' of 'person' can't be mocked by the 'byte-buddy' mock maker. Please use another mock maker supporting final methods." - - and: - person.finalPerson + person.finalAndNonFinalOverload("A") >> "B" + result == "B" } def "cannot mock final methods without specifying mockMaker"() { From c1486473a9272bc1ed18975d6ad0d14db551a0ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leonard=20Br=C3=BCnings?= Date: Sat, 28 Dec 2024 14:37:33 +0100 Subject: [PATCH 11/11] Fix minor things --- .../smoke/mock/JavaSpies.groovy | 22 +++++++++++++++++-- .../smoke/mock/JavaStubs.groovy | 7 +++++- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaSpies.groovy b/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaSpies.groovy index 135d5b7350..d82bc73ba3 100644 --- a/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaSpies.groovy +++ b/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaSpies.groovy @@ -31,6 +31,7 @@ class JavaSpies extends Specification { } def "construct spied-on object using provided constructor args"() { + given: Constructable spy = Spy(constructorArgs: ctorArgs) expect: @@ -51,6 +52,7 @@ class JavaSpies extends Specification { } def "call real methods by default"() { + given: Person person = Spy(constructorArgs: ["fred", 42]) expect: @@ -59,6 +61,7 @@ class JavaSpies extends Specification { } def "call real equals method by default"() { + given: Person fred1 = Spy(constructorArgs: ["fred", 42]) Person fred2 = Spy(constructorArgs: ["fred", 21]) Person barney = Spy(constructorArgs: ["barney", 33]) @@ -69,6 +72,7 @@ class JavaSpies extends Specification { } def "call real hashCode method by default"() { + given: Person person = Spy(constructorArgs: ["fred", 42]) expect: @@ -76,6 +80,7 @@ class JavaSpies extends Specification { } def "call real toString method by default"() { + given: Person person = Spy(constructorArgs: ["fred", 42]) expect: @@ -83,6 +88,7 @@ class JavaSpies extends Specification { } def "can verify interactions with real methods"() { + given: Person person = Spy(constructorArgs: ["fred", 42]) when: @@ -96,6 +102,7 @@ class JavaSpies extends Specification { } def "can be used as partial mocks"() { + given: def person = Spy(Person, constructorArgs: ["fred", 42]) { getWorkHours() >>> [3, 2, 1] } @@ -107,6 +114,7 @@ class JavaSpies extends Specification { } def "can spy on concrete instances"() { + given: def person = Spy(new Person()) when: @@ -119,6 +127,7 @@ class JavaSpies extends Specification { @Issue("https://github.com/spockframework/spock/issues/1035") def "using >>_ does not call original method and produces stubbed value"() { + given: def person = Spy(new Person()) when: @@ -138,6 +147,7 @@ class JavaSpies extends Specification { @Issue("https://github.com/spockframework/spock/issues/771") def "spying on concrete instances can use partial mocking"() { + given: def person = Spy(new Person()) when: @@ -186,6 +196,7 @@ class JavaSpies extends Specification { } def "can spy final methods as property with mockito"() { + given: FinalMethodPerson person = Spy(mockMaker: MockMakers.mockito) person.phoneNumber >> 6789 @@ -194,6 +205,7 @@ class JavaSpies extends Specification { } def "can spy final methods with mockito"() { + given: FinalMethodPerson person = Spy(mockMaker: MockMakers.mockito) person.getPhoneNumber() >> 6789 @@ -202,6 +214,7 @@ class JavaSpies extends Specification { } def "can spy final methods with mockito with closure"() { + given: FinalMethodPerson person = Spy(mockMaker: MockMakers.mockito) { phoneNumber >> 6789 } @@ -212,6 +225,7 @@ class JavaSpies extends Specification { @Issue("https://github.com/spockframework/spock/issues/2039") def "cannot spy on final methods with byteBuddy"() { + given: FinalMethodPerson person = Spy(mockMaker: MockMakers.byteBuddy) when: @@ -227,6 +241,7 @@ class JavaSpies extends Specification { @Issue("https://github.com/spockframework/spock/issues/2039") def "cannot spy on final methods as property with byteBuddy"() { + given: FinalMethodPerson person = Spy(mockMaker: MockMakers.byteBuddy) when: @@ -241,6 +256,7 @@ class JavaSpies extends Specification { } def "cannot spy on final methods without specifying mockMaker"() { + given: FinalMethodPerson person = Spy() when: @@ -299,15 +315,17 @@ class JavaSpies extends Specification { def "no static type specified"() { when: - Stub() + Spy() + then: InvalidSpecException ex = thrown() - ex.message == "Mock object type cannot be inferred automatically. Please specify a type explicitly (e.g. 'Mock(Person)')." + ex.message == "Spy object type cannot be inferred automatically. Please specify a type explicitly (e.g. 'Spy(Person)')." } def "specified instance is null"() { when: Spy((Object) null) + then: SpockException ex = thrown() ex.message == "Spy instance may not be null" diff --git a/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaStubs.groovy b/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaStubs.groovy index 4f5e0ea072..d1ae7186c3 100644 --- a/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaStubs.groovy +++ b/spock-specs/src/test/groovy/org/spockframework/smoke/mock/JavaStubs.groovy @@ -32,6 +32,7 @@ class JavaStubs extends Specification { } def "can be stubbed (using property syntax)"() { + given: person.name >> "fred" expect: @@ -39,6 +40,7 @@ class JavaStubs extends Specification { } def "can be stubbed (using method syntax)"() { + given: person.getName() >> "fred" expect: @@ -77,6 +79,7 @@ class JavaStubs extends Specification { } def "like to be stubbed at creation time"() { + given: person = Stub(IPerson) { getName() >> "fred" } @@ -87,6 +90,7 @@ class JavaStubs extends Specification { @FailsWith(InvalidSpecException) def "cannot be mocked"() { + given: 1 * person.name >> "fred" expect: @@ -134,6 +138,7 @@ class JavaStubs extends Specification { } def "can stub final methods as property with mockito"() { + given: FinalMethodPerson person = Stub(mockMaker: MockMakers.mockito) person.phoneNumber >> 6789 @@ -268,7 +273,7 @@ class JavaStubs extends Specification { then: InvalidSpecException ex = thrown() - ex.message == "Mock object type cannot be inferred automatically. Please specify a type explicitly (e.g. 'Mock(Person)')." + ex.message == "Stub object type cannot be inferred automatically. Please specify a type explicitly (e.g. 'Stub(Person)')." } interface IPerson {