refactoring changes to make testing possible for inode package using testify mock bucket #2696
Conversation
kislaykishore
requested review from
a team,
BrennaEpp,
tritone and
gargnitingoogle
and removed request for
a team,
BrennaEpp and
tritone
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2696 +/- ##
==========================================
- Coverage 77.53% 77.21% -0.33%
==========================================
Files 112 113 +1
Lines 15801 15879 +78
==========================================
+ Hits 12252 12261 +9
- Misses 3025 3095 +70
+ Partials 524 523 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
| @@ -124,3 +125,11 @@ func (m *TestifyMockBucket) CreateFolder(ctx context.Context, folderName string) | |||
| } | |||
| return nil, args.Error(1) | |||
| } | |||
|
|
|||
| func (m *TestifyMockBucket) SyncObject(ctx context.Context, fileName string, srcObject *gcs.Object, content gcsx.TempFile) (o *gcs.Object, err error) { | |||
There was a problem hiding this comment.
MockBucket is for mocking bucket operations. Why are we adding custom operations to this file?
There was a problem hiding this comment.
Added this so that mock bucket also implements SyncerBucketInterface. I want to use it for file_test.go file which uses syncer bucket. Don't think there is value in creating a duplicate struct. Let me know if you think otherwise.
| @@ -124,3 +125,11 @@ func (m *TestifyMockBucket) CreateFolder(ctx context.Context, folderName string) | |||
| } | |||
| return nil, args.Error(1) | |||
| } | |||
|
|
|||
| func (m *TestifyMockBucket) SyncObject(ctx context.Context, fileName string, srcObject *gcs.Object, content gcsx.TempFile) (o *gcs.Object, err error) { | |||
There was a problem hiding this comment.
also fakes are preferred over mocks. Why do you want to have a mock for syncObject when the methods used by internally has fake implementation.
There was a problem hiding this comment.
In file_test.go we are creating the content (temp file) before hand for the tests. This won't work for buffered writes as I want to ensure that in case of failure temp file is created. That is why I needed mocks. Do you think there is a better way to do this?
Description
This PR includes following refactoring changes:
Link to the issue in case of a bug fix.
NA
Testing details