Improve TooManyInvocationsError reporting#2315
Improve TooManyInvocationsError reporting#2315leonard84 wants to merge 1 commit intospockframework:masterfrom
Conversation
it now reports unsatisfied interactions with argument mismatch details. Varargs methods now correctly expand args in mismatch descriptions instead of reporting `<too few arguments>`.
📝 WalkthroughWalkthroughThese changes enhance mock invocation error diagnostics by introducing an enrichment mechanism for TooManyInvocationsError. New interface methods enable error enrichment with unsatisfied interactions and argument mismatch details. Implementation distributes this logic across interaction scopes, mock controllers, and constraint handlers, with comprehensive test coverage. Changes
Sequence DiagramsequenceDiagram
participant Controller as MockController
participant Scope as InteractionScope
participant Error as TooManyInvocationsError
participant Message as getMessage()
Controller->>Controller: catch TooManyInvocationsError
Controller->>Scope: enrichError(error)
Scope->>Scope: getUnsatisfiedInteractions()
Scope->>Error: enrichWithScopeContext(unsatisfied)
Error->>Error: store unsatisfiedInteractions
Error->>Error: clear cached message
Note over Controller,Error: Later, when error details requested
Controller->>Error: getMessage()
Error->>Message: check if unsatisfiedInteractions present
alt has unsatisfied
Message->>Message: appendUnmatchedInvocations()
Message->>Message: scoreInvocations()
Message->>Message: render mismatches with ranking
end
Message-->>Controller: enhanced error message
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR significantly improves Key changes:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant SUT as SUT (when block)
participant MC as MockController
participant IS as InteractionScope
participant MI as MockInteraction
participant TMIE as TooManyInvocationsError
SUT->>MC: handle(invocation)
MC->>IS: match(invocation)
IS-->>MC: exhausted interaction
MC->>MI: accept(invocation)
MI-->>MC: Supplier that throws TooManyInvocationsError
MC->>MC: resultSupplier.get()
MC->>IS: enrichError(error) [synchronized]
IS->>IS: getUnsatisfiedInteractions()
IS->>TMIE: enrichWithScopeContext(unsatisfiedList)
MC->>MC: errors.add(e) then rethrow
Note over TMIE: getMessage() called later
TMIE->>TMIE: build header and accepted invocations
loop for each relevant unsatisfied interaction
TMIE->>MI: matchesTargetAndMethod(acceptedInvocation)
MI-->>TMIE: true or false
TMIE->>MI: computeSimilarityScore(invocation)
MI-->>TMIE: score
TMIE->>MI: describeMismatch(invocation)
MI-->>TMIE: mismatch text
end
TMIE-->>SUT: formatted error with mismatch details
Last reviewed commit: faba9f0 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| final IMockInvocation invocation; | ||
| final int count; | ||
| final int score; | ||
|
|
||
| private ScoredInvocation(IMockInvocation invocation, int count, int score) { |
There was a problem hiding this comment.
Unused count field in ScoredInvocation
The count field is assigned in the constructor but is never read anywhere — not in compareTo, not in the rendering loop in appendUnmatchedInvocations, and not elsewhere. This is dead code that will confuse future readers, who might assume count-based tie-breaking is happening.
If the original intent was to rank more-frequently-seen invocations higher when scores are equal (which would make the "ordered by similarity" section more useful), the compareTo method is missing that secondary sort key. Otherwise the field should be removed.
| final IMockInvocation invocation; | |
| final int count; | |
| final int score; | |
| private ScoredInvocation(IMockInvocation invocation, int count, int score) { | |
| private static class ScoredInvocation implements Comparable<ScoredInvocation> { | |
| final IMockInvocation invocation; | |
| final int score; | |
| private ScoredInvocation(IMockInvocation invocation, int score) { | |
| this.invocation = invocation; | |
| this.score = score; | |
| } |
| private List<ScoredInvocation> scoreInvocations(IMockInteraction interaction, IMultiset<IMockInvocation> invocations) { | ||
| List<ScoredInvocation> result = new ArrayList<>(); | ||
| for (Map.Entry<IMockInvocation, Integer> entry : invocations.entrySet()) { | ||
| result.add(new ScoredInvocation(entry.getKey(), entry.getValue(), interaction.computeSimilarityScore(entry.getKey()))); | ||
| } | ||
| sort(result); | ||
| return result; | ||
| } |
There was a problem hiding this comment.
count argument passed but never used
scoreInvocations passes entry.getValue() as the count to ScoredInvocation, but since count is unused (see the inner class below), this call site passes unnecessary data. If count is intentionally kept for future use, a comment explaining the intent would help; otherwise, removing both the field and the argument here keeps the code clean.
| private List<ScoredInvocation> scoreInvocations(IMockInteraction interaction, IMultiset<IMockInvocation> invocations) { | |
| List<ScoredInvocation> result = new ArrayList<>(); | |
| for (Map.Entry<IMockInvocation, Integer> entry : invocations.entrySet()) { | |
| result.add(new ScoredInvocation(entry.getKey(), entry.getValue(), interaction.computeSimilarityScore(entry.getKey()))); | |
| } | |
| sort(result); | |
| return result; | |
| } | |
| private List<ScoredInvocation> scoreInvocations(IMockInteraction interaction, IMultiset<IMockInvocation> invocations) { | |
| List<ScoredInvocation> result = new ArrayList<>(); | |
| for (Map.Entry<IMockInvocation, Integer> entry : invocations.entrySet()) { | |
| result.add(new ScoredInvocation(entry.getKey(), interaction.computeSimilarityScore(entry.getKey()))); | |
| } | |
| sort(result); | |
| return result; | |
| } |
|
|
||
| boolean isRequired(); | ||
|
|
||
| boolean matchesTargetAndMethod(IMockInvocation invocation); |
There was a problem hiding this comment.
Breaking API change — no default implementation
IMockInteraction is a public interface, so adding matchesTargetAndMethod without a default method is a source-incompatible change for any third-party code that directly implements this interface (not via MockInteractionDecorator).
Providing a sensible default — e.g., delegating to matches(invocation) as a conservative fallback — would preserve binary and source compatibility:
| boolean matchesTargetAndMethod(IMockInvocation invocation); | |
| default boolean matchesTargetAndMethod(IMockInvocation invocation) { | |
| return matches(invocation); | |
| } |
|
|
||
| void verifyInteractions(); | ||
|
|
||
| void enrichError(TooManyInvocationsError error); |
There was a problem hiding this comment.
Breaking API change — no default implementation
IInteractionScope is a public interface. Adding enrichError without a default is a source-incompatible change. A no-op default would allow third-party implementations to compile without modifications — mirroring the same no-op used in DefaultJavaLangObjectInteractions:
| void enrichError(TooManyInvocationsError error); | |
| default void enrichError(TooManyInvocationsError error) { | |
| // no enrichment by default | |
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
spock-specs/src/test/groovy/org/spockframework/smoke/mock/TooManyInvocations.groovy (1)
44-56: Consider a small normalized-message assertion helper.The new tests repeat the same
normalize(e.message)/expected = normalize(...)scaffold many times. A tiny helper would leave each test focused on the message body and make future formatter edits cheaper.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spock-specs/src/test/groovy/org/spockframework/smoke/mock/TooManyInvocations.groovy` around lines 44 - 56, Introduce a small helper to centralize the repeated normalize-and-compare logic: add a method (e.g. assertNormalizedMessage(actualMessage, expectedBody) or assertNormalizedException(e, expectedBody)) that calls normalize on the actual and on the trimmed expected body and performs the equality assertion; replace the repeated patterns in TooManyInvocations.groovy (where code uses normalize(e.message), builds expected = normalize("""...""".trim()), and then compares) with calls to this new helper so each test only contains the expected message body.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/release_notes.adoc`:
- Line 15: Rewrite the release note bullet to a full, scannable sentence:
replace the fragment "* Fix argument mismatch description for varargs methods
now correctly expanding varargs instead of reporting `<too few arguments>`" with
a clearer line like "Fix argument mismatch descriptions for varargs methods by
expanding varargs instead of reporting `<too few arguments>`", ensuring the
phrasing mentions "argument mismatch descriptions", "varargs methods", and the
behavior change (expanding varargs vs reporting `<too few arguments>`).
In
`@spock-core/src/main/java/org/spockframework/mock/constraint/PositionalArgumentListConstraint.java`:
- Around line 53-55: The current logic in PositionalArgumentListConstraint only
calls expandVarArgs(args) when argConstraints.size() != args.size(), which fails
for single-vararg calls where the raw invocation.getArguments() is an array
wrapper; change the condition to call expandVarArgs whenever
hasExpandableVarArgs(invocation.getMethod(), args) is true (regardless of
argConstraints.size() equality) so args is expanded before comparison; update
the block that references argConstraints, args,
hasExpandableVarArgs(invocation.getMethod(), args), and expandVarArgs(args) to
always expand when hasExpandableVarArgs returns true.
In `@spock-core/src/main/java/org/spockframework/mock/IInteractionScope.java`:
- Around line 35-36: Change the abstract method declaration "void
enrichError(TooManyInvocationsError error);" in the public interface
IInteractionScope into a default method: declare "default void
enrichError(TooManyInvocationsError error) { ... }" so external custom
implementations keep working; implement the default body with the common
behavior used by Spock's internal implementations (or a safe no-op if the common
logic cannot be moved here) and, if needed, extract shared logic into a
private/static helper used by the default method and existing implementations so
callers of IInteractionScope.enrichError get the same behavior without breaking
binary compatibility.
In `@spock-core/src/main/java/org/spockframework/mock/IMockInteraction.java`:
- Around line 51-52: Change the new abstract method signature boolean
matchesTargetAndMethod(IMockInvocation invocation) in the IMockInteraction
interface to a default method that delegates to the existing
matches(IMockInvocation invocation) to preserve binary compatibility; implement
default boolean matchesTargetAndMethod(IMockInvocation invocation) { return
matches(invocation); } so third‑party implementations need not be recompiled and
can still override if desired (mirroring the delegation used in
DefaultInteraction).
In
`@spock-core/src/main/java/org/spockframework/mock/runtime/MockController.java`:
- Around line 65-68: The catch block uses scopes.getFirst() which can change
before the catch runs; instead capture the specific scope that matched the
invocation while still inside the original synchronized section and use that
captured scope to call enrichError; modify MockController so the code path that
performs the matching (inside the synchronized block where scopes and invocation
checks occur) assigns the matching scope to a local variable (e.g.
matchingScope) and then in the TooManyInvocationsError catch call
matchingScope.enrichError((TooManyInvocationsError)e) rather than
scopes.getFirst(), ensuring the enrichError call uses the exact scope that
produced the error.
---
Nitpick comments:
In
`@spock-specs/src/test/groovy/org/spockframework/smoke/mock/TooManyInvocations.groovy`:
- Around line 44-56: Introduce a small helper to centralize the repeated
normalize-and-compare logic: add a method (e.g.
assertNormalizedMessage(actualMessage, expectedBody) or
assertNormalizedException(e, expectedBody)) that calls normalize on the actual
and on the trimmed expected body and performs the equality assertion; replace
the repeated patterns in TooManyInvocations.groovy (where code uses
normalize(e.message), builds expected = normalize("""...""".trim()), and then
compares) with calls to this new helper so each test only contains the expected
message body.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2226f5d9-204b-4b43-8ffe-bbfe2e7e073b
📒 Files selected for processing (12)
docs/release_notes.adocspock-core/src/main/java/org/spockframework/mock/DefaultInteraction.javaspock-core/src/main/java/org/spockframework/mock/DefaultJavaLangObjectInteractions.javaspock-core/src/main/java/org/spockframework/mock/IInteractionScope.javaspock-core/src/main/java/org/spockframework/mock/IMockInteraction.javaspock-core/src/main/java/org/spockframework/mock/TooManyInvocationsError.javaspock-core/src/main/java/org/spockframework/mock/constraint/PositionalArgumentListConstraint.javaspock-core/src/main/java/org/spockframework/mock/runtime/InteractionScope.javaspock-core/src/main/java/org/spockframework/mock/runtime/MockController.javaspock-core/src/main/java/org/spockframework/mock/runtime/MockInteraction.javaspock-core/src/main/java/org/spockframework/mock/runtime/MockInteractionDecorator.javaspock-specs/src/test/groovy/org/spockframework/smoke/mock/TooManyInvocations.groovy
|
|
||
| === Misc | ||
|
|
||
| * Fix argument mismatch description for varargs methods now correctly expanding varargs instead of reporting `<too few arguments>` |
There was a problem hiding this comment.
Reword this bullet for readability.
This currently reads like a sentence fragment. Something like “Fix argument mismatch descriptions for varargs methods by expanding varargs instead of reporting <too few arguments>” is easier to scan in the release notes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/release_notes.adoc` at line 15, Rewrite the release note bullet to a
full, scannable sentence: replace the fragment "* Fix argument mismatch
description for varargs methods now correctly expanding varargs instead of
reporting `<too few arguments>`" with a clearer line like "Fix argument mismatch
descriptions for varargs methods by expanding varargs instead of reporting `<too
few arguments>`", ensuring the phrasing mentions "argument mismatch
descriptions", "varargs methods", and the behavior change (expanding varargs vs
reporting `<too few arguments>`).
| if (argConstraints.size() != args.size() && hasExpandableVarArgs(invocation.getMethod(), args)) { | ||
| args = expandVarArgs(args); | ||
| } |
There was a problem hiding this comment.
Expand varargs here whenever the raw argument form mismatches.
Line 53 only expands when the counts differ. For a call like foo(String... xs) with one actual vararg, invocation.getArguments() is still the array wrapper, so a mismatch such as expected foo("a") vs actual foo("b") skips expansion and Line 72 reports against the array object instead of the expanded element. That leaves the new diagnostics wrong for the common single-vararg case.
Possible fix
- if (argConstraints.size() != args.size() && hasExpandableVarArgs(invocation.getMethod(), args)) {
+ if (hasExpandableVarArgs(invocation.getMethod(), args) && !areConstraintsSatisfiedBy(args)) {
args = expandVarArgs(args);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@spock-core/src/main/java/org/spockframework/mock/constraint/PositionalArgumentListConstraint.java`
around lines 53 - 55, The current logic in PositionalArgumentListConstraint only
calls expandVarArgs(args) when argConstraints.size() != args.size(), which fails
for single-vararg calls where the raw invocation.getArguments() is an array
wrapper; change the condition to call expandVarArgs whenever
hasExpandableVarArgs(invocation.getMethod(), args) is true (regardless of
argConstraints.size() equality) so args is expanded before comparison; update
the block that references argConstraints, args,
hasExpandableVarArgs(invocation.getMethod(), args), and expandVarArgs(args) to
always expand when hasExpandableVarArgs returns true.
|
|
||
| void enrichError(TooManyInvocationsError error); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and examine the IInteractionScope interface
find . -name "IInteractionScope.java" -type fRepository: spockframework/spock
Length of output: 137
🏁 Script executed:
# Examine the interface content
cat -n spock-core/src/main/java/org/spockframework/mock/IInteractionScope.javaRepository: spockframework/spock
Length of output: 1455
🏁 Script executed:
# Check for implementations of IInteractionScope
rg "implements IInteractionScope" -t javaRepository: spockframework/spock
Length of output: 369
🏁 Script executed:
# Check if this is a public API interface and understand its visibility
rg "public interface IInteractionScope" -A 20 -B 5Repository: spockframework/spock
Length of output: 1837
🏁 Script executed:
# Check implementations for enrichError method
rg "enrichError" -A 3 spock-core/src/main/java/org/spockframework/mock/runtime/InteractionScope.javaRepository: spockframework/spock
Length of output: 191
🏁 Script executed:
# Check DefaultJavaLangObjectInteractions implementation
rg "enrichError" -A 3 spock-core/src/main/java/org/spockframework/mock/DefaultJavaLangObjectInteractions.javaRepository: spockframework/spock
Length of output: 179
🏁 Script executed:
# Check project Java version target to confirm default methods are supported
rg "source|target|maven.compiler" spock-core/pom.xml | head -20Repository: spockframework/spock
Length of output: 122
🏁 Script executed:
# Search for any documentation about binary compatibility or API guarantees
find . -name "*.md" -o -name "CHANGELOG*" -o -name "HISTORY*" | head -10Repository: spockframework/spock
Length of output: 46
🏁 Script executed:
# Find pom.xml files
find . -name "pom.xml" -type f | head -5Repository: spockframework/spock
Length of output: 46
🏁 Script executed:
# Check main pom.xml for Java version
cat -n pom.xml | head -50Repository: spockframework/spock
Length of output: 103
🏁 Script executed:
# Look for Java version configuration
rg "source|target" --type xml -A 1 -B 1 | head -30Repository: spockframework/spock
Length of output: 2598
🏁 Script executed:
# Check if this is part of a public API specification or if there are release notes
find . -name "*.md" -type f | xargs grep -l "API\|compatibility\|breaking" 2>/dev/null | head -5Repository: spockframework/spock
Length of output: 46
🏁 Script executed:
# Find build files - try gradle or other build systems
find . -maxdepth 2 -name "build.gradle*" -o -name "build.gradle.kts" -o -name "settings.gradle*"Repository: spockframework/spock
Length of output: 126
🏁 Script executed:
# Check for any gradle or maven files in the root
ls -la | grep -E "pom|gradle|build"Repository: spockframework/spock
Length of output: 506
🏁 Script executed:
# Look for version info and compatibility notes
find . -name "README*" -o -name "RELEASE*" -o -name "VERSION*" | head -5Repository: spockframework/spock
Length of output: 109
🏁 Script executed:
# Check if enrichError method is new or was previously optional
git log --oneline --all -S "enrichError" | head -10Repository: spockframework/spock
Length of output: 150
🏁 Script executed:
# Check build.gradle for Java version
cat build.gradle | grep -A 5 -B 5 "java\|sourceCompatibility\|targetCompatibility" | head -40Repository: spockframework/spock
Length of output: 1540
🏁 Script executed:
# Search for any additional implementations of IInteractionScope
rg "class.*implements.*IInteractionScope" -t javaRepository: spockframework/spock
Length of output: 369
🏁 Script executed:
# Check if there are any subclasses that might also be affected
rg "extends InteractionScope" -t javaRepository: spockframework/spock
Length of output: 46
🏁 Script executed:
# Look for external documentation about this being public API
cat README.adoc | head -100Repository: spockframework/spock
Length of output: 4348
🏁 Script executed:
# Check git history to understand the context of this change
git log --oneline --all | grep -i "enrichError\|IInteractionScope" | head -5Repository: spockframework/spock
Length of output: 46
🏁 Script executed:
# Check if there are any external packages or submodules that might implement IInteractionScope
rg "org\.spockframework\.mock\.IInteractionScope" --type java | grep -v "^spock-core"Repository: spockframework/spock
Length of output: 46
🏁 Script executed:
# Verify that Java 8+ is indeed the minimum target
grep -A 10 "java" build.gradle | grep -i "version\|compatibility\|source\|target" | head -10Repository: spockframework/spock
Length of output: 566
🏁 Script executed:
# Check the current state of the method to be absolutely certain it's abstract and not default
sed -n '35,37p' spock-core/src/main/java/org/spockframework/mock/IInteractionScope.javaRepository: spockframework/spock
Length of output: 117
🏁 Script executed:
# Look for any documentation about API compatibility in Spock
find . -name "*.adoc" -o -name "*.md" | xargs grep -l "API\|breaking\|compatibility" 2>/dev/null | head -3Repository: spockframework/spock
Length of output: 122
🏁 Script executed:
# Check if there's a changelog or migration guide
find . -name "*CHANGE*" -o -name "*MIGRATE*" -o -name "*RELEASE*" 2>/dev/null | head -5Repository: spockframework/spock
Length of output: 46
🏁 Script executed:
# Check Spock's versioning and release policy
cat -n docs/extensions.adoc | head -50Repository: spockframework/spock
Length of output: 2403
🏁 Script executed:
# Look for any semantic versioning or API stability guarantees
rg "breaking\|API.*change\|compatibility.*policy" -i docs/Repository: spockframework/spock
Length of output: 46
🏁 Script executed:
# Verify the project structure - confirm IInteractionScope is in the public API
find spock-core/src/main -name "*.java" | xargs grep "public.*class.*IInteraction" | head -5Repository: spockframework/spock
Length of output: 535
Make enrichError a default method for backwards compatibility with custom scope implementations.
IInteractionScope is public API, so adding enrichError(...) as an abstract method breaks existing external implementations. Since the method has concrete behavior in all known implementations within Spock itself, using a default method instead ensures external implementations don't break on upgrade.
Recommended fix
- void enrichError(TooManyInvocationsError error);
+ default void enrichError(TooManyInvocationsError error) {
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@spock-core/src/main/java/org/spockframework/mock/IInteractionScope.java`
around lines 35 - 36, Change the abstract method declaration "void
enrichError(TooManyInvocationsError error);" in the public interface
IInteractionScope into a default method: declare "default void
enrichError(TooManyInvocationsError error) { ... }" so external custom
implementations keep working; implement the default body with the common
behavior used by Spock's internal implementations (or a safe no-op if the common
logic cannot be moved here) and, if needed, extract shared logic into a
private/static helper used by the default method and existing implementations so
callers of IInteractionScope.enrichError get the same behavior without breaking
binary compatibility.
|
|
||
| boolean matchesTargetAndMethod(IMockInvocation invocation); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and examine IMockInteraction interface
fd IMockInteraction.java --type f -x cat -n {}Repository: spockframework/spock
Length of output: 1810
🏁 Script executed:
# Check for implementations of IMockInteraction
rg "implements IMockInteraction" -A 2Repository: spockframework/spock
Length of output: 1537
🏁 Script executed:
# Look for the matches method referenced in the suggested delegation
rg "boolean matches\(" spock-core/src/main/java/org/spockframework/mock/ -A 2Repository: spockframework/spock
Length of output: 3797
🏁 Script executed:
# Check Java version in build configuration
fd -e pom.xml -e build.gradle | head -5 | xargs grep -l "source\|target\|java" 2>/dev/null | head -3Repository: spockframework/spock
Length of output: 46
🏁 Script executed:
# Check if matchesTargetAndMethod is already implemented in the codebase
rg "matchesTargetAndMethod" --type java -A 3Repository: spockframework/spock
Length of output: 2792
🏁 Script executed:
# Check Java version in pom.xml
find . -name "pom.xml" -type f | head -1 | xargs grep -E "source|target|java.version" | head -20Repository: spockframework/spock
Length of output: 46
🏁 Script executed:
# Look for where matchesTargetAndMethod is being called
rg "matchesTargetAndMethod" -B 2 -A 2Repository: spockframework/spock
Length of output: 3602
🏁 Script executed:
# Check git history to see if matchesTargetAndMethod is a new addition
git log --oneline -p -- spock-core/src/main/java/org/spockframework/mock/IMockInteraction.java | head -100Repository: spockframework/spock
Length of output: 1854
🏁 Script executed:
# Check for Java version in root pom.xml
cat pom.xml | grep -A 5 -B 5 "source\|target\|java.version" | head -30Repository: spockframework/spock
Length of output: 103
🏁 Script executed:
# Check all Default*Interaction classes to ensure complete picture
fd "Default.*Interaction\.java" spock-core/src/main/java/org/spockframework/mock/ -x basename {}Repository: spockframework/spock
Length of output: 246
🏁 Script executed:
# Find the correct pom.xml or gradle file
find . -name "pom.xml" -o -name "build.gradle" -o -name "build.gradle.kts" | head -3Repository: spockframework/spock
Length of output: 78
🏁 Script executed:
# Check root directory for build config
ls -la | head -20Repository: spockframework/spock
Length of output: 1223
🏁 Script executed:
# Look for Java version in spock-core pom.xml
cat spock-core/pom.xml | grep -A 5 -B 5 "source\|target\|java.version" | head -40Repository: spockframework/spock
Length of output: 114
🏁 Script executed:
# Check Java version in build.gradle
cat build.gradle | grep -A 3 -B 3 "source\|target\|java\|jvm" | head -50Repository: spockframework/spock
Length of output: 1825
🏁 Script executed:
# Also check if there's a gradle.properties or settings.gradle for version info
cat gradle.properties 2>/dev/null || cat settings.gradle 2>/dev/null | head -30Repository: spockframework/spock
Length of output: 958
🏁 Script executed:
# Verify all Default*Interaction subclasses to see if they implement matchesTargetAndMethod
rg "class Default.*Interaction" -A 20 | grep -E "class Default|matchesTargetAndMethod"Repository: spockframework/spock
Length of output: 1093
Make this a default method to avoid breaking third-party implementations.
IMockInteraction is public API. Adding a new abstract method means any external implementation compiled against a previous release will fail to load without recompilation and implementation of matchesTargetAndMethod. A default implementation delegating to matches(invocation) preserves compatibility while allowing overrides. The same delegation pattern already exists in DefaultInteraction, confirming semantic correctness.
Compatibility-safe shape
- boolean matchesTargetAndMethod(IMockInvocation invocation);
+ default boolean matchesTargetAndMethod(IMockInvocation invocation) {
+ return matches(invocation);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| boolean matchesTargetAndMethod(IMockInvocation invocation); | |
| default boolean matchesTargetAndMethod(IMockInvocation invocation) { | |
| return matches(invocation); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@spock-core/src/main/java/org/spockframework/mock/IMockInteraction.java`
around lines 51 - 52, Change the new abstract method signature boolean
matchesTargetAndMethod(IMockInvocation invocation) in the IMockInteraction
interface to a default method that delegates to the existing
matches(IMockInvocation invocation) to preserve binary compatibility; implement
default boolean matchesTargetAndMethod(IMockInvocation invocation) { return
matches(invocation); } so third‑party implementations need not be recompiled and
can still override if desired (mirroring the delegation used in
DefaultInteraction).
| if (e instanceof TooManyInvocationsError) { | ||
| synchronized (this) { | ||
| scopes.getFirst().enrichError((TooManyInvocationsError) e); | ||
| } |
There was a problem hiding this comment.
Enrich with the matched scope, not whatever is first now.
Line 67 always uses scopes.getFirst(), but the interaction that produced the TooManyInvocationsError can come from a later scope when earlier scopes do not match. Since this happens after leaving the original synchronized block, another thread can also change the deque before the catch runs. That can attach unrelated unsatisfied interactions and make the new diagnostics misleading.
Suggested fix
public Object handle(IMockInvocation invocation) {
Supplier<Object> resultSupplier = null;
+ IInteractionScope matchedScope = null;
synchronized (this) {
for (IInteractionScope scope : scopes) {
IMockInteraction interaction = scope.match(invocation);
if (interaction != null) {
+ matchedScope = scope;
resultSupplier = requireNonNull(interaction.accept(invocation), "interaction must not return null");
break;
}
}
if (resultSupplier == null) {
@@
try {
return resultSupplier.get();
} catch (InteractionNotSatisfiedError e) {
- if (e instanceof TooManyInvocationsError) {
- synchronized (this) {
- scopes.getFirst().enrichError((TooManyInvocationsError) e);
- }
+ if (e instanceof TooManyInvocationsError && matchedScope != null) {
+ matchedScope.enrichError((TooManyInvocationsError) e);
}
errors.add(e);
throw e;
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@spock-core/src/main/java/org/spockframework/mock/runtime/MockController.java`
around lines 65 - 68, The catch block uses scopes.getFirst() which can change
before the catch runs; instead capture the specific scope that matched the
invocation while still inside the original synchronized section and use that
captured scope to call enrichError; modify MockController so the code path that
performs the matching (inside the synchronized block where scopes and invocation
checks occur) assigns the matching scope to a local variable (e.g.
matchingScope) and then in the TooManyInvocationsError catch call
matchingScope.enrichError((TooManyInvocationsError)e) rather than
scopes.getFirst(), ensuring the enrichError call uses the exact scope that
produced the error.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #2315 +/- ##
============================================
+ Coverage 82.13% 82.22% +0.08%
- Complexity 4794 4823 +29
============================================
Files 469 469
Lines 14954 15032 +78
Branches 1888 1907 +19
============================================
+ Hits 12283 12360 +77
+ Misses 1984 1982 -2
- Partials 687 690 +3
🚀 New features to boost your workflow:
|
|
|
||
| === Enhancements | ||
|
|
||
| * `TooManyInvocationsError` now reports unsatisfied interactions with argument mismatch details, making it easier to diagnose why invocations didn't match expected interactions |
There was a problem hiding this comment.
| * `TooManyInvocationsError` now reports unsatisfied interactions with argument mismatch details, making it easier to diagnose why invocations didn't match expected interactions | |
| * `TooManyInvocationsError` now reports unsatisfied interactions with argument mismatch details, making it easier to diagnose why invocations didn't match expected interactions spockPull:2315[] |
|
|
||
| === Misc | ||
|
|
||
| * Fix argument mismatch description for varargs methods now correctly expanding varargs instead of reporting `<too few arguments>` |
There was a problem hiding this comment.
| * Fix argument mismatch description for varargs methods now correctly expanding varargs instead of reporting `<too few arguments>` | |
| * Fix argument mismatch description for varargs methods now correctly expanding varargs instead of reporting `<too few arguments>` spockPull:2315[] |
| final int score; | ||
|
|
||
| private ScoredInvocation(IMockInvocation invocation, int count, int score) { | ||
| this.invocation = invocation; |
| out.defaultWriteObject(); | ||
| } | ||
|
|
||
| private static class ScoredInvocation implements Comparable<ScoredInvocation> { |
There was a problem hiding this comment.
This is the same code as in TooFewInvocationsError, how about extract the logic into a package-private Util class?
| for (ScoredInvocation si : scored) { | ||
| if (idx++ < 5) { | ||
| try { | ||
| builder.append(unsatisfied.describeMismatch(si.invocation)); | ||
| } catch (AssertionError | Exception e) { | ||
| builder.append("<Renderer threw Exception>: ").append(e.getMessage()); | ||
| } | ||
| builder.append('\n'); | ||
| } |
There was a problem hiding this comment.
This is similar code as in TooFewInvocationsError, how about extract the logic into a package-private Util class?
| | | | | ||
| 0 | 1 | ||
| false | ||
| '''.trim()) |
There was a problem hiding this comment.
| '''.trim()) | |
| ''') |
| | | | | ||
| 2 | 1 | ||
| false | ||
| '''.trim()) |
There was a problem hiding this comment.
| '''.trim()) | |
| ''') |
| | | | | ||
| 2 | 1 | ||
| false | ||
| '''.trim()) |
There was a problem hiding this comment.
| '''.trim()) | |
| ''') |
| Matching invocations (ordered by last occurrence): | ||
|
|
||
| 1 * list.add(2) <-- this triggered the error | ||
| '''.trim()) |
There was a problem hiding this comment.
| '''.trim()) | |
| ''') |
There was a problem hiding this comment.
Note: All the trim() calls, you already do that in the normalize() method.
I stopped to mark everything.
| if (e instanceof TooManyInvocationsError) { | ||
| synchronized (this) { | ||
| scopes.getFirst().enrichError((TooManyInvocationsError) e); | ||
| } | ||
| } |
There was a problem hiding this comment.
Maybe extract this into a new method to make it more readable.
2 participants
it now reports unsatisfied interactions with argument mismatch details.
Varargs methods now correctly expand args in mismatch descriptions instead of reporting
<too few arguments>.Summary by CodeRabbit
Bug Fixes
Tests