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.

Sign up for GitHub

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

Jump to bottom

Allow setting activity timeouts in testsuite #1187

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Allow setting activity timeouts in testsuite #1187

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions internal/internal_workflow_testsuite.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,9 @@ type (
onTimerScheduledListener func(timerID string, duration time.Duration)
onTimerFiredListener func(timerID string)
onTimerCanceledListener func(timerID string)

activityScheduleToCloseTimeout time.Duration
activityStartToCloseTimeout time.Duration
}

// testWorkflowEnvironmentImpl is the environment that runs the workflow/activity unit tests.
Expand Down Expand Up @@ -236,6 +239,9 @@ func newTestWorkflowEnvironmentImpl(s *WorkflowTestSuite, parentRegistry *regist
callbackChannel: make(chan testCallbackHandle, 1000),
testTimeout: 3 * time.Second,
expectedMockCalls: make(map[string]struct{}),

activityScheduleToCloseTimeout: 600 * time.Second,
activityStartToCloseTimeout: 600 * time.Second,
},

workflowInfo: &WorkflowInfo{
Expand Down Expand Up @@ -544,8 +550,8 @@ func (env *testWorkflowEnvironmentImpl) executeActivity(

parameters := ExecuteActivityParams{
ExecuteActivityOptions: ExecuteActivityOptions{
ScheduleToCloseTimeout: 600 * time.Second,
StartToCloseTimeout: 600 * time.Second,
ScheduleToCloseTimeout: env.activityScheduleToCloseTimeout,
StartToCloseTimeout: env.activityStartToCloseTimeout,
},
ActivityType: *activityType,
Input: input,
Expand Down
46 changes: 46 additions & 0 deletions internal/internal_workflow_testsuite_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4031,3 +4031,49 @@ func (s *WorkflowTestSuiteUnitTest) Test_WorkflowGetCurrentHistoryLength() {
s.NoError(env.GetWorkflowResult(&result))
s.Equal(17, result)
}

func (s *WorkflowTestSuiteUnitTest) Test_ActivityWithStartToCloseTimeout() {
timeout := 100 * time.Millisecond

timeoutActivity := func(ctx context.Context) error {
time.Sleep(timeout * 2)
return nil
}

noTimeoutActivity := func(ctx context.Context) error {
Copy link
Member

@cretz cretz Aug 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The low timing resolution/granularity of this test will lead to flakiness on an overloaded CI worker. I think you can just remove the no-timeout assertion part.

time.Sleep(timeout / 2)
return nil
}

env := s.NewTestActivityEnvironment()
env.SetActivityStartToCloseTimeout(timeout)
env.RegisterActivity(timeoutActivity)
env.RegisterActivity(noTimeoutActivity)
_, err := env.ExecuteActivity(timeoutActivity)
s.Error(err)
_, err = env.ExecuteActivity(noTimeoutActivity)
s.NoError(err)
}

func (s *WorkflowTestSuiteUnitTest) Test_ActivityWithScheduleToCloseTimeout() {
timeout := 100 * time.Millisecond

timeoutActivity := func(ctx context.Context) error {
time.Sleep(timeout * 2)
return nil
}

noTimeoutActivity := func(ctx context.Context) error {
time.Sleep(timeout / 2)
return nil
}

env := s.NewTestActivityEnvironment()
env.SetActivityScheduleToCloseTimeout(timeout)
env.RegisterActivity(timeoutActivity)
env.RegisterActivity(noTimeoutActivity)
_, err := env.ExecuteActivity(timeoutActivity)
s.Error(err)
_, err = env.ExecuteActivity(noTimeoutActivity)
s.NoError(err)
}
10 changes: 10 additions & 0 deletions internal/workflow_testsuite.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,16 @@ func (t *TestActivityEnvironment) SetOnActivityHeartbeatListener(
return t
}

// SetActivityStartToCloseTimeout sets the start to close timeouts of activities in the test environment.
func (e *TestActivityEnvironment) SetActivityStartToCloseTimeout(timeout time.Duration) {
e.impl.testWorkflowEnvironmentShared.activityStartToCloseTimeout = timeout
}

// SetActivityStartToCloseTimeout sets the schedule to close timeouts of activities in the test environment
func (e *TestActivityEnvironment) SetActivityScheduleToCloseTimeout(timeout time.Duration) {
e.impl.testWorkflowEnvironmentShared.activityScheduleToCloseTimeout = timeout
}
Comment on lines +265 to +273
Copy link
Member

Choose a reason for hiding this comment

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

Hrmm, longer than 10-minutes using this framework is a bit rough. You might consider a proper integration test with a real server instead.

Regardless, I wonder if we should document on these what the default is and document any relation to SetTestTimeout here and on that method.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah we should include a comment here on what the default values are


// RegisterWorkflow registers workflow implementation with the TestWorkflowEnvironment
func (e *TestWorkflowEnvironment) RegisterWorkflow(w interface{}) {
e.impl.RegisterWorkflow(w)
Expand Down