Skip to content
This repository has been archived by the owner on Apr 3, 2024. It is now read-only.

Added methods to create new test worker with options and custom clients #81

Merged
merged 14 commits into from
Jul 27, 2022

Conversation

grantfuhr
Copy link
Contributor

@grantfuhr grantfuhr commented Jun 6, 2022

Signed-off-by: grantfuhr grant.fuhr@datadoghq.com

What changed?
Added new methods for creating test workers with default options, custom options and custom clients.

Why?
I would like to test interceptors with the temporalite testing framework.

How did you test it?
I added a unit tests.

Potential risks
Minimal risk. Only the new methods could break.

Is hotfix candidate?
No

grantfuhr added 2 commits June 6, 2022 14:23
Signed-off-by: grantfuhr <grant.fuhr@datadoghq.com>
Signed-off-by: grantfuhr <grant.fuhr@datadoghq.com>
@codecov-commenter
Copy link

codecov-commenter commented Jun 6, 2022

Codecov Report

Merging #81 (8405c53) into main (95467df) will increase coverage by 3.09%.
The diff coverage is 87.75%.

@@            Coverage Diff             @@
##             main      #81      +/-   ##
==========================================
+ Coverage   37.71%   40.81%   +3.09%     
==========================================
  Files          12       13       +1     
  Lines         814      860      +46     
==========================================
+ Hits          307      351      +44     
  Misses        485      485              
- Partials       22       24       +2     
Impacted Files Coverage Δ
temporaltest/server.go 78.88% <78.57%> (-0.86%) ⬇️
internal/examples/helloworld/testinterceptor.go 88.88% <88.88%> (ø)
internal/examples/helloworld/helloworld.go 82.35% <100.00%> (+3.78%) ⬆️
temporaltest/options.go 82.60% <100.00%> (+27.05%) ⬆️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

grantfuhr added 3 commits June 6, 2022 15:50
Signed-off-by: grantfuhr <grant.fuhr@datadoghq.com>
Signed-off-by: grantfuhr <grant.fuhr@datadoghq.com>
Signed-off-by: grantfuhr <grant.fuhr@datadoghq.com>
@grantfuhr grantfuhr changed the title Added method to create new test worker with options Added methods to create new test worker with options and custom clients Jun 6, 2022
grantfuhr and others added 2 commits June 6, 2022 16:06
Signed-off-by: grantfuhr <grant.fuhr@datadoghq.com>
@@ -39,10 +40,43 @@ func (ts *TestServer) fatal(err error) {
}

// Worker registers and starts a Temporal worker on the specified task queue.
// WorkflowPanicPolicy is set to worker.FailWorkflow
func (ts *TestServer) Worker(taskQueue string, registerFunc func(registry worker.Registry)) worker.Worker {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm just thinking out loud, we should probably deprecate this and rename to NewWorker before the first release. That would be more consistent with NewClientWithOptions and NewWorkerWithOptions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can work on a followup PR that changes this. Should it be a breaking change, or should it support both methods?

Copy link
Collaborator

Choose a reason for hiding this comment

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

We can just mark as deprecated for now (no breaking change), and then remove it before the first tagged release.

temporaltest/options.go Show resolved Hide resolved
temporaltest/options.go Show resolved Hide resolved
w := worker.New(ts.Client(), taskQueue, worker.Options{
WorkflowPanicPolicy: worker.FailWorkflow,
})
ts.defaultWorkerOptions.WorkflowPanicPolicy = worker.FailWorkflow
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: calling this function probably shouldn't have a side effect of modifying TestServer fields. Instead we should set any defaults necessary during initialization.

temporaltest/server.go Outdated Show resolved Hide resolved
temporaltest/server.go Outdated Show resolved Hide resolved
temporaltest/server.go Outdated Show resolved Hide resolved
grantfuhr and others added 3 commits July 22, 2022 16:45
Co-authored-by: Jacob LeGrone <jlegrone@users.noreply.github.com>
Signed-off-by: grantfuhr <grant.fuhr@datadoghq.com>
Signed-off-by: grantfuhr <grant.fuhr@datadoghq.com>
temporaltest/server.go Outdated Show resolved Hide resolved
@jlegrone jlegrone enabled auto-merge (squash) July 27, 2022 14:48
@jlegrone jlegrone merged commit 593da1a into temporalio:main Jul 27, 2022
@grantfuhr grantfuhr deleted the new-worker-with-options branch July 27, 2022 15:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants