some func tests for update activity #6869
Conversation
| suite.Run(t, s) | ||
| } | ||
|
|
||
| func (s *ActivityApiUpdateClientTestSuite) waitForChan(ctx context.Context, ch chan struct{}) { |
There was a problem hiding this comment.
this needs to be generic helper which must be used on all chan operations!
|
|
||
| var activityCompleted atomic.Int32 | ||
| activityFunction := func() (string, error) { | ||
| if activityCompleted.Load() == 0 { |
There was a problem hiding this comment.
| s.Worker().RegisterWorkflow(workflowFn) | ||
| s.Worker().RegisterActivity(activityFunction) | ||
|
|
||
| wfId := "functional-test-activity-update-api" |
There was a problem hiding this comment.
I'd introduce some randomness here just in case you want to be able to rerun this test without restarting the server.
| wfId := "functional-test-activity-update-api" | |
| wfId := testcore.RandomizeStr("wfid-"+s.T().Name()) |
| description, err := s.SdkClient().DescribeWorkflowExecution(ctx, workflowRun.GetID(), workflowRun.GetRunID()) | ||
| assert.NoError(t, err) | ||
| assert.Equal(t, 1, len(description.PendingActivities)) | ||
| assert.Equal(t, int32(1), activityCompleted.Load()) |
There was a problem hiding this comment.
Why not check the number of attempts via DescribeWorkflowExecution so you know for sure the server got the first error?
| s.NotNil(resp) | ||
|
|
||
| description, err := s.SdkClient().DescribeWorkflowExecution(ctx, workflowRun.GetID(), workflowRun.GetRunID()) | ||
| s.NotNil(description) |
There was a problem hiding this comment.
You can assume that describe response will not be nil and if there's an error, you'll want the test to fail with that error so we know what happened.
| s.NotNil(description) | |
| s.NoError(err) |
| s.activityRetryPolicy = &temporal.RetryPolicy{ | ||
| InitialInterval: s.initialRetryInterval, | ||
| BackoffCoefficient: 1, | ||
| MaximumInterval: 10 * time.Second, |
There was a problem hiding this comment.
Is this required if the coefficient is 1?
| assert.Equal(t, int32(2), activityCompleted.Load()) | ||
| }, 3*time.Second, 500*time.Millisecond) | ||
|
|
||
| description, err = s.SdkClient().DescribeWorkflowExecution(ctx, workflowRun.GetID(), workflowRun.GetRunID()) |
| ActivityID: "activity-id", | ||
| DisableEagerExecution: true, | ||
| StartToCloseTimeout: s.startToCloseTimeout, | ||
| ScheduleToCloseTimeout: s.scheduleToCloseTimeout, |
There was a problem hiding this comment.
I wouldn't set this. It doesn't give you much. You have a context deadline in the test already that will prevent it from running too long.
| s.NotNil(resp) | ||
|
|
||
| // activity should fail immediately | ||
| s.EventuallyWithT(func(t *assert.CollectT) { |
There was a problem hiding this comment.
Same as before, if the workflow is closed there won't be pending activities.
| s.EventuallyWithT(func(t *assert.CollectT) { | ||
| assert.Equal(t, int32(1), activityCompleted.Load()) | ||
| }, 2*time.Second, 200*time.Millisecond) |
There was a problem hiding this comment.
I would wait for the server to record the failure, it's more reliable.
| WorkflowId: workflowRun.GetID(), | ||
| ActivityId: "activity-id", | ||
| ActivityOptions: &activitypb.ActivityOptions{ | ||
| ScheduleToCloseTimeout: durationpb.New(5 * time.Second), |
There was a problem hiding this comment.
This isn't changing anything, you already set the schedule to close timeout to 5 seconds in the beginning of this test.
What changed?
Why?
To build some functional test coverage for the activity API