Add ContinueAsNew search attribute tests for test server#2870
Add ContinueAsNew search attribute tests for test server#2870baekgyu-kim wants to merge 1 commit into
Conversation
maciejdudko
left a comment
There was a problem hiding this comment.
Hi @baekgyu-kim, thank you for your contribution. Unfortunately there was a mistake in the referenced issue - the real server only preserves Search Attributes on Continue-as-New; Memos are cleared. Which means the PR in its current form does not match the real server behavior. Please remove the changes to Memo handling and only keep the changes to Search Attributes handling. Make sure all tests pass both when run on test server and on real server. You can make the tests use a local dev server by setting environment variables USE_DOCKER_SERVICE=true and USE_EXTERNAL_SERVICE=true. You can start a local dev server by using Temporal CLI: temporal server start-dev --namespace UnitTest --search-attribute CustomKeywordField=Keyword
|
Hello @maciejdudko , I checked the server code and confirmed the mismatch. The correct search-attribute propagation already existed on master. I verified that the ContinueAsNewTest cases pass on both the in-process test server and a real dev server ( Whenever you have a moment, I would appreciate it if you could take another look. |
|
Apologies again. After looking more into both test server and real server implementation, it looks like the server doesn't preserve Search Attributes across CaN either. We have client-side logic in the SDK to automatically transfer SA when none are set in the request (SyncWorkflowContext.java), and this gives the illusion it's the server doing this work. The bottom line is - the test server preserves neither Memos nor Search Attributes, this behavior matches the real server, the tests in this PR exercise SDK behavior rather than test server behavior, and issue #2655 was invalid to begin with. I'm sorry for wasting your time on this. Still, I really appreciate your contribution and you're welcome to look at other open issues and submit more PRs. |
What was changed
Added regression test coverage for search-attribute behavior on ContinueAsNew in the in-memory test server.
No production code is changed.
The correct behavior already exists on master: on ContinueAsNew the server propagates the command's search attributes to the new run, and the SDK carries the previous run's search attributes into the command when none are specified.
This PR adds the tests that were missing to lock that behavior in.
Note: an earlier version of this PR also made the test server inherit memo and search attributes from the previous run.
That does not match the real Temporal service, which builds the new run's StartWorkflowExecutionRequest directly from the command (
Memo: command.Memo,SearchAttributes: command.SearchAttributes) and inherits neither.Search attributes only survive because the SDK carries them into the command, while memos are not carried over and are effectively cleared. Those production changes were removed.
Why?
To verify that ContinueAsNew carries search attributes in the test server, matching the real Temporal service, and to guard against regressions. Memo is intentionally not preserved, consistent with the real service.
Checklist
Closes Testing server continue-as-new doesn't carry search attributes #2655.
How was this tested:
No. This is test-only coverage for existing test server behavior; no public API or behavior change.