diff --git a/docs/interaction_based_testing.adoc b/docs/interaction_based_testing.adoc index 0cd6956152..92a16a5bc3 100644 --- a/docs/interaction_based_testing.adoc +++ b/docs/interaction_based_testing.adoc @@ -952,6 +952,8 @@ it is only used to describe the interaction. NOTE: A global mock can only be created for a class type. It effectively replaces all instances of that type for the duration from mock creation up until the end of the feature method. +CAUTION: Using global mocks for standard types from the JDK, for example `ArrayList`, is a bad idea and can lead to unforeseen and widespread consequences. + CAUTION: The declaration order of global mocks is relevant. The `GroovySpy(global:true, )` must come before all creations of new mocked/spied objects of ``. The global spies will only take effect on objects of that type, if the `GroovySpy(global:true, )` was @@ -983,6 +985,15 @@ When `Specifications` or `Features` are executed concurrently you have to make s global mocks on the same types are properly guarded against each other, because a global mock changes the global state for the mocked `Class` during execution. +[[global-mocks-parallel-execution]] +==== Global mocks and parallel execution + +Creating a global `GroovyMock`/`GroovyStub`/`GroovySpy` when <> is enabled, +requires that the spec is annotated with <> or `@ResourceLock(org.spockframework.runtime.model.parallel.Resources.META_CLASS_REGISTRY)`. +If it is only used for a feature, then it suffices that the feature is annotated with `@ResourceLock(org.spockframework.runtime.model.parallel.Resources.META_CLASS_REGISTRY)`. +The rule of thumb to choose between `@ResourceLock` and `@Isolated`, is to look at how widespread the mocked type is used. +If it is widely used, then `@Isolated` is the safe choice, otherwise `@ResourceLock` may be sufficient. + .How Are Global Groovy Mocks Implemented? **** Global Groovy mocks get their super powers from Groovy meta-programming. To be more precise, diff --git a/docs/parallel_execution.adoc b/docs/parallel_execution.adoc index a927155b41..e632812874 100644 --- a/docs/parallel_execution.adoc +++ b/docs/parallel_execution.adoc @@ -643,6 +643,7 @@ skinparam shadowing false @endwbs .... +[[isolated-execution]] === Isolated Execution Sometimes, you want to modify and test something that affects every other feature, you could put a `READ` `@ResourceLock` on _every_ feature, but that is impractical. diff --git a/docs/release_notes.adoc b/docs/release_notes.adoc index f58c8ab3d1..ef8a610dca 100644 --- a/docs/release_notes.adoc +++ b/docs/release_notes.adoc @@ -8,6 +8,9 @@ include::include.adoc[] === Breaking Changes * Calling `old(...)` with multiple arguments is now a compilation error. Previously the additional arguments were simply ignored. +* Creating `GroovyMock`/`GroovyStub`/`GroovySpy` for an already mocked type will now fail. +* Creating a global `GroovyMock`/`GroovyStub`/`GroovySpy` when <> is enabled, + will now require that the spec is annotated with <> or `@ResourceLock(org.spockframework.runtime.model.parallel.Resources.META_CLASS_REGISTRY)`. See <> === Misc * Add support for <> diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/GroovyMockFactory.java b/spock-core/src/main/java/org/spockframework/mock/runtime/GroovyMockFactory.java index b577b93cdd..e3a8e979b1 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/GroovyMockFactory.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/GroovyMockFactory.java @@ -17,15 +17,28 @@ import org.spockframework.mock.*; import org.spockframework.runtime.GroovyRuntimeUtil; import org.spockframework.runtime.RunContext; +import org.spockframework.runtime.model.FeatureInfo; +import org.spockframework.runtime.model.IterationInfo; +import org.spockframework.runtime.model.parallel.ExclusiveResource; +import org.spockframework.runtime.model.parallel.ResourceAccessMode; +import org.spockframework.runtime.model.parallel.Resources; import org.spockframework.util.ReflectionUtil; import org.spockframework.util.SpockDocLinks; +import spock.config.RunnerConfiguration; import spock.lang.Specification; import java.lang.reflect.Modifier; +import java.util.Set; import groovy.lang.*; public class GroovyMockFactory implements IMockFactory { + + private static final ExclusiveResource META_CLASS_REGISTRY_RW = new ExclusiveResource(Resources.META_CLASS_REGISTRY, + ResourceAccessMode.READ_WRITE); + private static final ExclusiveResource GLOBAL_LOCK = new ExclusiveResource( + org.junit.platform.engine.support.hierarchical.ExclusiveResource.GLOBAL_KEY, ResourceAccessMode.READ_WRITE); + public static final GroovyMockFactory INSTANCE = new GroovyMockFactory(); @Override @@ -35,12 +48,23 @@ public boolean canCreate(IMockConfiguration configuration) { @Override public Object create(IMockConfiguration configuration, Specification specification) throws CannotCreateMockException { + final Class type = configuration.getType(); final MetaClass oldMetaClass = GroovyRuntimeUtil.getMetaClass(configuration.getType()); + if (oldMetaClass instanceof GroovyMockMetaClass) { + throw new CannotCreateMockException(type, + ". The given type is already mocked by Spock."); + } GroovyMockMetaClass newMetaClass = new GroovyMockMetaClass(configuration, specification, oldMetaClass); - final Class type = configuration.getType(); boolean hasAdditionalInterfaces = !configuration.getAdditionalInterfaces().isEmpty(); if (configuration.isGlobal()) { + if (!isIsolatedOrHasMetaClassRegistryReadWriteLock(specification)) { + throw new CannotCreateMockException(type, + ". Global mocking in parallel execution mode is only possible, when the specification is @Isolated, " + + "or the specification or feature is annotated with " + + "@ResourceLock(org.spockframework.runtime.model.parallel.Resources.META_CLASS_REGISTRY)."); + } + if (type.isInterface()) { throw new CannotCreateMockException(type, ". Global mocking is only possible for classes, but not for interfaces."); @@ -93,6 +117,20 @@ public Object create(IMockConfiguration configuration, Specification specificati return proxy; } + private boolean isIsolatedOrHasMetaClassRegistryReadWriteLock(Specification specification) { + if (!RunContext.get().getConfiguration(RunnerConfiguration.class).parallel.enabled) { + // we don't have a problem in single threaded execution + return true; + } + FeatureInfo feature = specification.getSpecificationContext().getCurrentIteration().getFeature(); + Set specExclusiveResources = feature.getSpec().getExclusiveResources(); + if (specExclusiveResources.contains(GLOBAL_LOCK) || specExclusiveResources.contains(META_CLASS_REGISTRY_RW)) { + return true; + } + // GLOBAL_LOCK can't be declared on the feature + return feature.getExclusiveResources().contains(META_CLASS_REGISTRY_RW); + } + private boolean isFinalClass(Class type) { return !type.isInterface() && Modifier.isFinal(type.getModifiers()); } diff --git a/spock-specs/src/test/groovy/org/spockframework/docs/interaction/GlobalMockDocSpec.groovy b/spock-specs/src/test/groovy/org/spockframework/docs/interaction/GlobalMockDocSpec.groovy index 18203b3989..d60730f41c 100644 --- a/spock-specs/src/test/groovy/org/spockframework/docs/interaction/GlobalMockDocSpec.groovy +++ b/spock-specs/src/test/groovy/org/spockframework/docs/interaction/GlobalMockDocSpec.groovy @@ -1,10 +1,13 @@ package org.spockframework.docs.interaction +import org.spockframework.runtime.model.parallel.Resources import spock.lang.Issue +import spock.lang.ResourceLock import spock.lang.Specification import spock.lang.Stepwise @Stepwise +@ResourceLock(Resources.META_CLASS_REGISTRY) class GlobalMockDocSpec extends Specification { @Issue("https://github.com/spockframework/spock/issues/785") diff --git a/spock-specs/src/test/groovy/org/spockframework/smoke/mock/GroovyMockGlobalClass.groovy b/spock-specs/src/test/groovy/org/spockframework/smoke/mock/GroovyMockGlobalClass.groovy index d2ac722404..c20e9db93a 100644 --- a/spock-specs/src/test/groovy/org/spockframework/smoke/mock/GroovyMockGlobalClass.groovy +++ b/spock-specs/src/test/groovy/org/spockframework/smoke/mock/GroovyMockGlobalClass.groovy @@ -1,8 +1,11 @@ package org.spockframework.smoke.mock -import org.spockframework.smoke.mock.Subject -import spock.lang.* +import org.spockframework.runtime.model.parallel.Resources +import spock.lang.Issue +import spock.lang.ResourceLock +import spock.lang.Specification +@ResourceLock(Resources.META_CLASS_REGISTRY) @Issue('https://github.com/spockframework/spock/issues/761') class GroovyMockGlobalClass extends Specification { @@ -32,4 +35,5 @@ class Subject { } class ClassA {} + class ClassB {} diff --git a/spock-specs/src/test/groovy/org/spockframework/smoke/mock/GroovyMocks.groovy b/spock-specs/src/test/groovy/org/spockframework/smoke/mock/GroovyMocks.groovy index 414963145c..c71b824f81 100644 --- a/spock-specs/src/test/groovy/org/spockframework/smoke/mock/GroovyMocks.groovy +++ b/spock-specs/src/test/groovy/org/spockframework/smoke/mock/GroovyMocks.groovy @@ -1,6 +1,10 @@ package org.spockframework.smoke.mock +import org.spockframework.mock.CannotCreateMockException +import org.spockframework.runtime.model.parallel.Resources +import spock.lang.ResourceLock import spock.lang.Specification +import spock.lang.Unroll class GroovyMocks extends Specification { def "implement GroovyObject"() { @@ -8,4 +12,39 @@ class GroovyMocks extends Specification { GroovyMock(List) instanceof GroovyObject GroovyMock(ArrayList) instanceof GroovyObject } + + @ResourceLock(Resources.META_CLASS_REGISTRY) + @Unroll("A Groovy#typeB can't be created for a type that was already Groovy#typeA'd") + def "global GroovyMocks can't be created for a type that is already mocked"(String typeA, String typeB) { + given: + createMock(typeA) + + when: + createMock(typeB) + + then: + CannotCreateMockException e = thrown() + e.message == 'Cannot create mock for class org.spockframework.smoke.mock.GroovyMocks$LocalClassForMocking. The given type is already mocked by Spock.' + + where: + [typeA, typeB] << ([['Mock', 'Stub', 'Spy']] * 2).combinations() + } + + void createMock(String type) { + switch (type) { + case 'Mock': + GroovyMock(global: true, LocalClassForMocking) + break + case 'Stub': + GroovyStub(global: true, LocalClassForMocking) + break + case 'Spy': + GroovySpy(global: true, LocalClassForMocking) + break + default: + throw new IllegalArgumentException(type) + } + } + + class LocalClassForMocking {} }