weiqingy opened a new pull request, #659:
URL: https://github.com/apache/flink-agents/pull/659
Linked issue: #646
### Purpose of change
Closes the heavier test coverage explicitly deferred at PR #546's review for
the manager classes that decomposed `ActionExecutionOperator`. PR #546 added 4
contract test files (7 tests, 295 lines) using mocks and stubs; heavier
coverage — snapshot/recovery cycle, listener notification, watermark draining
order, memory-context lifecycle — was deferred and tracked in #646. This PR
closes that gap.
**Scope** (matches issue #646's deferred-coverage table):
| Manager | New coverage |
|---------|--------------|
| `DurableExecutionManager` | Full snapshot + recovery cycle;
`notifyCheckpointComplete` pruning behaviour (3 new tests) |
| `EventRouter` | Listener notification on event processed; watermark
draining order under concurrent keys (5 new tests) |
| `ActionTaskContextManager` | Full memory-context lifecycle: create →
transfer → complete → cleanup (5 new tests) |
`OperatorStateManager` is dropped from this PR — its observable contracts
are already covered by `ActionExecutionOperatorTest.java:176–231` (rescale +
recovery) and `:254–306` (durable state recovery). Adding a manager-scoped
harness test would duplicate without adding coverage.
`PythonBridgeManager` is out of scope per the issue body's deferred-coverage
table.
### Commit structure (4 commits)
1. **[runtime] Add @VisibleForTesting seams for manager tests** — three
narrow seams in production code, all following the existing
`DurableExecutionManager.getActionStateStore()` precedent:
- `DurableExecutionManager.getCheckpointIdToSeqNums()` accessor
- `EventRouter.addEventListener(EventListener)` accessor
- `EventRouter` 3-arg `@VisibleForTesting` constructor for `EventLogger`
injection (2-arg constructor delegates; production signature unchanged)
2. **[runtime] Add comprehensive tests for DurableExecutionManager** — 3 new
tests
3. **[runtime] Add comprehensive tests for EventRouter** — 5 new tests
4. **[runtime] Add comprehensive tests for ActionTaskContextManager** — 5
new tests
### Tests
- **13 new tests** added; 6 existing manager tests untouched. After this PR:
5 + 7 + 7 + 1 = **20 tests** across the four manager test classes (was 7).
- Full `runtime` module: **262 tests, 0 failures, 0 errors, 0 skipped**.
- Spotless format check: `### FORMAT CHECK PASSED ###`.
- License check: clean (no tracked files missing headers).
Verification commands:
```
mvn -pl runtime -am test -Dsurefire.failIfNoSpecifiedTests=false
./tools/lint.sh -c
```
Final diff: **5 files, +464 / -2** (2 production-code files for the seams; 3
test files for the new tests).
#### Test design notes (each test catches a distinct mutation)
-
`DurableExecutionManagerTest.notifyPrunesNotifiedCheckpointAndPreservesOthers`
is a single consolidated test that exercises BOTH the multi-key loop body (two
keys under one checkpoint, both must be pruned) AND multi-checkpoint isolation
(a third key under a different checkpoint must survive). Strictly stronger than
two separate tests.
- `DurableExecutionManagerTest.handleRecoveryCallsRebuildState` verifies the
*call* contract; mutation observability for the `rebuildState` side effect is
covered by operator-level integration tests against real stores
(`InMemoryActionStateStore.rebuildState` is a no-op).
-
`ActionTaskContextManagerTest.transferContextsCopiesMemoryAndContinuationToNewTask`
covers the Java path; the Python path requires Pemja and is covered by
`ActionExecutionOperatorTest`'s Python flows.
- `EventRouterTest.processEligibleWatermarksDrainsInArrivalOrder` drives
`SegmentedQueue` across two segments and removes keys in completion order that
differs from arrival; ordered assertion `containsExactly(WM1, WM2)` catches
both wrong-order draining and skipped watermarks.
### API
No public API changes. Three new `@VisibleForTesting` package-private
members in production code (one accessor on `DurableExecutionManager`, one
accessor + one constructor overload on `EventRouter`). The existing
`EventRouter(AgentPlan, boolean)` constructor signature is unchanged; it
delegates to the new 3-arg form. `createEventLogger` is converted from instance
to `static` (required by Java for chained `this(...)` constructors; the
original body uses only its parameter).
### Documentation
- [ ] `doc-needed`
- [x] `doc-not-needed`
- [ ] `doc-included`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
