IWF-475: Cleanup unnecessary waiting in integ tests and add wait comments #549
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #549 +/- ##
=======================================
Coverage 66.06% 66.06%
=======================================
Files 62 62
Lines 6927 6927
=======================================
Hits 4576 4576
Misses 2072 2072
Partials 279 279 ☔ View full report in Codecov by Sentry. |
integ/timer_test.go
Outdated
| @@ -197,7 +199,7 @@ func doTestTimerWorkflow(t *testing.T, backendType service.BackendType, config * | |||
| timer3.Status = service.TimerSkipped | |||
| assertTimerQueryResponseEqual(assertions, expectedTimerInfos, timerInfos) | |||
|
|
|||
| // wait for the workflow | |||
| // Wait for the workflow | |||
There was a problem hiding this comment.
// Wait for the workflow to complete
?
There was a problem hiding this comment.
Updated here and a good handful of other places with the same comment.
integ/deadend_test.go
Outdated
| @@ -115,8 +117,9 @@ func doTestDeadEndWorkflow(t *testing.T, backendType service.BackendType, config | |||
| failTestAtHttpError(err, httpResp, t) | |||
| } | |||
|
|
|||
| // Short wait for workflow | |||
There was a problem hiding this comment.
Can we add more info here to what we expect from this wait?
integ/greedy_timer_test.go
Outdated
| @@ -103,6 +103,7 @@ func doTestGreedyTimerWorkflowCustomConfig(t *testing.T, backendType service.Bac | |||
| }).Execute() | |||
| failTestAtHttpError(err, httpResp, t) | |||
|
|
|||
| // Short wait for workflow to initialize | |||
There was a problem hiding this comment.
I'm not sure about these three
- L106: This is after starting a workflow, but it's a sync operation, since the response returns workflowId meaning the workflow has been intialized
- L129: No workflow start right before it
- L181: No workflow start right before it
| @@ -129,7 +129,7 @@ func doTestAnyCommandCombinationWorkflow(t *testing.T, backendType service.Backe | |||
| }).Execute() | |||
| failTestAtHttpError(err, httpResp, t) | |||
|
|
|||
| // wait and check the workflow, it should be still running | |||
| // Wait and check the workflow, it should be still running | |||
There was a problem hiding this comment.
I think this one adds delay to make sure the timer has been skipped before moving on?
There was a problem hiding this comment.
Updated comment and moved wait up to where it is more relevant.
| reqWait := apiClient.DefaultApi.ApiV1WorkflowGetWithWaitPost(context.Background()) | ||
| _, httpResp, err = reqWait.WorkflowGetRequest(iwfidl.WorkflowGetRequest{ | ||
| WorkflowId: wfId, | ||
| }).Execute() | ||
| failTestAtHttpError(err, httpResp, t) |
There was a problem hiding this comment.
I think you can remove L98+ since the workflow will be completed before moving on to the assertions
There was a problem hiding this comment.
Removed unneeded code
| reqWait := apiClient.DefaultApi.ApiV1WorkflowGetWithWaitPost(context.Background()) | ||
| _, httpResp, err = reqWait.WorkflowGetRequest(iwfidl.WorkflowGetRequest{ | ||
| WorkflowId: wfId, | ||
| }).Execute() | ||
| failTestAtHttpError(err, httpResp, t) |
There was a problem hiding this comment.
Same as integ/wf_state_options_data_attributes_loading_test.go
There was a problem hiding this comment.
Removed unneeded code
|
@lwolczynski I pushed a commit based on your comments. Thanks. |
…ents
Description
Checklist
Related Issue
Closes #issue_number