IWF-343: Fix concurrent write panic for parallel test #545
Conversation
| h.invokeHistory.Store(req.GetWorkflowStateId()+"_start", value.(int64)+1) | ||
| } else { | ||
| h.invokeHistory.Store(req.GetWorkflowStateId()+"_start", int64(1)) | ||
| } |
There was a problem hiding this comment.
I couldn't reproduce the issue described in #472, but I can see that this issue could very well happen due to the nature of the Workflow in this test.
Looking for opinions on if this is something that we need to polish a bit more, and if we want to make all our other tests also use sync.Map for storing the current invokeHistory in the integration tests to avoid this issue in the future too
There was a problem hiding this comment.
As discussed today, I have now applied the same logic now for all the tests. i.e. all tests now use sync.Map which can be written to concurrently
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #545 +/- ##
==========================================
+ Coverage 66.06% 66.08% +0.02%
==========================================
Files 62 62
Lines 6927 6927
==========================================
+ Hits 4576 4578 +2
+ Misses 2072 2070 -2
Partials 279 279 ☔ View full report in Codecov by Sentry. |
| @@ -10,7 +10,7 @@ import ( | |||
| "time" | |||
|
|
|||
| "github.com/indeedeng/iwf/gen/iwfidl" | |||
| anycommandconbination "github.com/indeedeng/iwf/integ/workflow/any_command_combination" | |||
| anycommandcombination "github.com/indeedeng/iwf/integ/workflow/any_command_combination" | |||
There was a problem hiding this comment.
This was just a typo that I noticed while working on the tests
Description
Checklist
Related Issue
Closes #472