feat: api schema tests #168
Conversation
elijaharita
force-pushed
the
feat/api-schema-tests
branch
from
c371cf0
to
406d61a
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #168 +/- ##
==========================================
+ Coverage 30.82% 31.49% +0.67%
==========================================
Files 5 5
Lines 743 743
==========================================
+ Hits 229 234 +5
+ Misses 479 474 -5
Partials 35 35
|
…ata must be present previously
| resp.Body.Close() | ||
|
|
||
| // Status code must be as expected | ||
| require.Equal(t, test.expectStatus, resp.StatusCode, "Incorrect status code for test %#v (resp body: %v)", test, body) |
There was a problem hiding this comment.
I recommend using
https://github.com/parnurzeal/gorequest
to simplify above code
There was a problem hiding this comment.
I would avoid extra dependencies if we can help it.
There was a problem hiding this comment.
avoiding extra dependencies was my thinking while writing this
There was a problem hiding this comment.
I think such dependency would greatly reduce the LOC for testing, plus, it won't be built into the binary.
| } | ||
|
|
||
| // Find matching schema case and mark as covered | ||
| for i := range schemaCases { |
There was a problem hiding this comment.
I would do it differently
- use client generation tool such as github.com/deepmap/oapi-codegen to generate the client code, this also validates the yaml file against openapi spec
- Since we only have 3 APIs, it maybe overkill to check all API paths are covered so I would just rely on code coverage tool.
There was a problem hiding this comment.
Re 1. Do you think it's worth the increase in LOC? No one is asking for http client library for such simple api (yet) right?
Re 2. Either works. Considering the work is done already, and we want to focus on e2e tests first/more than vide coverage would it make sense to revisit this at later date and leave it as is for now?
There was a problem hiding this comment.
considering that everything works right now, and the code does and will continue to do its job for now, i'd say we can look at codegen client / coverage as a followup, as they are more heavy commitments compared to what i wrote so far
closes #28
the test runs through an array of test cases for each endpoint, method, and status code. the test reads openapi.yaml and validates that all possible endpoint/method/status combination is covered, otherwise it will not pass.
writing the tests helped catch 1 bug - an anonymous error was returned instead of
blob.ErrBlobNotFoundon the SingularityDescribe()implementation when an unknown ID was provided