-
Notifications
You must be signed in to change notification settings - Fork 42
More tests #51
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
More tests #51
Conversation
| _, err2 := handle.GetResult() | ||
| if err2 == nil { | ||
| t.Fatal("Second call to GetResult should return an error") | ||
| return nil, fmt.Errorf("Second call to GetResult should return an error") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
t.Fatal terminates the goroutine, but this is an inner goroutine ran by the test goroutine itself. The resulting error is quite hard to parse and the test goroutine will move on to its cleanup actions, potentially deadlocking w/o much information. So, as a rule, we should only call t.Fatal in the "top level" test goroutine.
| } | ||
|
|
||
| var typeErasedStepNameToStepName = make(map[string]string) | ||
| var typeErasedStepNameToStepName sync.Map |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This map should be read-mostly. The first invocation of a step will store the reflection name.
|
|
||
| // Register adds a workflow function to the registry (thread-safe, only once per name) | ||
| func registerWorkflow(ctx DBOSContext, workflowName string, fn WrappedWorkflowFunc, maxRetries int) { | ||
| func registerWorkflow(ctx DBOSContext, workflowFQN string, fn WrappedWorkflowFunc, maxRetries int, customName string) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's going on here? What's the difference between FQN and customName?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And the tests seem to use neither?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The FQN is the reflection name, which we get with runtime.FuncForPC(reflect.ValueOf(fn).Pointer()).Name().
The FQN is the only information we get at runtime when a user calls RunAsWorkflow.
A user can set a custom name when registering a workflow. There are now tests exercising this path.
The custom name is stored in the registry, alongside other registration-time parameters like maxRetries.
RunAsWorkflow resolve the workflow name at runtime when looking up registration options (from the registry.)
This PR adds
Custom name registration: we don't have custom interfaces / struct for workflows and at runtime have only access to the reflection-found workflow full qualified name. So we leverage the existing mechanism to retrieve registration-time workflow parameters (the registry is indexed by reflection names). This should be consistent within application versions, but remember that reflection names can change if the code changes.
Added Tests:
Send,Receive,SetEvent,GetEvent