Skip to content

Add unit tests for the checkpoint and ECM promise handlers #7117

Description

@aglinxinyuan

Task Summary

The checkpoint and embedded-control-message promise handlers are the lowest-coverage cluster in the engine:

Handler Missed Coverage
TakeGlobalCheckpointHandler 21 4.5%
FinalizeCheckpointHandler 21 4.5%
PrepareCheckpointHandler 20 4.8%
EmbeddedControlMessageHandler 11 8.3%
EvaluatePythonExpressionHandler 11 8.3%

They are unit-testable. RetrieveWorkflowStateHandlerSpec, PortCompletedHandlerSpec and JumpToOperatorRegionHandlerSpec already establish the harness: build a real CoordinatorProcessor plus a second handler initializer and capture what the output handler emits, so the real handler method runs. ram:// in-memory VFS storage (used by LoggingSpec and DPThreadSpec) covers the storage-URI plumbing without touching disk.

Worth pinning: the single-physical-operator guard, the already-taken short-circuit that downgrades a checkpoint to an estimate, the per-checkpoint subfolder that WorkflowActor.setupReplay later resolves, the estimation branches on both worker handlers, the CheckpointSupport hand-off and its skip branch, and the fan-in behaviour that must not answer before every targeted worker has replied.

Two parts are genuinely out of unit scope and should be left alone: the coordinator serializing its own CP state (needs a live transferService), and PrepareCheckpointHandler's main-thread closure that drains the input queue and starts input recording (needs a running DP loop and worker main thread).

Separately, CheckpointSpec contains two commented-out test blocks that are dead beyond repair and should be removed as part of this work:

  • The first matches on OpExecInitInfoWithCode / OpExecInitInfoWithFunc. Those names appear nowhere else in the repo — OpExecInitInfo is now a ScalaPB sealed oneof of OpExecWithClassName / OpExecWithCode. It also casts to CheckpointSupport, which no production operator implements, contains zero assertions, and has one branch that is literally ???. Its intent is already covered by SerializationManagerSpec and CheckpointSubsystemSpec.
  • The second calls AmberClient with six constructor arguments where the current signature takes five, and its body starts a real two-operator workflow with 30s awaits — integration scope if anywhere.

Task Type

  • Refactor / Cleanup
  • DevOps / Deployment / CI
  • Testing / QA
  • Documentation
  • Performance
  • Other

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions