Add ChildWorkflowOptions support to WorkflowImplementationOptions (#2)#2887
Add ChildWorkflowOptions support to WorkflowImplementationOptions (#2)#2887porunov wants to merge 1 commit into
Conversation
|
|
…plementationOptions Fixes temporalio#2790 - Add childWorkflowOptions map and defaultChildWorkflowOptions fields - Add setChildWorkflowOptions() and setDefaultChildWorkflowOptions() builder methods - Add getChildWorkflowOptions() and getDefaultChildWorkflowOptions() getters - Update SyncWorkflowContext to store and expose child workflow options - Update WorkflowInternal.newChildWorkflowStub() to merge predefined options - Add mergeChildWorkflowOptions() method to ChildWorkflowOptions.Builder - Add integration and unit tests --------- Signed-off-by: Oleksandr Porunov <alexandr.porunov@gmail.com>
bd8e2d9 to
3295d53
Compare
maciejdudko
left a comment
There was a problem hiding this comment.
Hi @porunov, thank you for your contribution. The code looks good, but the tests need to be improved before we can merge this - see comment.
| @Test | ||
| public void testDefaultChildWorkflowOptionsApplied() { |
There was a problem hiding this comment.
This test does not actually assert that the right options were used. Please rewrite it so that the child workflow only succeeds if the default options were correctly applied. I think checking its memo would be most straight forward.
Please also add a test that verifies default, not per-type child workflow options work too.
Fixes #2790
Tests are added to:
ChildWorkflowOptionsInWorkflowImplementationOptionsTest.javaPotential documentation place: https://docs.temporal.io/develop/java/workflows/child-workflows (it could be something similar as activity options documentation).
Disclaimer: Code was Generated by Opus 4.6 in Copilot. Reviewed manually.