test: use Petstore spec from GitHub#2400
Conversation
We're seeing intermittent issues from the Petstore project such as:
error loading swagger spec in spec.yaml
: failed to load OpenAPI specification: error resolving reference
"https://petstore3.swagger.io/api/v3/openapi.json#/components/schemas/Pet":
error loading "https://petstore3.swagger.io/api/v3/openapi.json":
request returned status code 403
We can try and switch to the raw spec in GitHub to do this lookup, to
avoid hitting the Petstore API.
It's unclear if GitHub are going to be more performant, but it may help.
Greptile SummaryThis PR moves the externalref Petstore fixture away from the live Petstore API. The main changes are:
Confidence Score: 4/5This is close, but the external fixture should be pinned before merging.
internal/test/externalref/spec.yaml and internal/test/externalref/externalref.cfg.yaml should use a pinned commit or a local vendored spec.
|
| Filename | Overview |
|---|---|
| internal/test/externalref/spec.yaml | Changes the Petstore external reference to a raw GitHub branch URL. |
| internal/test/externalref/externalref.cfg.yaml | Updates the import mapping to match the new Petstore URL. |
| internal/test/externalref/externalref.gen.go | Regenerates the embedded OpenAPI fixture and raw-spec path mapping. |
Reviews (1): Last reviewed commit: "test: use Petstore spec from GitHub" | Re-trigger Greptile
| pet: | ||
| $ref: https://petstore3.swagger.io/api/v3/openapi.json#/components/schemas/Pet | ||
| $ref: https://raw.githubusercontent.com/swagger-api/swagger-petstore/refs/heads/master/src/main/resources/openapi.yaml#/components/schemas/Pet |
There was a problem hiding this comment.
This now points go generate at refs/heads/master, so the generated fixture depends on whatever Petstore has on that branch at generation time. If swagger-api changes components.schemas.Pet, this repository can start producing different generated code or failing generation without any local change. Pinning this to a commit or vendored file keeps the externalref test reproducible.
| pet: | |
| $ref: https://petstore3.swagger.io/api/v3/openapi.json#/components/schemas/Pet | |
| $ref: https://raw.githubusercontent.com/swagger-api/swagger-petstore/refs/heads/master/src/main/resources/openapi.yaml#/components/schemas/Pet | |
| pet: | |
| $ref: https://raw.githubusercontent.com/swagger-api/swagger-petstore/<commit-sha>/src/main/resources/openapi.yaml#/components/schemas/Pet |
1f54120 to
d1bf1a4
Compare
|
I don't think that unit tests should be fetching stuff over the network at all. We can do that entirely with local files, but change looks good as-is. This isn't a performance issue, HTTP/403 means we're tripping over some kind of auth or rate limiting |
|
Yep, agreed that we shouldn't be hitting external services as part of our own builds (local or otherwise) and instead vendoring files Relatedly: #1564 Re performance, I mean more that these calls to GitHub may be more likely to be slow and/or hit issues given their recent availability |
We're seeing intermittent issues from the Petstore project such as:
We can try and switch to the raw spec in GitHub to do this lookup, to
avoid hitting the Petstore API.
It's unclear if GitHub are going to be more performant, but it may help.