Adds test for execution/mutable_state_builder.go #5744
Conversation
davidporter-id-au
requested review from
Shaddoll,
neil-xie,
Groxx,
shijiesheng,
agautam478,
jakobht,
3vilhamster,
sankari165,
dkrotx,
taylanisikdemir and
demirkayaender
as code owners
Codecov Report
Additional details and impacted files
... and 14 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Pull Request Test Coverage Report for Build 018e16d6-0f2b-4f72-8c31-01dd993f21b8Details
💛 - Coveralls |
| @@ -990,3 +994,274 @@ func (s *mutableStateSuite) buildWorkflowMutableState() *persistence.WorkflowMut | |||
| VersionHistories: versionHistories, | |||
| } | |||
| } | |||
|
|
|||
| func TestNewMutableStateBuilderWithEventV2(t *testing.T) { | |||
|
|
|||
There was a problem hiding this comment.
nit: remove empty first lines in functions
There was a problem hiding this comment.
sure, but I'd suggest putting that in lint if it's a thing we want to do more generally
There was a problem hiding this comment.
Would be nice to remove generous new lines like this via linter which IMO hurts readability a little bit. Consistently spaced codebase is much easier to read thru. We are far from consistency so this new line is not in the top of my list :) Maybe @Groxx can quickly come up with the magic regex for this too
| }, nil) | ||
|
|
||
| }, | ||
|
|
|
|
||
| for name, td := range tests { | ||
| t.Run(name, func(t *testing.T) { | ||
|
|
There was a problem hiding this comment.
nit: remove empty first line of funcs
| res, err := td.currentState.GetCompletionEvent(context.Background()) | ||
|
|
||
| assert.Equal(t, td.expectedResult, res) | ||
| assert.Equal(t, td.expectedErr, err) |
There was a problem hiding this comment.
assert.EqualError does better job (I could be wrong. little experience with this assert package)
There was a problem hiding this comment.
I also admit not being super familiar with that specifically. It looks like it is just doing string comparison though so probably assert.ErrorAs and assert.ErrorIs I suspect. Let me look
This is part of a wider initiative to increase unit-test coverage for this package