-
Notifications
You must be signed in to change notification settings - Fork 18
Extend test helpers #189
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
Extend test helpers #189
Conversation
| - name: Test Kitchensink ${{ matrix.sdk }} | ||
| run: | | ||
| SDK=${{ matrix.sdk }} go test -race ./loadgen -run TestKitchensink -v | ||
| SDK=${{ matrix.sdk }} go test -race ./loadgen -run TestKitchenSink -v |
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.
Renamed for consistency.
| // WorkflowExecutionStarted {"taskQueue":"foo-bar"} | ||
| // ... | ||
| // WorkflowExecutionCompleted | ||
| type partialHistoryMatcher string |
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.
Needed because some tests are not deterministic enough to run the full history matcher.
| if executeErr != nil { | ||
| return fmt.Errorf("failed to execute kitchen sink workflow: %w", executeErr) | ||
| } | ||
| if clientActionsErr != nil { |
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.
Data race here.
| if b.stdout == nil { | ||
| b.stdout = &logWriter{logger: b.Logger} | ||
| } | ||
| if b.stderr == nil { |
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.
With these the build logs will be associated with the right test logs.
0145548 to
e410caf
Compare
| @@ -0,0 +1,190 @@ | |||
| package kitchensink | |||
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.
Moved over here for discoverability (helpful once we have multiple impls).
| EventID int64 | ||
| Type string | ||
| Fields map[string]any | ||
| type event struct { |
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.
Some refactorings to make the events a typed entity.
| }, | ||
| } { | ||
| t.Run(tc.name, func(t *testing.T) { | ||
| t.Parallel() |
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.
Parallel!
6b9875d to
5000aa8
Compare
| github.com/spf13/pflag v1.0.5 | ||
| github.com/stretchr/testify v1.10.0 | ||
| github.com/temporalio/features v0.0.0-20250714172315-c9d352c46b16 | ||
| github.com/temporalio/features v0.0.0-20250808182149-bb2a99cdf200 |
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.
Updated to pull in temporalio/features#662
99e32ae to
06abbb2
Compare
| "github.com/temporalio/omes/loadgen" | ||
| ) | ||
|
|
||
| func TestThroughputStress(t *testing.T) { |
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.
New end-to-end test using the new test environment.
d025a68 to
ffe57f0
Compare
| WorkflowExecutionCompleted`), | ||
| }, | ||
| { | ||
| name: "ClientSequence - Nested", |
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.
Example of a new test case that uses partialHistoryMatcher - more to follow.
ffe57f0 to
293cc59
Compare
| return historyEvents | ||
| } | ||
|
|
||
| func ensureWorkerBuilt(t *testing.T, sdk cmdoptions.Language) error { |
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.
Moved all this into test helper.
293cc59 to
cdf2832
Compare
cdf2832 to
a705954
Compare
| github.com/spf13/pflag v1.0.5 // indirect | ||
| github.com/stretchr/objx v0.5.2 // indirect | ||
| github.com/stretchr/testify v1.10.0 // indirect | ||
| github.com/temporalio/features v0.0.0-20250808182149-bb2a99cdf200 // indirect |
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.
Transitive dependency ...
42b10f5 to
f704727
Compare
f704727 to
12861e0
Compare
Sushisource
left a comment
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 guess a bunch of this is in the Go Client activity type PR, yes? Just skimmed it b/c of that.
|
Oh yeah, it's the same commit as in #179 |
ℹ️ This PR includes the #189 PR; you can just look at the "Client ActivityType" commit for review. <!--- Note to EXTERNAL Contributors --> <!-- Thanks for opening a PR! If it is a significant code change, please **make sure there is an open issue** for this. We work best with you when we have accepted the idea first before you code. --> <!--- For ALL Contributors 👇 --> ## What was changed <!-- Describe what has changed in this PR --> Adds new "Client" ActivityType to execute client actions (e.g. Update, Signal) from a workflow activity. ## Why? <!-- Tell your future self why have you made these changes --> - Keeps all action execution in one place (ie the workflow). - Makes client actions part of the event history (free resume-ability, easier debugging). - Execution is (more) deterministic. ## Checklist <!--- add/delete as needed ---> 1. Closes <!-- add issue number here --> 2. How was this tested: <!--- Please describe how you tested your changes/how we can test them --> 3. Any docs updates needed? <!--- update README if applicable or point out where to update docs.temporal.io -->
What was changed
Prelude for #179:
Why?
Improve test experience.
Checklist
Closes
How was this tested: