Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix off-by-one regression in AbstractMethodMockingControl [SPR-11385] #16012

Closed
2 tasks done
spring-projects-issues opened this issue Feb 3, 2014 · 2 comments
Closed
2 tasks done

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Feb 3, 2014

Sam Brannen opened SPR-11385 and commented

Status Quo

AnnotationDrivenStaticEntityMockingControlTests has been omitted from the build for quite some time (perhaps forever) since it was previously named AnnotationDrivenStaticEntityMockingControlTest which did not comply with Spring Framework's naming convention for test classes. Consequently, the tests within this class have not been included in the test suite.

Due to changes to the build configuration in #16011, it is now apparent that some of the tests in AnnotationDrivenStaticEntityMockingControlTests are broken.


Analysis

Initial analysis of the failing tests has made it apparent that there is an off-by-one bug in the implementation of AbstractMethodMockingControl.Expectations.nextCall(). Specifically, the verified count is one-based; whereas, the calls list is zero-based. Therefore, returning calls.get(verified) leads to inappropriate behavior (i.e., either accessing the wrong call or out-of-bounds exceptions).

Further analysis reveals that this off-by-one bug is in fact a regression that was accidentally introduced in the fix for #15513.


Deliverables

  1. Fix the off-by-one bug in AbstractMethodMockingControl.Expectations.nextCall().
  2. Ensure that all broken tests (currently annotated with @Ignore) in AnnotationDrivenStaticEntityMockingControlTests pass once again.

Affects: 3.2.5

Issue Links:

  • #15513 Abstract method mocking prints wrong call count in exception
  • #16011 Ensure all tests are executed in the Gradle build

Referenced from: commits 3a89bc4, 03e243a, 69a89b1

Backported to: 3.2.8

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Feb 4, 2014

Sam Brannen commented

Fixed as described in the comments for GitHub commit 69a89b1:

Fix off-by-one regression in AbstractMethodMockingControl

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

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.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Feb 4, 2014

Sam Brannen commented

Backported from master to 3.2.x (i.e., for the upcoming 3.2.8 release) in GitHub commit 03e243a.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants