[Access] Make response for data providers consistent. #6846
Conversation
…github.com:The-K-R-O-K/flow-go into AndriiDiachuk/6819-make-response-for-data-providers-consistent
…achuk/6819-make-response-for-data-providers-consistent
AndriiDiachuk
requested review from
Guitarheroua,
UlyanaAndrukhiv and
illia-malachyn
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6846 +/- ##
=======================================
Coverage 41.18% 41.18%
=======================================
Files 2109 2109
Lines 185660 185689 +29
=======================================
+ Hits 76460 76476 +16
- Misses 102788 102795 +7
- Partials 6412 6418 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| Height: strconv.FormatUint(accountStatusesResponse.Height, 10), | ||
| AccountEvents: accountStatusesResponse.AccountEvents, | ||
| MessageIndex: index, | ||
| p.send <- &models.BaseDataProvidersResponse{ |
There was a problem hiding this comment.
How about adding a common function or method Build for BaseDataProvidersResponse that creates/fills BaseDataProvidersResponse and is used by every provider? Also I guess, we can use baseDataProvider as an argument to fill SubscriptionID and Topic. This approach would help avoid code duplication in the providers. What do you think?
There was a problem hiding this comment.
This PR should be merged after #6802 and use response.Build functionality for Payload field
|
|
||
| require.Equal(s.T(), expectedAccountStatusesResponse.BlockID.String(), actualResponse.BlockID) | ||
| require.Equal(s.T(), len(expectedAccountStatusesResponse.AccountEvents), len(actualResponse.AccountEvents)) | ||
| actualResponseData, ok := actualResponse.Payload.(*models.AccountStatusesResponse) |
There was a problem hiding this comment.
nit: I think we should also check the Topic here and in other tests if we added it to the response.
Closes: #6819