Improve TooManyInvocationsError reporting#2315
Improve TooManyInvocationsError reporting#2315leonard84 merged 9 commits 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>`.
📝 WalkthroughWalkthroughAdds diagnostic context for TooManyInvocationsError: new interface methods to expose non-exhausted interactions and target/method matching, a diagnostic utility for scoring/rendering invocations, runtime changes to collect scope context and enrich errors, varargs mismatch handling improvements, and expanded tests and release notes. Changes
Sequence Diagram(s)sequenceDiagram
actor Caller
participant MockController
participant InteractionScope
participant IMockInteraction
participant TooManyInvocationsError
participant InteractionDiagnostics
Caller->>MockController: invoke mocked method
MockController->>InteractionScope: handle invocation
InteractionScope->>IMockInteraction: match constraints
InteractionScope-->>MockController: raises InteractionNotSatisfiedError/TooManyInvocationsError
alt Error is TooManyInvocationsError
MockController->>InteractionScope: getNonExhaustedInteractions()
InteractionScope-->>MockController: List<IMockInteraction>
MockController->>TooManyInvocationsError: enrichWithScopeContext(interactions)
TooManyInvocationsError->>IMockInteraction: matchesTargetAndMethod(invocation)
IMockInteraction-->>TooManyInvocationsError: boolean
TooManyInvocationsError->>InteractionDiagnostics: scoreMatchingInvocations(...) / appendMismatchDescriptions(...)
InteractionDiagnostics-->>TooManyInvocationsError: scored list / formatted text
TooManyInvocationsError-->>MockController: enriched message
end
MockController-->>Caller: throw enriched error
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)
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 |
spock-core/src/main/java/org/spockframework/mock/TooManyInvocationsError.java
Outdated
Show resolved
Hide resolved
spock-core/src/main/java/org/spockframework/mock/TooManyInvocationsError.java
Outdated
Show resolved
Hide resolved
spock-core/src/main/java/org/spockframework/mock/IMockInteraction.java
Outdated
Show resolved
Hide resolved
spock-core/src/main/java/org/spockframework/mock/IInteractionScope.java
Outdated
Show resolved
Hide resolved
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
...-core/src/main/java/org/spockframework/mock/constraint/PositionalArgumentListConstraint.java
Show resolved
Hide resolved
spock-core/src/main/java/org/spockframework/mock/IInteractionScope.java
Outdated
Show resolved
Hide resolved
spock-core/src/main/java/org/spockframework/mock/IMockInteraction.java
Outdated
Show resolved
Hide resolved
spock-core/src/main/java/org/spockframework/mock/runtime/MockController.java
Outdated
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #2315 +/- ##
============================================
+ Coverage 82.13% 82.23% +0.09%
- Complexity 4797 4827 +30
============================================
Files 469 472 +3
Lines 14954 15036 +82
Branches 1888 1905 +17
============================================
+ Hits 12283 12365 +82
+ Misses 1984 1983 -1
- Partials 687 688 +1
🚀 New features to boost your workflow:
|
spock-core/src/main/java/org/spockframework/mock/TooManyInvocationsError.java
Outdated
Show resolved
Hide resolved
spock-core/src/main/java/org/spockframework/mock/TooManyInvocationsError.java
Outdated
Show resolved
Hide resolved
spock-core/src/main/java/org/spockframework/mock/TooManyInvocationsError.java
Outdated
Show resolved
Hide resolved
spock-specs/src/test/groovy/org/spockframework/smoke/mock/TooManyInvocations.groovy
Outdated
Show resolved
Hide resolved
spock-specs/src/test/groovy/org/spockframework/smoke/mock/TooManyInvocations.groovy
Outdated
Show resolved
Hide resolved
spock-specs/src/test/groovy/org/spockframework/smoke/mock/TooManyInvocations.groovy
Outdated
Show resolved
Hide resolved
spock-specs/src/test/groovy/org/spockframework/smoke/mock/TooManyInvocations.groovy
Outdated
Show resolved
Hide resolved
spock-core/src/main/java/org/spockframework/mock/runtime/MockController.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
🧹 Nitpick comments (2)
spock-core/src/main/java/org/spockframework/mock/InteractionDiagnostics.java (2)
65-76: Off-by-one in mismatch description limit.The loop appends the invocation header (
count * invocation) for all entries, but only appends mismatch descriptions for the first 5 (idx++ < MAX_MISMATCH_DESCRIPTIONS). However, the increment happens after comparison, so whenidx=4, the check passes, thenidxbecomes 5. On the next iteration withidx=5, the check fails. This means exactly 5 descriptions are rendered, which is correct.One minor nit: The header is always rendered but the description may be omitted after 5 entries. Consider adding a note like
"(mismatch details omitted)"for entries beyond the limit to clarify why some invocations lack descriptions.🤖 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/InteractionDiagnostics.java` around lines 65 - 76, In appendScoredInvocations, after rendering the invocation header for entries beyond the MAX_MISMATCH_DESCRIPTIONS limit (using idx and the existing idx++ check that controls appendMismatchDescription), append a short note such as " (mismatch details omitted)" to the builder so callers can see why there is no subsequent mismatch description; keep the current logic that calls appendMismatchDescription(interaction, si.invocation) for the first MAX_MISMATCH_DESCRIPTIONS entries and only add the omission note for subsequent entries.
111-116: Consider using Integer.compare to avoid potential overflow.While similarity scores are likely bounded in practice,
score - other.scorecan overflow for extreme integer values. UsingInteger.compare()is a safer idiom.♻️ Safer comparison
`@Override` public int compareTo(ScoredInvocation other) { - int result = score - other.score; + int result = Integer.compare(score, other.score); if (result != 0) return result; return invocation.toString().compareTo(other.invocation.toString()); }🤖 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/InteractionDiagnostics.java` around lines 111 - 116, The compareTo implementation in ScoredInvocation inside InteractionDiagnostics uses subtraction (score - other.score) which can overflow; replace that subtraction with Integer.compare(score, other.score) to compute the primary ordering, and keep the existing tie-breaker using invocation.toString().compareTo(other.invocation.toString()); update the compareTo(ScoredInvocation other) method accordingly so it returns Integer.compare(score, other.score) if non-zero, otherwise the invocation string comparison.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@spock-core/src/main/java/org/spockframework/mock/InteractionDiagnostics.java`:
- Around line 65-76: In appendScoredInvocations, after rendering the invocation
header for entries beyond the MAX_MISMATCH_DESCRIPTIONS limit (using idx and the
existing idx++ check that controls appendMismatchDescription), append a short
note such as " (mismatch details omitted)" to the builder so callers can see why
there is no subsequent mismatch description; keep the current logic that calls
appendMismatchDescription(interaction, si.invocation) for the first
MAX_MISMATCH_DESCRIPTIONS entries and only add the omission note for subsequent
entries.
- Around line 111-116: The compareTo implementation in ScoredInvocation inside
InteractionDiagnostics uses subtraction (score - other.score) which can
overflow; replace that subtraction with Integer.compare(score, other.score) to
compute the primary ordering, and keep the existing tie-breaker using
invocation.toString().compareTo(other.invocation.toString()); update the
compareTo(ScoredInvocation other) method accordingly so it returns
Integer.compare(score, other.score) if non-zero, otherwise the invocation string
comparison.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 833d2e05-8d26-4eed-a3bc-ca529f9a063c
📒 Files selected for processing (7)
docs/release_notes.adocspock-core/src/main/java/org/spockframework/mock/InteractionDiagnostics.javaspock-core/src/main/java/org/spockframework/mock/TooFewInvocationsError.javaspock-core/src/main/java/org/spockframework/mock/TooManyInvocationsError.javaspock-core/src/main/java/org/spockframework/mock/runtime/MockController.javaspock-specs/src/test/groovy/org/spockframework/smoke/mock/TooFewInvocations.groovyspock-specs/src/test/groovy/org/spockframework/smoke/mock/TooManyInvocations.groovy
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/release_notes.adoc
and check all active scopes
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@spock-core/src/main/java/org/spockframework/mock/InteractionDiagnostics.java`:
- Around line 113-118: The compareTo implementation in InteractionDiagnostics'
ScoredInvocation currently compares only count then invocation.toString(),
ignoring the computed similarity score; update
ScoredInvocation.compareTo(ScoredInvocation other) to order by score first
(higher score => comes first), then by count (higher count => comes first), and
finally by invocation.toString() as a stable tie-breaker — use a numeric
comparison for the score (e.g., Double.compare with appropriate sign for
descending order) followed by Integer.compare for count and string comparison
for invocation.toString().
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a9ad7b05-f38c-4edb-975d-1f0c35bd8dc8
📒 Files selected for processing (6)
spock-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/InteractionDiagnostics.javaspock-core/src/main/java/org/spockframework/mock/runtime/InteractionScope.javaspock-core/src/main/java/org/spockframework/mock/runtime/MockController.javaspock-specs/src/test/groovy/org/spockframework/smoke/mock/TooManyInvocations.groovy
🚧 Files skipped from review as they are similar to previous changes (1)
- spock-core/src/main/java/org/spockframework/mock/runtime/MockController.java
spock-core/src/main/java/org/spockframework/mock/InteractionDiagnostics.java
Show resolved
Hide resolved
spock-core/src/main/java/org/spockframework/mock/InteractionDiagnostics.java
Show resolved
Hide resolved
spock-core/src/main/java/org/spockframework/mock/runtime/MockController.java
Show resolved
Hide resolved
AndreasTu
left a comment
There was a problem hiding this comment.
Thanks for the change and the fixes.
Some test assertions are failing, but the other stuff looks good to me.
13e615a to
0c764ab
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
spock-core/src/main/java/org/spockframework/mock/InteractionDiagnostics.java (1)
113-118:⚠️ Potential issue | 🟠 MajorSort the most relevant invocations to the front.
Line 115 still orders by increasing
score, andcountis no longer used at all. Since Lines 72-74 and 84-87 only render mismatch details for the first five entries, this can put the least useful diagnostics at the top and hide the better matches behind the cap.Suggested fix
`@Override` public int compareTo(ScoredInvocation other) { - int result = Integer.compare(score, other.score); + int result = Integer.compare(other.score, score); if (result != 0) return result; + result = Integer.compare(other.count, count); + if (result != 0) return result; return invocation.toString().compareTo(other.invocation.toString()); }🤖 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/InteractionDiagnostics.java` around lines 113 - 118, The compareTo implementation in ScoredInvocation orders by increasing score and ignores count, causing low-relevance entries to appear first; update ScoredInvocation.compareTo to sort by descending score (compare other.score to score), then by descending count (compare other.count to count) as a tie-breaker, and finally by invocation.toString() for deterministic ordering so the most relevant invocations appear at the front.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@spock-core/src/main/java/org/spockframework/mock/InteractionDiagnostics.java`:
- Around line 95-96: The catch in InteractionDiagnostics currently appends a
hardcoded "<Renderer threw Exception>" even when an AssertionError is caught;
update the catch block in InteractionDiagnostics so the message reflects the
actual throwable (e.g. use a neutral phrase or include
e.getClass().getSimpleName()) instead of always saying "Exception". Locate the
catch (AssertionError | Exception e) and change the builder.append call to
include the runtime type or a neutral label (for example "<Renderer threw: " +
e.getClass().getSimpleName() + ">: " + e.getMessage() or "<Renderer threw an
error>: " + e.getMessage()) so diagnostics are accurate.
---
Duplicate comments:
In
`@spock-core/src/main/java/org/spockframework/mock/InteractionDiagnostics.java`:
- Around line 113-118: The compareTo implementation in ScoredInvocation orders
by increasing score and ignores count, causing low-relevance entries to appear
first; update ScoredInvocation.compareTo to sort by descending score (compare
other.score to score), then by descending count (compare other.count to count)
as a tie-breaker, and finally by invocation.toString() for deterministic
ordering so the most relevant invocations appear at the front.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 17b3c0b7-ed41-4cbb-b00d-330f9ac9d3c2
📒 Files selected for processing (2)
spock-core/src/main/java/org/spockframework/mock/IMockInteraction.javaspock-core/src/main/java/org/spockframework/mock/InteractionDiagnostics.java
🚧 Files skipped from review as they are similar to previous changes (1)
- spock-core/src/main/java/org/spockframework/mock/IMockInteraction.java
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