search

uber-go / cadence-client

Framework for authoring workflows and activities running on top of the Cadence orchestration engine.
https://cadenceworkflow.io
MIT License
339 stars 128 forks source link

add mocks generation + add worker mocks #1239

Open sapk opened 1 year ago

sapk commented 1 year ago

This PR add mock generation with latest version and add worker.Registry and worker.Worker mocks.

I want to be able to unit test registration of my worker so I need at least the registry mock. If I generate on my side it link to internal package which is forbidden. I can remediate this limitation on my side with writing a custom mock but I feel it maybe useful to other to have a clean generate one in this repository along side the others provided mocks.

I validated that I can use this in my repository by using a replace in the go.mod since we can link to internal in the same repository.

Risk should be low as it only update or add new mocks.

Some part of this PR are probably opinionated (ex go:generate comments) so let me know if it need any change.

CLAassistant commented 1 year ago

CLA assistant check
All committers have signed the CLA.

Groxx commented 1 year ago

tbh I'm planning on deleting these mocks entirely in a future version. mocks can be generated by users with the library of their choice, rather than being stuck with our code / tool versions / etc.

I haven't dug into them super closely, but AFAICT they aren't relying on any internal/ visibility rules, so they should be fully possible to build externally. or are there in fact problems with that?

sapk commented 1 year ago

Yes the main reason for the need for internal is that mockery translate the aliased options struct to internal ones. Like in the generated one (ex:worker) in this MR.

It can be work around by doing a manual version of the mock to not rely on the aliased internal options.

Maybe the real solution is to fix mockery to not uncover the alias.

Otherwise, I totally understand if you want to remove them. It was just to ease if someone else had the same need.

Groxx commented 1 year ago

Yeah, we've been having that issue with mockery and gomock internally too.

Gomock at least has a "source" mode in the flags (as opposed to using reflection), which seems to work better.

Otherwise, I'm hoping that we'll have a v2 of this client library soon (a literal v2, so you can use both and migrate gradually), and that'll definitely be getting rid of those internal aliases. There's just no way to do so without making other breaking changes, so it has to wait for v2.

Groxx commented 1 year ago

but in the meantime! if you're interested, I think this could work. I want to pin versions though, and bake them into the makefile - it's not too hard.

do you want to attempt that, or should I just prep a commit/? for you? I'm not entirely sure how to do that within github, but it's easy enough to just prep something to copy/paste.

Groxx commented 1 year ago

eh, that took more fiddling than expected. https://github.com/Groxx/cadence-client/commit/ff949d2dab21386c7c38944446b1e6188b1472ef <- if you want to add those changes, we can merge this :)

(there may be more to merge in and re-tidy soon, sorry about that if so)