Skip to content

Commit d0a6e9a

Browse files
Skobeltsynclaude
andcommitted
test(#2048): Branch.matchRoute direct coverage + document NULL_RETURNS suspend-Unit noise
PIT 2026-05-19 showed Branch.matchRoute at 9 NO_COVERAGE despite BranchSessionTest exercising it — PIT can't see through `scope.launch { Channel.consumeAsFlow() }` continuations. Two fixes: 1. BranchMatchRouteTest (9 tests) — call matchRoute directly (it's internal, same-package access). Covers TypeRoute precedence, NullRoute/ElseRoute fallback chain, no-match → null. 2. build.gradle.kts comment documents that NULL_RETURNS is kept but produces equivalent mutants on `suspend fun ... ): Unit` (Kotlin lowers Unit-suspend to JVM Object returning kotlin.Unit). Concrete impact: 26 SURVIVED+NO_COVERAGE in ClaudeClient.dispatchSseEvent are this pattern. Don't chase — same convention as VOID_METHOD_CALLS (dropped) and lambda$N inline-attribution. Filed as fresh ticket #2048 (NOT a reopen of #880, which closed before #1748 added matchRoute). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent c7ea2ae commit d0a6e9a

2 files changed

Lines changed: 199 additions & 0 deletions

File tree

build.gradle.kts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,19 @@ pitest {
115115
// partially-redundant with adjacent calls anyway. Revisit when adding
116116
// an `arcmutate-kotlin-equivalence-filter` plugin for per-call-target
117117
// filtering.
118+
//
119+
// NULL_RETURNS is KEPT but produces known equivalent-mutant noise on
120+
// `suspend fun ... ): Unit` methods. The Kotlin compiler lowers such
121+
// functions to JVM `Object foo(Continuation)` (returns either
122+
// `kotlin.Unit` or `COROUTINE_SUSPENDED`), so PIT applies NULL_RETURNS
123+
// and the mutated `return null` is observationally indistinguishable
124+
// from `return Unit` for any caller. Concrete impact (PIT 2026-05-19):
125+
// 26 SURVIVED+NO_COVERAGE in `ClaudeClient.dispatchSseEvent` are this
126+
// pattern, plus ~7 more in `parseSseStream`/`lambda$*$dispatch`.
127+
// Don't chase these — same convention as the lambda$N inline-attribution
128+
// and VOID_METHOD_CALLS noise. Dropping NULL_RETURNS entirely would
129+
// lose real coverage on object-returning functions (`parseResponse`,
130+
// `materializeSnapshot`, etc), which is a worse trade.
118131
mutators.set(setOf(
119132
"CONDITIONALS_BOUNDARY",
120133
"INCREMENTS",
Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,186 @@
1+
package agents_engine.composition.branch
2+
3+
import agents_engine.core.agent
4+
import kotlinx.coroutines.runBlocking
5+
import kotlin.test.Test
6+
import kotlin.test.assertEquals
7+
import kotlin.test.assertNotNull
8+
import kotlin.test.assertNull
9+
import kotlin.test.assertSame
10+
import kotlin.test.assertTrue
11+
12+
// Tests for #880 / #1748 — direct coverage on Branch.matchRoute (the
13+
// route-picking helper used by Branch.session). The existing
14+
// BranchSessionTest exercises matchRoute at runtime, but PIT's coverage
15+
// tracker can't see through `scope.launch { ... }` continuations, so
16+
// every mutant in matchRoute reports as NO_COVERAGE. These tests call
17+
// matchRoute directly on a constructed Branch, which is unambiguous in
18+
// the bytecode PIT instruments.
19+
//
20+
// matchRoute precedence (mirrors invokeSuspend):
21+
// - null result → first NullRoute, else first ElseRoute, else null
22+
// - non-null → first TypeRoute whose klass.isInstance(result),
23+
// else first ElseRoute, else null
24+
class BranchMatchRouteTest {
25+
26+
sealed interface Animal
27+
data class Dog(val name: String) : Animal
28+
data class Cat(val name: String) : Animal
29+
data class Fish(val name: String) : Animal
30+
31+
open class Vehicle
32+
class Car : Vehicle()
33+
class Truck : Vehicle()
34+
class Bike : Vehicle()
35+
36+
private fun stringAgent() = agent<String, String>("a") {
37+
skills { skill<String, String>("s") { implementedBy { it } } }
38+
}
39+
40+
private fun handler(name: String): suspend (Any?) -> String = { "$name:$it" }
41+
42+
// ── non-null result branches ──────────────────────────────────────────────
43+
44+
@Test
45+
fun `matchRoute picks first TypeRoute whose klass isInstance result`() {
46+
// Kills NegateConditionalsMutator on L 103/104 (TypeRoute match check).
47+
val routes = listOf<BranchRoute<String>>(
48+
BranchRoute.TypeRoute(Dog::class, handler("dog")),
49+
BranchRoute.TypeRoute(Cat::class, handler("cat")),
50+
BranchRoute.TypeRoute(Fish::class, handler("fish")),
51+
)
52+
val branch = Branch<String, String>(source = stringAgent(), routes = routes)
53+
54+
val cat = Cat("luna")
55+
val matched = branch.matchRoute(cat)
56+
assertNotNull(matched, "Cat must match the TypeRoute(Cat)")
57+
assertSame(routes[1], matched, "must pick the Cat route (index 1), not Dog or Fish")
58+
}
59+
60+
@Test
61+
fun `matchRoute picks FIRST matching TypeRoute when subtype matches multiple klass instances`() {
62+
// Kills the "first matching wins" ordering — if iteration order were
63+
// reversed (NegateConditional inverting the for-loop continue), the
64+
// wrong route would fire. We register a permissive Vehicle route
65+
// BEFORE specific Car, then expect Vehicle (the registered-first
66+
// match) to win even for a Car instance.
67+
val vehicleRoute = BranchRoute.TypeRoute<String>(Vehicle::class, handler("vehicle"))
68+
val carRoute = BranchRoute.TypeRoute<String>(Car::class, handler("car"))
69+
val branch = Branch<String, String>(
70+
source = stringAgent(),
71+
routes = listOf(vehicleRoute, carRoute),
72+
)
73+
val matched = branch.matchRoute(Car())
74+
assertSame(vehicleRoute, matched,
75+
"first matching route wins; permissive Vehicle registered first must beat Car")
76+
}
77+
78+
@Test
79+
fun `matchRoute returns null when non-null result matches no TypeRoute and no ElseRoute`() {
80+
// Kills NullReturnValsMutator on L 108 (`return null` at end of matchRoute).
81+
// Without an ElseRoute, an unmatched non-null result must produce null —
82+
// caller will then surface its own error.
83+
val branch = Branch<String, String>(
84+
source = stringAgent(),
85+
routes = listOf(
86+
BranchRoute.TypeRoute(Dog::class, handler("dog")),
87+
BranchRoute.TypeRoute(Cat::class, handler("cat")),
88+
),
89+
)
90+
assertNull(branch.matchRoute(Fish("nemo")),
91+
"Fish doesn't match Dog/Cat and there's no ElseRoute → null")
92+
}
93+
94+
@Test
95+
fun `matchRoute falls through to ElseRoute when no TypeRoute matches non-null result`() {
96+
// Kills NegateConditionalsMutator on the `is BranchRoute.ElseRoute ->`
97+
// branch of the when (L 105). Order matters: ElseRoute placed LAST.
98+
val typeRoute = BranchRoute.TypeRoute<String>(Dog::class, handler("dog"))
99+
val elseRoute = BranchRoute.ElseRoute<String>(handler("else"))
100+
val branch = Branch<String, String>(
101+
source = stringAgent(),
102+
routes = listOf(typeRoute, elseRoute),
103+
)
104+
val matched = branch.matchRoute(Fish("nemo"))
105+
assertSame(elseRoute, matched, "no TypeRoute matched Fish → ElseRoute wins")
106+
}
107+
108+
@Test
109+
fun `matchRoute skips NullRoute when result is non-null even if registered first`() {
110+
// Kills mutants on the `is BranchRoute.NullRoute -> { /* skipped */ }`
111+
// when arm. A NullRoute must NOT match a non-null result.
112+
val nullRoute = BranchRoute.NullRoute<String>(handler("null"))
113+
val typeRoute = BranchRoute.TypeRoute<String>(Dog::class, handler("dog"))
114+
val branch = Branch<String, String>(
115+
source = stringAgent(),
116+
routes = listOf(nullRoute, typeRoute),
117+
)
118+
val matched = branch.matchRoute(Dog("rex"))
119+
assertSame(typeRoute, matched,
120+
"non-null Dog must skip NullRoute and land on the TypeRoute(Dog)")
121+
}
122+
123+
// ── null result branches ──────────────────────────────────────────────────
124+
125+
@Test
126+
fun `matchRoute picks first NullRoute when result is null`() {
127+
// Kills NegateConditionalsMutator on L 97 (`if (result == null)` boundary)
128+
// AND on L 98 (firstOrNull predicate for NullRoute).
129+
val nullRoute = BranchRoute.NullRoute<String>(handler("null"))
130+
val typeRoute = BranchRoute.TypeRoute<String>(Dog::class, handler("dog"))
131+
val elseRoute = BranchRoute.ElseRoute<String>(handler("else"))
132+
val branch = Branch<String, String>(
133+
source = stringAgent(),
134+
routes = listOf(typeRoute, nullRoute, elseRoute),
135+
)
136+
val matched = branch.matchRoute(null)
137+
assertSame(nullRoute, matched,
138+
"null result prefers NullRoute over ElseRoute even when registered later")
139+
}
140+
141+
@Test
142+
fun `matchRoute falls back to ElseRoute when result is null and no NullRoute declared`() {
143+
// Kills the `?: routes.firstOrNull { it is BranchRoute.ElseRoute }`
144+
// Elvis fallback for null results.
145+
val typeRoute = BranchRoute.TypeRoute<String>(Dog::class, handler("dog"))
146+
val elseRoute = BranchRoute.ElseRoute<String>(handler("else"))
147+
val branch = Branch<String, String>(
148+
source = stringAgent(),
149+
routes = listOf(typeRoute, elseRoute),
150+
)
151+
val matched = branch.matchRoute(null)
152+
assertSame(elseRoute, matched,
153+
"null result with no NullRoute → ElseRoute fallback fires")
154+
}
155+
156+
@Test
157+
fun `matchRoute returns null when result is null and neither NullRoute nor ElseRoute declared`() {
158+
// Kills NullReturnValsMutator on L 98 (the ?: chain's final null).
159+
// Without any NullRoute or ElseRoute, a null result has nowhere to go —
160+
// matchRoute must return null (caller surfaces the error).
161+
val branch = Branch<String, String>(
162+
source = stringAgent(),
163+
routes = listOf(
164+
BranchRoute.TypeRoute(Dog::class, handler("dog")),
165+
BranchRoute.TypeRoute(Cat::class, handler("cat")),
166+
),
167+
)
168+
assertNull(branch.matchRoute(null),
169+
"null result with no NullRoute or ElseRoute → null (no error here, caller decides)")
170+
}
171+
172+
@Test
173+
fun `matchRoute on empty routes list returns null for both null and non-null inputs`() {
174+
// Kills mutants on the for-loop entry + the early routes.firstOrNull
175+
// returning null on an empty list.
176+
val branch = Branch<String, String>(source = stringAgent(), routes = emptyList())
177+
assertNull(branch.matchRoute(null), "empty routes + null → null")
178+
assertNull(branch.matchRoute(Dog("x")), "empty routes + non-null → null")
179+
}
180+
181+
// Note: invokeSuspend's null-result branches (L 73-80 in Branch.kt) are
182+
// defensive dead-code at the type level — `agent<IN, OUT : Any>` rejects
183+
// a null OUT, so no legal source agent can produce null. PIT correctly
184+
// reports those lines as NO_COVERAGE; per the convention in
185+
// BranchSuspendTest, we don't write tests for unreachable defensive code.
186+
}

0 commit comments

Comments
 (0)