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

Fixing unit_test failure detection, and tests for data converters #1341

Closed Groxx closed 3 weeks ago

Groxx commented 3 weeks ago

Apparently these also failed on the original PR, but due to missing a \ in #1303 it wasn't failing in CI because the env var was gone by the time exit ran. I should really take that as a hint to finally rewrite that chunk of the makefile, so it just does a normal go test ./... :\ but not today.

Thankfully this seems to be the only set of tests that snuck past, and they were just incorrect. I'm not sure why I apparently didn't run them or something while writing them in #1331 but it seems pretty clear I didn't since the old pre-merge SHA fails too. Bleh.


To correct the tests' original flaws:

  1. memos are supposed to be encoded by custom dataconverters, and they are. we don't search them so they're not JSON like search attrs are, they're just blobs we expose in list APIs (and in workflow metadata).
  2. Equal(string, []byte) just doesn't work, and the default dataconverter adds a trailing newline to separate args.
  3. the []byte-heavy copypasta meant the Input-checking tests were odd, so I changed them, and fixed the args to the dataconverter. Internally we splat the args in here, so it's not []interface{..}-packed: https://github.com/uber-go/cadence-client/blob/6decfc78571a9d91d943815ae3a445a3bc115fa8/internal/internal_worker.go#L573-L579
codecov[bot] commented 3 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 73.26%. Comparing base (f5ec359) to head (ba4e2ac).

Additional details and impacted files [see 1 file with indirect coverage changes](https://app.codecov.io/gh/uber-go/cadence-client/pull/1341/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go) ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/uber-go/cadence-client/pull/1341?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://app.codecov.io/gh/uber-go/cadence-client/pull/1341?dropdown=coverage&src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go). Last update [f5ec359...ba4e2ac](https://app.codecov.io/gh/uber-go/cadence-client/pull/1341?dropdown=coverage&src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go).