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

Improve Update Workflow testing UX #1721

Merged
merged 4 commits into from
Nov 25, 2024

Conversation

yuandrew
Copy link
Contributor

What was changed

Added default implementation TestUpdateCallbacks for basic test scenarios

Why?

so tests don't need to implement UpdateCallbacks themselves. To make testing a little easier

Checklist

  1. Closes Improve UX for testing Workflow Update with the Workflow Test Environment #1639

  2. How was this tested:

existing unit tests that were effected still pass

@yuandrew yuandrew requested a review from a team as a code owner November 21, 2024 22:43
@yuandrew
Copy link
Contributor Author

@Quinn-With-Two-Ns lmk if there's other improvements you were thinking about when you filed the issue that I didn't cover

// TestUpdateCallback is a basic implementation of the UpdateCallbacks interface for testing purposes.
// Tests are welcome to implement their own version of this interface if they need to test more complex
// update logic. This is a simple implementation to make testing basic Workflow Updates easier.
TestUpdateCallback struct {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue was intended to improve the UX for users so we would need to expose this https://github.com/temporalio/sdk-go/blob/master/testsuite/testsuite.go#L35

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops! didn't realize this was needed. Thanks for pointing it out

// Tests are welcome to implement their own version of this interface if they need to test more complex
// update logic. This is a simple implementation to make testing basic Workflow Updates easier.
TestUpdateCallback struct {
accept func()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These fields are private so you also need to make these public so the type can be constructed. To make them public you need to capitalize the first letter. I would also suggest prefixing them with On to be a bit more consistent with the other test environment API.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks :) tested with temporalio/samples-go#371, hopefully not missing anything else

@yuandrew yuandrew merged commit c7fa7e8 into temporalio:master Nov 25, 2024
13 checks passed
@yuandrew yuandrew deleted the improve-workflow-update-ux branch November 25, 2024 22:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve UX for testing Workflow Update with the Workflow Test Environment
2 participants