Skip to content
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

Ignore search attributes from GetVersion in mocks #1643

Open
ndtretyak opened this issue Sep 19, 2024 · 4 comments
Open

Ignore search attributes from GetVersion in mocks #1643

ndtretyak opened this issue Sep 19, 2024 · 4 comments

Comments

@ndtretyak
Copy link
Contributor

ndtretyak commented Sep 19, 2024

Expected Behavior

Search attributes from GetVersion does not affect OnUpsertSearchAttributes mock

Actual Behavior

Tests fail if UpsertSearchAttributes calls from versioning are not mocked explicitly

Steps to Reproduce the Problem

  1. Use GetVersion and UpsertSearchAttribute in your workflow
  2. Use env.OnUpsertSearchAttribute in a test to validate the arguments of UpsertSearchAttributes
  3. Test will fail on unexpected calls to UpsertSearchAttributes made from GetVersion
@Quinn-With-Two-Ns
Copy link
Contributor

Search attributes from GetVersion is intended to affect OnUpsertSearchAttributes. OnUpsertSearchAttributes is intended to be called on any upsert search attribute, which includes GetVersion. We should clarify in the docs that OnUpsertSearchAttributes will be used used for GetVersion

@ndtretyak
Copy link
Contributor Author

When multiple GetVersion calls are present across different branches of a workflow, mocking each OnUpsertSearchAttributes becomes challenging due to the accumulation of all previous changeIDs in an unpredictable order.

Additionally, getChangeVersions is non-deterministic because it iterates over a map, and its result is used in the UpsertSearchAttributes call.

TemporalChangeVersion: getChangeVersions(changeID, version, existingChangeVersions),

@Quinn-With-Two-Ns
Copy link
Contributor

You shouldn't need to assert the exact value of changeID if you are not interested in it. You should be able to use something like MatchedBy to just match the key in the map.

env.OnUpsertSearchAttributes(mock.MatchedBy(func(attr map[string]interface{}) bool {

@ndtretyak
Copy link
Contributor Author

Thanks! This is exactly what I needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants