Adding UTs for negative cases in MRD #3010
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3010 +/- ##
==========================================
+ Coverage 75.25% 75.55% +0.30%
==========================================
Files 125 126 +1
Lines 17743 17727 -16
==========================================
+ Hits 13352 13394 +42
+ Misses 3824 3778 -46
+ Partials 567 555 -12
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
kislaykishore
requested review from
a team,
gargnitingoogle,
tritone,
vadlakondaswetha and
BrennaEpp
and removed request for
vadlakondaswetha,
a team,
gargnitingoogle,
tritone and
BrennaEpp
abhishek10004
changed the title
| buf := make([]byte, tc.dataSize) | ||
|
|
||
| bytesRead, err := t.rr.wrapped.readFromMultiRangeReader(t.rr.ctx, buf, 0, int64(t.object.Size), TestTimeoutForMultiRangeRead) | ||
|
|
||
| if i == len(testCases)-1 && bytesRead != 0 { | ||
| if tc.name == "TimeoutValue" && bytesRead != 0 { |
There was a problem hiding this comment.
only 1 test case matches this name. why do you need 2nd condition?
| t.rr.wrapped.mrdWrapper = &fakeMRDWrapper | ||
| t.mockBucket.On("NewMultiRangeDownloader", mock.Anything, mock.Anything).Return(fake.NewFakeMultiRangeDownloaderWithSleep(t.object, testContent, tc.sleepTime)).Times(1) | ||
| t.mockBucket.On("BucketType", mock.Anything).Return(gcs.BucketType{Zonal: true}).Times(1) | ||
| if tc.name == "NilMRDWrapper" { |
There was a problem hiding this comment.
Its preferred to extract testcases in seperate methods if you need if-else. It becomes difficult to maintain and extend in future.
For eg, NilMRDWrapper doesn't require lot of this code. can you move it as seperate test method..
| buf := make([]byte, tc.end-tc.start) | ||
| t.mrdWrapper.Wrapped = nil | ||
| if tc.name == "ErrorInCreatingMRD" { | ||
| t.mockBucket.On("NewMultiRangeDownloader", mock.Anything, mock.Anything).Return(nil, fmt.Errorf("Error in creating MRD")).Once() |
There was a problem hiding this comment.
Please extract these also into seperate test methods. Its okay to duplicate code in test methods for readability.
| @@ -160,6 +165,57 @@ func (t *mrdWrapperTest) Test_Read() { | |||
| } | |||
| } | |||
|
|
|||
| func (t *mrdWrapperTest) Test_Read_ErrorHandling() { | |||
There was a problem hiding this comment.
Other cases are when callback returns EOF and non EOF errors.
Adding UTs for negative cases