Add unit tests for PinotVisibilityStore #5714
Conversation
bowenxia
requested review from
Shaddoll,
neil-xie,
davidporter-id-au,
Groxx,
shijiesheng,
agautam478,
jakobht,
3vilhamster,
sankari165,
dkrotx,
taylanisikdemir and
demirkayaender
as code owners
bowenxia
changed the title
Pull Request Test Coverage Report for Build 018e152d-ca0e-4ec0-8d30-3c3b356a75a7Details
💛 - Coveralls |
common/pinot/GenericClient_mock.go
Outdated
| // SOFTWARE. | ||
|
|
||
| // Code generated by MockGen. DO NOT EDIT. | ||
| // Source: interfaces.go |
There was a problem hiding this comment.
how did you generate this? I don't see common/pinot/interfaces.go in the diff
There was a problem hiding this comment.
https://github.com/uber/cadence/blob/master/common/pinot/interfaces.go
I implemented it previously. Since ESVisibilityStore were using a generic es client, I used the same pattern.
There was a problem hiding this comment.
I meant interface.go doesn't have any change in the PR. We usually generate mocks by adding go:generate lines to the interface file. example: https://github.com/uber/cadence/blob/68706a6219d11a2113d9bc1d257997c85da0f433/client/admin/interface.go#L31
There was a problem hiding this comment.
This is because of a go version issue. The previous branch was massed up, so I just copied and pasted the GenericClient_mock.go. I've added the comment back.
| mockPinotClient := pnt.NewMockGenericClient(ctrl) | ||
| mockProducer := &mocks.KafkaProducer{} | ||
| mgr := NewPinotVisibilityStore(mockPinotClient, &service.Config{ | ||
| ValidSearchAttributes: dynamicconfig.GetMapPropertyFn(definition.GetDefaultIndexedKeys()), | ||
| ESIndexMaxResultWindow: dynamicconfig.GetIntPropertyFn(3), | ||
| }, mockProducer, testlogger.New(t)) | ||
| visibilityStore := mgr.(*pinotVisibilityStore) | ||
|
|
||
| // test non-empty request fields match | ||
| request := &p.InternalRecordWorkflowExecutionStartedRequest{} | ||
| request.WorkflowID = "wid" | ||
| memoBytes := []byte(`test bytes`) | ||
| request.Memo = p.NewDataBlob(memoBytes, common.EncodingTypeThriftRW) |
There was a problem hiding this comment.
create all the mocks inside the test body (line 97) so they are isolated for each test case
There was a problem hiding this comment.
Moved the mocks into test body, but left request here because this is the input of the test.
Codecov Report
Additional details and impacted files
... and 5 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
| ) | ||
|
|
||
| func TestRecordWorkflowExecutionStarted(t *testing.T) { | ||
|
|
| expectedResp *p.InternalListWorkflowExecutionsResponse | ||
| expectedError error | ||
| }{ | ||
| "Case1: normal case with nil response": { |
There was a problem hiding this comment.
Let's cover the error cases as well. maybe in follow up PR you can add more cases to the tests in this file.
What changed?
Add unit test
Why?
increment the coverage to 85%+
How did you test it?
Potential risks
Release notes
Documentation Changes