Refactor QueryService tests #3060
Conversation
Signed-off-by: albertteoh <albert.teoh@logz.io>
| archiveSpanWriter *spanstoremocks.Writer | ||
| } | ||
|
|
||
| type QueryServiceOptionApplier func(*testMocks, *QueryServiceOptions) |
There was a problem hiding this comment.
usually these types are named simply XyzOption
There was a problem hiding this comment.
btw, I'm planning to do the same thing for QueryServiceOptions in the next PR, as I think MetricsQuery reader belongs in this struct.
Can you think of a good name for the Option? QueryServiceOptionsOption seems a little too stutter-y and I'm a little short on other ideas on names. :P
Codecov Report
@@ Coverage Diff @@
## master #3060 +/- ##
==========================================
+ Coverage 95.97% 96.03% +0.06%
==========================================
Files 229 229
Lines 9956 9956
==========================================
+ Hits 9555 9561 +6
+ Misses 330 326 -4
+ Partials 71 69 -2
Continue to review full report at Codecov.
|
| } | ||
|
|
||
| func initializeTestService() (*QueryService, *spanstoremocks.Reader, *depsmocks.Reader) { | ||
| func initializeTestService(optionAppliers ...QueryServiceOptionApplier) (*QueryService, *testMocks) { |
There was a problem hiding this comment.
since you're already introducing a new struct testMocks, you might instead return a single testQueryService struct which contains both the *QS and the mocks at top level
There was a problem hiding this comment.
glad you mentioned this as this was also what I wanted to do initially, just thought there may have been a preference to separate the "thing under test" from the mocks.
There was a problem hiding this comment.
tqsOption - these are private types, no need to be especially creative
Signed-off-by: albertteoh <albert.teoh@logz.io>
|
Thanks Yuri! |
Signed-off-by: albertteoh albert.teoh@logz.io
Which problem is this PR solving?
Short description of the changes
testMocksstruct rather than return multiple mock objects.