-
Notifications
You must be signed in to change notification settings - Fork 180
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Access] Make response for data providers consistent. #6846
base: master
Are you sure you want to change the base?
[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
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6846 +/- ##
==========================================
+ Coverage 41.11% 41.13% +0.01%
==========================================
Files 2116 2117 +1
Lines 185749 185778 +29
==========================================
+ Hits 76378 76418 +40
+ Misses 102954 102947 -7
+ Partials 6417 6413 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
engine/access/rest/websockets/data_providers/account_statuses_provider.go
Outdated
Show resolved
Hide resolved
engine/access/rest/websockets/data_providers/account_statuses_provider_test.go
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not have any other comments. Looks good!
engine/access/rest/websockets/data_providers/blocks_provider.go
Outdated
Show resolved
Hide resolved
…achuk/6819-make-response-for-data-providers-consistent
…providers-consistent
…achuk/6819-make-response-for-data-providers-consistent
…sistent' of github.com:The-K-R-O-K/flow-go into AndriiDiachuk/6819-make-response-for-data-providers-consistent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added a couple small comments, but otherwise looks good.
} | ||
|
||
// Build creates BaseDataProvidersResponse instance for consistent responses of the data providers. | ||
func (b *BaseDataProvidersResponse) Build(subscriptionID uuid.UUID, topic string, payload interface{}) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is changing to string: #6847
func (b *BaseDataProvidersResponse) Build(subscriptionID uuid.UUID, topic string, payload interface{}) { | |
func (b *BaseDataProvidersResponse) Build(subscriptionID string, topic string, payload interface{}) { |
expectedResponse, ok := expected.(*models.BaseDataProvidersResponse) | ||
require.True(s.T(), ok, "Expected *models.BaseDataProvidersResponse, got %T", expected) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like you're doing this in a lot of places. maybe create a helper function? something like
func extractPayload[T any](t *testing.T, v interface{}) T {
response, ok := v.(*BaseDataProvidersResponse)
require.True(t, ok, "Expected *models.BaseDataProvidersResponse, got %T", v)
payload, ok := response.Payload.(T)
require.True(t, ok, "Unexpected response payload type: %T", response.Payload)
return payload
}
then usage
expectedResponsePayload := extractPayload[*models.AccountStatusesResponse](t, expected)
actualResponsePayload := extractPayload[*models.AccountStatusesResponse](t, actual)
Closes: #6819