Fix dynamicConfigOverrides defaults getting tied to test's lifecycle#9918
Merged
long-nt-tran merged 1 commit intotemporalio:mainfrom Apr 13, 2026
Merged
Conversation
2eb0117 to
cd21545
Compare
stephanos
reviewed
Apr 13, 2026
cd21545 to
c02f83d
Compare
stephanos
approved these changes
Apr 13, 2026
Contributor
stephanos
left a comment
There was a problem hiding this comment.
Awesome - thank you for tracking that down!
5 tasks
long-nt-tran
added a commit
that referenced
this pull request
Apr 28, 2026
## What changed? When we set Nexus callback URL in test_env.go, the dynamic config override is still tied to the test's lifetime, not the cluster's lifetime, so a subsequent test that reuse this cluster will not have that override. Moving the override to onebox.go (similar pattern to #9918) so this default lives for the lifetime of the cluster. ## Why? Ran into issue with task token not set in #9614, this solves it. Breaking the fix in a separate PR for ease of review + checking this in first. ## How did you test it? - [ ] built - [ ] run locally and tested manually - [x] covered by existing tests - [ ] added new unit test(s) - [ ] added new functional test(s)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed?
Changed onebox.go to set global
dynamicConfigOverrideswithout binding cleanup to a test, and added unit test to verify behavior works -- reused test gets the same defaultdynamicConfigOverrides.Also renamed the existing function
overrideDynamicConfig->overrideTestLevelDynamicConfigto make it clearer that that function's dynamic config override is bounded to the test's lifecycle.Why?
When onebox.go sets up a new Temporal cluster, it applies some default overrides that is not set by the test.
However, these are set via
overrideDynamicConfig(...), which ties the cleanup of these configs to a specific test's lifecycle.As a result, if we have Test1 that gets a fresh cluster, if some Test2 later reuses this cluster after Test1 ends, Test2 will not have these default
dynamicConfigOverrides-- in fact the dynamicConfig would be empty because we cleaned it up after Test1 finished.These defaults should not be bounded to the lifetime of the test that instantiated the fresh cluster.
Note
Existing tests that instantiate their
envwithtestcore.WithDynamicConfigOverrides(...)options already get a fresh cluster, so this bug impacted tests that don't override any dynamic config options when callingtestcore.NewEnv(...).How did you test it?
I added a
test_cluster_pool_test.gowhich deterministically repros this without the code change inonebox.go:Expected failure without change in onebox.go:
With change, it passes:
Potential risks
Might be breaking changes if some tests rely on the existing bug for some reason (?)