Skip to content

fix: configure slow branch test mock#505

Merged
zhongkechen merged 2 commits into
mainfrom
codex/fix-slow-branch-test-mock
Jul 3, 2026
Merged

fix: configure slow branch test mock#505
zhongkechen merged 2 commits into
mainfrom
codex/fix-slow-branch-test-mock

Conversation

@zhongkechen

@zhongkechen zhongkechen commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Configure test_executor_returns_before_slow_branch_completes to use the real user-function wrapper behavior.
  • Relax the map_with_min_successful example assertion to allow the operation tree to observe an already-running child that completes after the handler's BatchResult snapshot.

Root Cause Analysis

Finding 1: Unit test mock bypassed branch execution

test_executor_returns_before_slow_branch_completes creates a mocked ExecutionState but did not configure wrap_user_function. Concurrent branch execution goes through child_handler, which calls state.wrap_user_function(...) and then invokes the returned wrapper. With an unconfigured Mock, that call returns another Mock instead of the original function wrapper, so the fast and slow branch bodies can be bypassed. In that state the slow branch may not execute its sleep path, making the test timing dependent and allowing both branches to appear completed.

The fix configures execution_state.wrap_user_function to return the original function, matching the real ExecutionState behavior used by the neighboring tests.

Finding 2: Example test assumed two snapshots stay identical

map_with_min_successful returns once min_successful is reached. The returned BatchResult is a snapshot taken by the handler at that point. The operation tree inspected later by the test can legitimately be slightly newer because already-running child operations may still finish and checkpoint after the BatchResult snapshot is built. That is why the handler result can report 6 successes while map_op.child_operations later shows 7 succeeded children.

This behavior is acceptable for min_successful concurrency. The test now keeps the important checks strict: no failed child operations, bounded successful child count, and STARTED count consistent with the operation-tree view.

Refs #405

Verification

  • hatch run test:all packages/aws-durable-execution-sdk-python-examples/test/map/test_map_with_min_successful.py::test_map_with_min_successful packages/aws-durable-execution-sdk-python/tests/concurrency_test.py::test_executor_returns_before_slow_branch_completes

@zhongkechen zhongkechen merged commit 5195ec1 into main Jul 3, 2026
72 checks passed
@zhongkechen zhongkechen deleted the codex/fix-slow-branch-test-mock branch July 3, 2026 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants