-
Notifications
You must be signed in to change notification settings - Fork 41
test dbos mocks #115
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
test dbos mocks #115
Conversation
go.mod
Outdated
| gotest.tools/v3 v3.5.2 // indirect | ||
| ) | ||
|
|
||
| replace github.com/dbos-inc/dbos-transact-golang/dbos/mocks => ./mocks |
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 only importers of this module are our tests and they should always get the mocks from the development directory.
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 does this do? It looks sketchy.
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.
I see, the mocks files are being committed as part of our core codebase--do we really need to do that?
| return *new(R), err | ||
| } | ||
|
|
||
| // Wrapper handle -- useful for handling mocks in RunWorkflow |
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 does this do? Why do we need it? Do users need to worry about it?
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 situation we have with RunWorkflow, is that only the interface methods are mockable. If you want to mock RunWorkflow and have it return a mock handle, only the "inner" call with get a mock. Then, in the package level method, we use information from that handle to return a new typed handle. This new handle, not being the mock, makes testing impossible.
What this does -- users don't need to worry -- is to fall back on a "proxy" handler if the interface RunWorkflow does not return the "normal" handlers (direct or polling). The proxy just calls in whatever was returned by RunWorkflow. This allow a mock handle to be passed through.
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.
Can't we just return a polling handle (passing in the mocked context) instead of this new thing?
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.
Returning a new polling handle would be returning a new object that is not the mock returned by DBOSContext.RunWorkflow. We need a way to pass through the mock while respecting the function signature (return a generic WorkflowHandle[R] interface). Unfortunately we cannot just "cast" the mock (SomeMockType[any]) to WorkflowHandle[R]: any and R are different type. This proxy type:
- Let us pass through the mock
- Let us returned a typed mock
RunWorkflow(it does return a new handle from theDBOSContext.RunWorkflow). This will only be used on the mocking path.