Reintroduce missing code for rest endpoint wiring #2080
Conversation
mmulji-ic
requested review from
alexanderbez,
zmanian,
yaruwangway,
crodriguezvega,
jackzampolin,
glnro,
jtremback,
mpoke,
sainoe,
MSalopek and
shaspitz
as code owners
|
Note integration tests are not added at this point. |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #2080 +/- ##
==========================================
- Coverage 84.85% 84.62% -0.24%
==========================================
Files 21 21
Lines 1466 1470 +4
==========================================
Hits 1244 1244
- Misses 177 181 +4
Partials 45 45
|
| ) | ||
|
|
||
| // Helper function to read the response body and unmarshal it into a map | ||
| func readJSON(resp *http.Response) (map[string]interface{}, error) { |
There was a problem hiding this comment.
We have a similar helper here...https://github.com/cosmos/gaia/blob/main/tests/e2e/http_util.go and similar functionality in https://github.com/cosmos/gaia/blob/main/tests/e2e/query.go#L23...maybe there's a way to refactor
| if err != nil { | ||
| testOk = false | ||
| s.T().Logf("failed to get endpoint: %s, %v", endpointURL+endpoint.Path, err) | ||
| continue | ||
| } |
There was a problem hiding this comment.
| if err != nil { | |
| testOk = false | |
| s.T().Logf("failed to get endpoint: %s, %v", endpointURL+endpoint.Path, err) | |
| continue | |
| } | |
| s.NoError(err, fmt.Sprintf("Failed to get endpoint: %s%s, %s", endpointURL, endpoint.Path, err.Error())) |
| jsonBody, errJSON := readJSON(resp) | ||
| if errJSON != nil { | ||
| testOk = false | ||
| s.T().Logf("failed to read body of endpoint: %s, %v", endpointURL+endpoint.Path, errJSON) | ||
| continue | ||
| } |
There was a problem hiding this comment.
| jsonBody, errJSON := readJSON(resp) | |
| if errJSON != nil { | |
| testOk = false | |
| s.T().Logf("failed to read body of endpoint: %s, %v", endpointURL+endpoint.Path, errJSON) | |
| continue | |
| } | |
| jsonBody, err := readJSON(resp) | |
| s.NoError(err, fmt.Sprintf("Failed to read body of endpoint: %s%s, %s", endpointURL, endpoint.Path, err.Error())) |
| for _, endpoint := range testEndpoints { | ||
|
|
||
| // Call the required endpoint | ||
| resp, err := http.Get(endpointURL + endpoint.Path) |
There was a problem hiding this comment.
For failure cases, if the endpoint doesn't exist, there's not really a reason for the final test clause, you might as well just assert s.EqualValues(resp.StatusCode, 200), which will fail as the missing_endpoint test case will return a 501
There was a problem hiding this comment.
This is good. I also wanted to check the structure strictly (will leave that for later).
| {"/node_info", false}, | ||
| {"/syncing", false}, | ||
| {"/missing_endpoint", true}, | ||
| } |
There was a problem hiding this comment.
we can check every endpoint, not limited to these 3, but can do this later
There was a problem hiding this comment.
+1. atm just want to make sure that the module is wired ok.
|
|
||
| if endpoint.ExpectedFail == false && jsonBody["message"] == "Not Implemented" { | ||
| testOk = false | ||
| s.T().Logf("Encountered a Not implemented endpoint: %s", endpointURL+endpoint.Path) |
There was a problem hiding this comment.
is it should be "Not Implemented""? otherwise, can just fail here, log might not make people realize if this test fails. maybe can use s.Require.False(jsonBody["message"] == "Not Implemented") or use s.Require.True to check how the jsonBody["message"] should be might be better.
| 8. Staking params | ||
| */ | ||
|
|
||
| var ( |
There was a problem hiding this comment.
Nit: maybe consider using const.
| var ( | |
| const ( |
|
Kudos, SonarCloud Quality Gate passed!
|
* Reintroduce missing code for rest endpoint wiring * Fix gofumpt * e2e test to check if endpoints are available * Ran linter/formater * Refactor and add comprehensive tests for missing routes (#2094) * Fixed var -> const definition --------- Co-authored-by: lg <lauren@interchain.io> Co-authored-by: lg <8335464+glnro@users.noreply.github.com> (cherry picked from commit 690f167)
* Reintroduce missing code for rest endpoint wiring * Fix gofumpt * e2e test to check if endpoints are available * Ran linter/formater * Refactor and add comprehensive tests for missing routes (#2094) * Fixed var -> const definition --------- Co-authored-by: lg <lauren@interchain.io> Co-authored-by: lg <8335464+glnro@users.noreply.github.com> (cherry picked from commit 690f167)
* Reintroduce missing code for rest endpoint wiring * Fix gofumpt * e2e test to check if endpoints are available * Ran linter/formater * Refactor and add comprehensive tests for missing routes (#2094) * Fixed var -> const definition --------- Co-authored-by: lg <lauren@interchain.io> Co-authored-by: lg <8335464+glnro@users.noreply.github.com> (cherry picked from commit 690f167) Co-authored-by: Milan Mulji <98309852+mmulji-ic@users.noreply.github.com>
* Reintroduce missing code for rest endpoint wiring * Fix gofumpt * e2e test to check if endpoints are available * Ran linter/formater * Refactor and add comprehensive tests for missing routes (#2094) * Fixed var -> const definition --------- Co-authored-by: lg <lauren@interchain.io> Co-authored-by: lg <8335464+glnro@users.noreply.github.com> (cherry picked from commit 690f167) Co-authored-by: Milan Mulji <98309852+mmulji-ic@users.noreply.github.com>
Fixes issue #2074