root
/
spring-framework
mirror of https://github.com/spring-projects/spring-framework.git
1
0
Fork 0
Code Issues Actions 8 Packages Projects Releases Wiki Activity

Fix off-by-one regression in AbstractMethodMockingControl 

This commit fixes the off-by-one regression accidentally introduced in
commit 55961544a7.

Specifically, this fix ensures that the correct recorded call is
indexed in the 'calls' list in the implementation of
AbstractMethodMockingControl.Expectations.nextCall().

In addition, this commit improves the Javadoc for
AbstractMethodMockingControl, @MockStaticEntityMethods, and
AnnotationDrivenStaticEntityMockingControl and introduces a proper
toString() implementation for the internal Expectations.Call class in
AbstractMethodMockingControl. Furthermore, code from the obsolete
Delegate test class has been inlined in
AnnotationDrivenStaticEntityMockingControlTests.

Issue: SPR-11385, SPR-10885
This commit is contained in:
Sam Brannen 2014-02-04 23:14:39 +01:00
parent 1cd5071e6a
commit 69a89b1bb0
5 changed files with 166 additions and 197 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

107
spring-aspects/src/main/java/org/springframework/mock/staticmock/AbstractMethodMockingControl.aj
View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2013 the original author or authors.
* Copyright 2002-2014 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.
@ -18,16 +18,22 @@ package org.springframework.mock.staticmock;
import java.util.Arrays;
import java.util.LinkedList;
import java.util.List;
import org.springframework.util.ObjectUtils;
/**
* Abstract aspect to enable mocking of methods picked out by a pointcut.
* Sub-aspects must define the mockStaticsTestMethod() pointcut to
* indicate call stacks when mocking should be triggered, and the
* methodToMock() pointcut to pick out a method invocations to mock.
*
* <p>Sub-aspects must define:
* <ul>
* <li>the {@link #mockStaticsTestMethod()} pointcut to indicate call stacks
* when mocking should be triggered
* <li>the {@link #methodToMock()} pointcut to pick out method invocations to mock
* </ul>
*
* @author Rod Johnson
* @author Ramnivas Laddad
* @author Sam Brannen
*/
public abstract aspect AbstractMethodMockingControl percflow(mockStaticsTestMethod()) {
@ -35,24 +41,34 @@ public abstract aspect AbstractMethodMockingControl percflow(mockStaticsTestMeth
protected abstract pointcut methodToMock();
private boolean recording = true;
static enum CallResponse { nothing, return_, throw_ };
// Represents a list of expected calls to static entity methods
static enum CallResponse {
nothing, return_, throw_
};
/**
* Represents a list of expected calls to methods.
*/
// Public to allow inserted code to access: is this normal??
public class Expectations {
// Represents an expected call to a static entity method
/**
* Represents an expected call to a method.
*/
private class Call {
private final String signature;
private final Object[] args;
private Object responseObject; // return value or throwable
private CallResponse responseType = CallResponse.nothing;
public Call(String name, Object[] args) {
this.signature = name;
public Call(String signature, Object[] args) {
this.signature = signature;
this.args = args;
}
@ -77,7 +93,7 @@ public abstract aspect AbstractMethodMockingControl percflow(mockStaticsTestMeth
public Object throwException(String lastSig, Object[] args) {
checkSignature(lastSig, args);
throw (RuntimeException)responseObject;
throw (RuntimeException) responseObject;
}
private void checkSignature(String lastSig, Object[] args) {
@ -88,16 +104,29 @@ public abstract aspect AbstractMethodMockingControl percflow(mockStaticsTestMeth
throw new IllegalArgumentException("Arguments don't match");
}
}
@Override
public String toString() {
return String.format("Call with signature [%s] and arguments %s", this.signature,
ObjectUtils.nullSafeToString(args));
}
}
private List<Call> calls = new LinkedList<Call>();
// Calls already verified
/**
* The list of recorded calls.
*/
private final LinkedList<Call> calls = new LinkedList<Call>();
/**
* The number of calls already verified.
*/
private int verified;
public void verify() {
if (verified != calls.size()) {
throw new IllegalStateException("Expected " + calls.size() + " calls, received " + verified);
throw new IllegalStateException("Expected " + calls.size() + " calls, but received " + verified);
}
}
@ -105,31 +134,32 @@ public abstract aspect AbstractMethodMockingControl percflow(mockStaticsTestMeth
* Validate the call and provide the expected return value.
*/
public Object respond(String lastSig, Object[] args) {
Call call = nextCall();
CallResponse responseType = call.responseType;
if (responseType == CallResponse.return_) {
return call.returnValue(lastSig, args);
Call c = nextCall();
switch (c.responseType) {
case return_: {
return c.returnValue(lastSig, args);
}
else if (responseType == CallResponse.throw_) {
return call.throwException(lastSig, args);
case throw_: {
return c.throwException(lastSig, args);
}
default: {
throw new IllegalStateException("Behavior of " + c + " not specified");
}
else if (responseType == CallResponse.nothing) {
// do nothing
}
throw new IllegalStateException("Behavior of " + call + " not specified");
}
private Call nextCall() {
verified++;
if (verified > calls.size()) {
throw new IllegalStateException("Expected " + calls.size() + " calls, received " + verified);
throw new IllegalStateException("Expected " + calls.size() + " calls, but received " + verified);
}
return calls.get(verified);
// The 'verified' count is 1-based; whereas, 'calls' is 0-based.
return calls.get(verified - 1);
}
public void expectCall(String lastSig, Object lastArgs[]) {
Call call = new Call(lastSig, lastArgs);
calls.add(call);
public void expectCall(String lastSig, Object[] lastArgs) {
calls.add(new Call(lastSig, lastArgs));
}
public boolean hasCalls() {
@ -137,23 +167,26 @@ public abstract aspect AbstractMethodMockingControl percflow(mockStaticsTestMeth
}
public void expectReturn(Object retVal) {
Call call = calls.get(calls.size() - 1);
if (call.hasResponseSpecified()) {
throw new IllegalStateException("No static method invoked before setting return value");
Call c = calls.getLast();
if (c.hasResponseSpecified()) {
throw new IllegalStateException("No method invoked before setting return value");
}
call.setReturnVal(retVal);
c.setReturnVal(retVal);
}
public void expectThrow(Throwable throwable) {
Call call = calls.get(calls.size() - 1);
if (call.hasResponseSpecified()) {
throw new IllegalStateException("No static method invoked before setting throwable");
Call c = calls.getLast();
if (c.hasResponseSpecified()) {
throw new IllegalStateException("No method invoked before setting throwable");
}
c.setThrow(throwable);
}
call.setThrow(throwable);
}
}
private Expectations expectations = new Expectations();
private final Expectations expectations = new Expectations();
after() returning : mockStaticsTestMethod() {
if (recording && (expectations.hasCalls())) {

52
spring-aspects/src/main/java/org/springframework/mock/staticmock/AnnotationDrivenStaticEntityMockingControl.aj
View File

@ -17,20 +17,21 @@
package org.springframework.mock.staticmock;
/**
* Annotation-based aspect to use in test builds to enable mocking static methods
* on JPA-annotated {@code @Entity} classes, as used by Spring Roo for finders.
* Annotation-based aspect to use in test builds to enable mocking of static methods
* on JPA-annotated {@code @Entity} classes, as used by Spring Roo for so-called
* <em>finder methods</em>.
*
* <p>Mocking will occur in the call stack of any method in a class (typically a test class)
* that is annotated with the {@code @MockStaticEntityMethods} annotation.
* <p>Mocking will occur within the call stack of any method in a class (typically a
* test class) that is annotated with {@code @MockStaticEntityMethods}.
*
* <p>Also provides static methods to simplify the programming model for
* entering playback mode and setting expected return values.
* <p>This aspect also provides static methods to simplify the programming model for
* setting expectations and entering playback mode.
*
* <p>Usage:
* <ol>
* <li>Annotate a test class with {@code @MockStaticEntityMethods}.
* <li>In each test method, {@code AnnotationDrivenStaticEntityMockingControl}
* will begin in recording mode.
* will begin in <em>recording</em> mode.
* <li>Invoke static methods on JPA-annotated {@code @Entity} classes, with each
* recording-mode invocation being followed by an invocation of either the static
* {@link #expectReturn(Object)} method or the static {@link #expectThrow(Throwable)}
@ -48,22 +49,37 @@ package org.springframework.mock.staticmock;
public aspect AnnotationDrivenStaticEntityMockingControl extends AbstractMethodMockingControl {
/**
* Stop recording mock calls and enter playback state
* Expect the supplied {@link Object} to be returned by the previous static
* method invocation.
* @see #playback()
*/
public static void expectReturn(Object retVal) {
AnnotationDrivenStaticEntityMockingControl.aspectOf().expectReturnInternal(retVal);
}
/**
* Expect the supplied {@link Throwable} to be thrown by the previous static
* method invocation.
* @see #playback()
*/
public static void expectThrow(Throwable throwable) {
AnnotationDrivenStaticEntityMockingControl.aspectOf().expectThrowInternal(throwable);
}
/**
* Stop recording mock expectations and enter <em>playback</em> mode.
* @see #expectReturn(Object)
* @see #expectThrow(Throwable)
*/
public static void playback() {
AnnotationDrivenStaticEntityMockingControl.aspectOf().playbackInternal();
}
public static void expectReturn(Object retVal) {
AnnotationDrivenStaticEntityMockingControl.aspectOf().expectReturnInternal(retVal);
}
public static void expectThrow(Throwable throwable) {
AnnotationDrivenStaticEntityMockingControl.aspectOf().expectThrowInternal(throwable);
}
// Only matches directly annotated @Test methods, to allow methods in
// @MockStatics classes to invoke each other without resetting the mocking environment
// Apparently, the following pointcut was originally defined to only match
// methods directly annotated with @Test (in order to allow methods in
// @MockStaticEntityMethods classes to invoke each other without resetting
// the mocking environment); however, this is no longer the case. The current
// pointcut applies to all public methods in @MockStaticEntityMethods classes.
protected pointcut mockStaticsTestMethod() : execution(public * (@MockStaticEntityMethods *).*(..));
protected pointcut methodToMock() : execution(public static * (@javax.persistence.Entity *).*(..));

5
spring-aspects/src/main/java/org/springframework/mock/staticmock/MockStaticEntityMethods.java
View File

@ -22,12 +22,13 @@ import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
/**
* Annotation to indicate a test class for whose {code @Test} methods
* Annotation to indicate a test class for whose {@code @Test} methods
* static methods on JPA-annotated {@code @Entity} classes should be mocked.
*
* <p>See {@code AnnotationDrivenStaticEntityMockingControl} for details.
* <p>See {@link AnnotationDrivenStaticEntityMockingControl} for details.
*
* @author Rod Johnson
* @author Sam Brannen
*/
@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.TYPE)

109
spring-aspects/src/test/java/org/springframework/mock/staticmock/AnnotationDrivenStaticEntityMockingControlTests.java
View File

@ -16,16 +16,18 @@
package org.springframework.mock.staticmock;
import java.rmi.RemoteException;
import javax.persistence.PersistenceException;
import org.junit.Ignore;
import org.junit.Test;
import static org.junit.Assert.*;
import static org.springframework.mock.staticmock.AnnotationDrivenStaticEntityMockingControl.*;
/**
* Tests for static entity mocking framework.
* Tests for Spring's static entity mocking framework (i.e., @{@link MockStaticEntityMethods}
* and {@link AnnotationDrivenStaticEntityMockingControl}).
*
* @author Rod Johnson
* @author Ramnivas Laddad
@ -34,10 +36,8 @@ import static org.springframework.mock.staticmock.AnnotationDrivenStaticEntityMo
@MockStaticEntityMethods
public class AnnotationDrivenStaticEntityMockingControlTests {
// TODO Fix failing test
@Ignore
@Test
public void noArgIntReturn() {
public void noArgumentMethodInvocationReturnsInt() {
int expectedCount = 13;
Person.countPeople();
expectReturn(expectedCount);
@ -45,20 +45,16 @@ public class AnnotationDrivenStaticEntityMockingControlTests {
assertEquals(expectedCount, Person.countPeople());
}
// TODO Fix failing test
@Ignore
@Test(expected = PersistenceException.class)
public void noArgThrows() {
public void noArgumentMethodInvocationThrowsException() {
Person.countPeople();
expectThrow(new PersistenceException());
playback();
Person.countPeople();
}
// TODO Fix failing test
@Ignore
@Test
public void argMethodMatches() {
public void methodArgumentsMatch() {
long id = 13;
Person found = new Person();
Person.findPerson(id);
@ -67,8 +63,6 @@ public class AnnotationDrivenStaticEntityMockingControlTests {
assertEquals(found, Person.findPerson(id));
}
// TODO Fix failing test
@Ignore
@Test
public void longSeriesOfCalls() {
long id1 = 13;
@ -91,36 +85,26 @@ public class AnnotationDrivenStaticEntityMockingControlTests {
assertEquals(0, Person.countPeople());
}
/**
* Note delegation is used when tests are invalid and should fail, as otherwise the
* failure will occur on the verify() method in the aspect after this method returns,
* failing the test case.
*/
// TODO Fix failing test
@Ignore
@Test
public void argMethodNoMatchExpectReturn() {
try {
new Delegate().argMethodNoMatchExpectReturn();
fail();
}
catch (IllegalArgumentException expected) {
}
}
// TODO Fix failing test
@Ignore
@Test(expected = IllegalArgumentException.class)
public void argMethodNoMatchExpectThrow() {
new Delegate().argMethodNoMatchExpectThrow();
public void methodArgumentsDoNotMatchAndReturnsObject() {
long id = 13;
Person found = new Person();
Person.findPerson(id);
AnnotationDrivenStaticEntityMockingControl.expectReturn(found);
AnnotationDrivenStaticEntityMockingControl.playback();
assertEquals(found, Person.findPerson(id + 1));
}
private void called(Person found, long id) {
assertEquals(found, Person.findPerson(id));
@Test(expected = IllegalArgumentException.class)
public void methodArgumentsDoNotMatchAndThrowsException() {
long id = 13;
Person found = new Person();
Person.findPerson(id);
AnnotationDrivenStaticEntityMockingControl.expectThrow(new PersistenceException());
AnnotationDrivenStaticEntityMockingControl.playback();
assertEquals(found, Person.findPerson(id + 1));
}
// TODO Fix failing test
@Ignore
@Test
public void reentrant() {
long id = 13;
@ -131,31 +115,56 @@ public class AnnotationDrivenStaticEntityMockingControlTests {
called(found, id);
}
@Test(expected = IllegalStateException.class)
public void rejectUnexpectedCall() {
new Delegate().rejectUnexpectedCall();
private void called(Person found, long id) {
assertEquals(found, Person.findPerson(id));
}
// TODO Fix failing test
@Ignore
@Test(expected = IllegalStateException.class)
public void failTooFewCalls() {
new Delegate().failTooFewCalls();
public void rejectUnexpectedCall() {
AnnotationDrivenStaticEntityMockingControl.playback();
Person.countPeople();
}
@Test(expected = IllegalStateException.class)
public void tooFewCalls() {
long id = 13;
Person found = new Person();
Person.findPerson(id);
AnnotationDrivenStaticEntityMockingControl.expectReturn(found);
Person.countPeople();
AnnotationDrivenStaticEntityMockingControl.expectReturn(25);
AnnotationDrivenStaticEntityMockingControl.playback();
assertEquals(found, Person.findPerson(id));
}
@Test
public void empty() {
// Test that verification check doesn't blow up if no replay() call happened
// Test that verification check doesn't blow up if no replay() call happened.
}
@Test(expected = IllegalStateException.class)
public void doesntEverReplay() {
new Delegate().doesntEverReplay();
public void doesNotEnterPlaybackMode() {
Person.countPeople();
}
@Test(expected = IllegalStateException.class)
public void doesntEverSetReturn() {
new Delegate().doesntEverSetReturn();
public void doesNotSetExpectedReturnValue() {
Person.countPeople();
AnnotationDrivenStaticEntityMockingControl.playback();
}
/**
* Note: this test method currently does NOT actually verify that the mock
* verification fails.
*/
// TODO Determine if it's possible for a mock verification failure to fail a test in
// JUnit 4+ if the test method itself throws an expected exception.
@Test(expected = RemoteException.class)
public void verificationFailsEvenWhenTestFailsInExpectedManner() throws Exception {
Person.countPeople();
AnnotationDrivenStaticEntityMockingControl.playback();
// No calls in order to allow verification failure
throw new RemoteException();
}
}

90
spring-aspects/src/test/java/org/springframework/mock/staticmock/Delegate.java
View File

@ -1,90 +0,0 @@
/*
* Copyright 2002-2014 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
*
* http://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.springframework.mock.staticmock;
import java.rmi.RemoteException;
import javax.persistence.PersistenceException;
import org.junit.Ignore;
import static org.junit.Assert.*;
/**
* This isn't meant for direct testing; rather it is driven from
* {@link AnnotationDrivenStaticEntityMockingControlTests}.
*
* @author Rod Johnson
* @author Ramnivas Laddad
* @author Sam Brannen
*/
@MockStaticEntityMethods
@Ignore("Used because verification failures occur after method returns, so we can't test for them in the test case itself")
public class Delegate {
public void argMethodNoMatchExpectReturn() {
long id = 13;
Person found = new Person();
Person.findPerson(id);
AnnotationDrivenStaticEntityMockingControl.expectReturn(found);
AnnotationDrivenStaticEntityMockingControl.playback();
assertEquals(found, Person.findPerson(id + 1));
}
public void argMethodNoMatchExpectThrow() {
long id = 13;
Person found = new Person();
Person.findPerson(id);
AnnotationDrivenStaticEntityMockingControl.expectThrow(new PersistenceException());
AnnotationDrivenStaticEntityMockingControl.playback();
assertEquals(found, Person.findPerson(id + 1));
}
public void failTooFewCalls() {
long id = 13;
Person found = new Person();
Person.findPerson(id);
AnnotationDrivenStaticEntityMockingControl.expectReturn(found);
Person.countPeople();
AnnotationDrivenStaticEntityMockingControl.expectReturn(25);
AnnotationDrivenStaticEntityMockingControl.playback();
assertEquals(found, Person.findPerson(id));
}
public void doesntEverReplay() {
Person.countPeople();
}
public void doesntEverSetReturn() {
Person.countPeople();
AnnotationDrivenStaticEntityMockingControl.playback();
}
public void rejectUnexpectedCall() {
AnnotationDrivenStaticEntityMockingControl.playback();
Person.countPeople();
}
public void verificationFailsEvenWhenTestFailsInExpectedManner()
throws RemoteException {
Person.countPeople();
AnnotationDrivenStaticEntityMockingControl.playback();
// No calls to allow verification failure
throw new RemoteException();
}
}