test(schema-registry): pin CachedSchemaResolver failure-caching contract (§19)#193
Merged
Merged
Conversation
Adds three tests that nail down the failure-propagation path under contention — the criticality-8 coverage gap in the prior audit. The §19 "cache forever in-process" invariant is correct ONLY for successful lookups. If a future regression ever wrote a sentinel into the ConcurrentHashMap before the delegate exception unwound (e.g. a "negative cache" optimization), a transient SR outage would pin a topic into a permanent decode-error state. These tests prove that doesn't happen today and will catch it if it ever does. - failedLookupIsNotCachedAndIsRetriedOnNextCall: delegate throws once, then succeeds. Asserts the exception propagates, cache stays empty, and the retry actually re-invokes the delegate. - concurrentMissOnSameIdCollapsesToOneDelegateCall: uses a CountDownLatch to guarantee the second virtual thread enters computeIfAbsent while the first holds the bucket. Asserts exactly one delegate call. Pins the §19 "concurrent misses collapse to one HTTP call" guarantee. - concurrentFailureOnSameIdDoesNotPoisonCache: 16 virtual threads race a failing delegate, then one recovery call with a successful delegate. Asserts every racing thread observes the failure, cache stays empty through the storm, and the recovery call succeeds and caches normally.
Owner
Author
|
@copilot please review |
Copilot stopped work on behalf of
eschizoid due to an error
June 18, 2026 19:29
Per user feedback, code comments must state the rule directly rather than cite section numbers from architecture docs (the section numbers drift; the load-bearing context survives). Replaces the two §19 references in CachedSchemaResolverTest with direct wording: "Confluent SR schema IDs are immutable" and "computeIfAbsent must collapse concurrent misses".
Owner
Author
|
@copilot please review |
Copilot stopped work on behalf of
eschizoid due to an error
June 18, 2026 19:37
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #193 +/- ##
============================================
- Coverage 77.64% 77.60% -0.04%
+ Complexity 732 731 -1
============================================
Files 68 68
Lines 2800 2800
Branches 346 346
============================================
- Hits 2174 2173 -1
Misses 465 465
- Partials 161 162 +1 ☔ View full report in Codecov by Harness. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the criticality-8 coverage gap in
CachedSchemaResolverflaggedby the prior coverage audit: the failure-propagation path under
contention. The §19 "cache forever in-process" invariant in
CLAUDE.mdonly holds for successful lookups — a failed delegatecall must NOT poison the cache with a sentinel, or a transient SR
outage would pin a topic into a permanent decode-error state.
No production change. The current implementation relies on the
ConcurrentHashMap.computeIfAbsentcontract (mapping-function throws →no entry stored, exception propagates) and these tests now lock that in.
Tests
failedLookupIsNotCachedAndIsRetriedOnNextCall— delegate throwson the first call, succeeds on the second. Asserts the exception
propagates verbatim,
cache.size()stays 0 after the failure, and theretry actually re-invokes the delegate (proving no sentinel). Catches:
a regression where
computeIfAbsentwas wrapped in a try/catch thatsilently stored an empty-string sentinel — the second call would
either also throw or return
""without hitting the delegate.concurrentMissOnSameIdCollapsesToOneDelegateCall— two virtualthreads race a miss on the same id; the first is parked inside the
delegate via a
CountDownLatchso the second is guaranteed to entercomputeIfAbsentwhile the bucket is held. Asserts exactly onedelegate call and both threads see the same schema string. Catches: a
regression to a check-then-act
get/putshape that would let boththreads issue the HTTP call.
concurrentFailureOnSameIdDoesNotPoisonCache— 16 virtual threadsrace a failing delegate, then one recovery call with a successful
delegate. Asserts every racing thread observes the failure,
size()is 0 through the storm, the recovery call succeeds and now caches.
Catches: a regression that races a cache-write before the delegate
throw clears state (e.g. a misplaced
cache.put(id, ...)outside thecomputeIfAbsentmapping function).§19 wire-up
Per the §19 invariants in
.claude/CLAUDE.md:Test #2 is the direct executable proof of that line. Tests #1 and #3
are the implicit corollary: the "cache forever" claim is conditional on
"...after a successful resolve" — anything else (failure storms, partial
state) must leave the cache empty.
Test plan
./gradlew :lib:kpipe-schema-registry-confluent:test— 11 tests pass (8 prior + 3 new), 0 failures.Thread.ofVirtual()+CountDownLatchper project test doctrine.CachedSchemaResolveralready satisfies the contract.