Skip to content

Lazy SdkClient and SdkWorker#9745

Merged
stephanos merged 2 commits intomainfrom
stephanos/test-env-unsused-worker
Mar 31, 2026
Merged

Lazy SdkClient and SdkWorker#9745
stephanos merged 2 commits intomainfrom
stephanos/test-env-unsused-worker

Conversation

@stephanos
Copy link
Copy Markdown
Contributor

@stephanos stephanos commented Mar 30, 2026

What changed?

Deprecate WithSdkWorker testEnv option and init it lazily instead.

Why?

Requires less manual setup.

How did you test it?

  • built
  • run locally and tested manually
  • covered by existing tests
  • added new unit test(s)
  • added new functional test(s)

@stephanos stephanos force-pushed the stephanos/test-env-unsused-worker branch from 3f2d38e to 9c1447c Compare March 30, 2026 20:12
// WithSdkWorker sets up an SDK client and worker for the test.
// Cleanup is handled automatically via t.Cleanup().
// Deprecated: this option is no longer required and will be removed once all callers have been updated.
func WithSdkWorker() TestOption {
Copy link
Copy Markdown
Contributor Author

@stephanos stephanos Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not deleting it here to keep PR simple; can be follow-up PR. But also happy to include that here as well.

@stephanos stephanos force-pushed the stephanos/test-env-unsused-worker branch from 9c1447c to 350f6c9 Compare March 30, 2026 20:13
Comment on lines +74 to +77
sdkClientOnce sync.Once
sdkClient sdkclient.Client
sdkWorkerOnce sync.Once
sdkWorker sdkworker.Worker
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made both client and worker init separately since but I wasn't sure if there's a use case; I suspect there's maybe a case where you want the client but don't need the worker (e.g. to test server validation?). So I think it's worth keeping since it doesn't cost much complexity.

@stephanos stephanos marked this pull request as ready for review March 30, 2026 20:27
@stephanos stephanos requested review from a team as code owners March 30, 2026 20:27
t: t,
tv: testvars.New(t),
ctx: setupTestTimeoutWithContext(t, options.timeout),
sdkWorkerTQ: RandomizeStr("tq-" + t.Name()),
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be eager; very low overhead.

@stephanos stephanos force-pushed the stephanos/test-env-unsused-worker branch from 350f6c9 to 892b484 Compare March 30, 2026 22:33
e.t.Fatalf("Failed to start SDK worker: %v", err)
}
e.t.Cleanup(func() { e.sdkWorker.Stop() })
})
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^ same code as before

e.t.Fatalf("Failed to create SDK client: %v", err)
}
e.t.Cleanup(func() { e.sdkClient.Close() })
})
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^ same code as before

@stephanos stephanos enabled auto-merge (squash) March 31, 2026 15:57
@stephanos stephanos merged commit 5920e17 into main Mar 31, 2026
68 of 70 checks passed
@stephanos stephanos deleted the stephanos/test-env-unsused-worker branch March 31, 2026 16:02
chaptersix added a commit to chaptersix/temporal that referenced this pull request Apr 2, 2026
## What changed?

Deprecate `WithSdkWorker` testEnv option and init it lazily instead.

## Why?

Requires less manual setup.

## 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)

Co-authored-by: Alex Stanfield <13949480+chaptersix@users.noreply.github.com>
chaptersix added a commit to chaptersix/temporal that referenced this pull request Apr 2, 2026
## What changed?

Deprecate `WithSdkWorker` testEnv option and init it lazily instead.

## Why?

Requires less manual setup.

## 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)

Co-authored-by: Alex Stanfield <13949480+chaptersix@users.noreply.github.com>
chaptersix added a commit that referenced this pull request Apr 2, 2026
## What changed?

Deprecate `WithSdkWorker` testEnv option and init it lazily instead.

## Why?

Requires less manual setup.

## 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)

Co-authored-by: Alex Stanfield <13949480+chaptersix@users.noreply.github.com>
chaptersix added a commit to chaptersix/temporal that referenced this pull request Apr 2, 2026
## What changed?

Deprecate `WithSdkWorker` testEnv option and init it lazily instead.

## Why?

Requires less manual setup.

## 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)

Co-authored-by: Alex Stanfield <13949480+chaptersix@users.noreply.github.com>
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.

2 participants