add block encoding param to block index api#1752
add block encoding param to block index api#1752najeal wants to merge 5 commits intoava-labs:mainfrom
Conversation
| "github.com/ava-labs/hypersdk/codec" | ||
| ) | ||
|
|
||
| func TestEncodingValidate(t *testing.T) { |
| } | ||
| } | ||
|
|
||
| func TestBlockResponse(t *testing.T) { |
There was a problem hiding this comment.
Could we test via httptest rather than calling individual functions this way?
ex of setting up httptest server: https://github.com/ava-labs/hypersdk/blob/main/tests/integration/integration.go#L236
There was a problem hiding this comment.
There are two ways we could go testing here imo:
- test the server/client w/ mocked indexer using httptest and test the indexer separately
- test the indexer + server/client solely through the server/client
Could we add basic tests here that use httptest and the server/client with a mocked of the indexer (will need to change indexer to interface as it's used in server/client)?
There was a problem hiding this comment.
We should also avoid this pattern of using t.Run within a single unit test. I think this makes sense for table tests (and this could be changed to a table test imo), but less so in this pattern with mostly independent test logic:
t.Run("JSON", func(t *testing.T) {
// independent test logic
})
t.Run("Hex", func(t *testing.T) {
// independent test logic
})There was a problem hiding this comment.
Good idea by the way! The client was broken because of my changes !
| ctx, | ||
| "getBlock", | ||
| &GetBlockRequest{BlockID: blkID}, | ||
| &GetBlockRequest{BlockID: blkID, Encoding: Hex}, |
There was a problem hiding this comment.
I did not provide the Encoding through variable, I think this client will continue to always use Hex.
2 participants
Closes #1632