Repository navigation
ci(lint): 193 test-only bodyclose findings in providers/azure (split from #1754) #63
Description
Split out of LeanerCloud/cloud-commitments-cli#1754 so it can move independently of that 1291-finding backlog. This subset is one linter, in one module.
Triage result: 193 findings, 0 real leaks
Requested triage was: separate live request paths from error paths from test-only/init-once code, and establish which are reachable in production. The answer is that none are, on two independent grounds, both verified rather than inferred.
1. Production reachability is zero, by Go build semantics
All 193 findings sit at call sites in _test.go files — 10 files, no exceptions:
| file | findings |
|---|---|
services/internal/reservations/purchase_test.go |
35 |
services/compute/client_test.go |
27 |
services/search/client_test.go |
22 |
services/managedredis/client_test.go |
22 |
services/database/client_test.go |
22 |
services/cosmosdb/client_test.go |
20 |
services/cache/client_test.go |
20 |
services/synapse/client_test.go |
15 |
services/savingsplans/client_test.go |
5 |
internal/pricing/retail_prices_test.go |
5 |
Go never compiles _test.go into a production binary, so this is not "probably unreachable" — it is unreachable by language rule.
The one shared helper involved, providers/azure/mocks/azure_mocks.go:138 CreateMockHTTPResponse, is an ordinary non-test file and therefore could in principle ship. It does not: every importer of providers/azure/mocks is a _test.go file (verified across the repo). It is also not itself flagged — bodyclose flags the call sites.
2. There is no resource to leak, by construction
Every flagged response is built by one of two shapes, both in-memory:
func CreateMockHTTPResponse(statusCode int, body string) *http.Response {
return &http.Response{
StatusCode: statusCode,
Body: io.NopCloser(bytes.NewBufferString(body)),
Header: make(http.Header),
}
}func fakeResp(status int, body string) *http.Response {
return &http.Response{
StatusCode: status,
Body: io.NopCloser(bytes.NewBufferString(body)),
Header: make(http.Header),
}
}plus local r.Body = io.NopCloser(bytes.NewReader(...)) assignments in compute, managedredis and synapse.
io.NopCloser.Close() returns nil and does nothing, wrapping a bytes.Buffer / bytes.Reader held in memory. There is no socket, no connection, no file descriptor. The condition bodyclose exists to detect — an unclosed body pinning a connection so it cannot be reused or released — cannot occur here.
Reachability classes requested: live request path 0, production error path 0, init-once 0, test-only 193.
Corrected severity
An earlier revision of LeanerCloud/cloud-commitments-cli#1754 described these as "a resource-leak class on a provider that makes real API calls", i.e. connection and file-descriptor exhaustion. That was wrong and is corrected there. It was inferred from the linter's name rather than measured. These are 193 true-positive lint findings and 0 true-positive defects.
They still matter, but only because 193 findings will drown a lint gate when providers/azure is added to it (LeanerCloud/cloud-commitments-cli#1754). That is a reviewability problem, not a reliability one — hence priority/p3.
The genuinely production-code findings in this module and its siblings (49 total: errcheck 18, unused 12, staticcheck 10, errorlint 8, gosec 1) stay in LeanerCloud/cloud-commitments-cli#1754 and should be triaged well ahead of this.
Fix shape and sizing
bodyclose reports at the call site, not the helper, so there is no single place to fix and no annotation that would cover the set.
- Close the bodies at the 193 call sites. Mechanical, no judgement needed, but it adds ~193 lines that close a no-op closer over a memory buffer — noise that a future reader has to re-derive as pointless. Effort:
m, low risk, tedious. - Exclude
bodycloseon_test.goin.golangci.yml. Roughly four lines. Defensible on the merits rather than as suppression: the linter's premise is a real network response, and the file already excludesgocycloon test files for a comparable "this check does not apply here" reason. Effort:xs.
Recommendation: option 2, with the reason written into the config comment so it reads as a scoping decision rather than a silenced gate. It needs a deliberate reviewer sign-off, because the standing rule here is that CI debt does not get gate-suppressed — the distinction being whether a real defect is hidden or a linter is applied outside its domain. The evidence above says the latter, but that call belongs to a reviewer, not to this triage.
Whoever takes it should confirm no production bodyclose findings appear later by re-checking the prod/test split rather than the total.
Verification notes for whoever picks this up
- Any before/after finding comparison must be a set diff (
common normalized, sorted findings), not a total. Equal totals hide an offsetting swap. golangci-lint runmust end at exit 0 with a non-empty findings block before being recorded as clean.--out-formatwas removed in v2 and a concurrent invocation exits 3 — both produce zero findings and read exactly like a clean run.
Related
- LeanerCloud/cloud-commitments-cli#1754 — parent: lint job is root-only, 1291 findings across five unlinted modules; carries the correction to the original severity claim.
- ci: unit and integration test jobs only run the root module — providers/*, pkg and tests/e2e are never tested cloud-commitments-cli#1751 / PR ci: run unit and integration tests in every workspace module cloud-commitments-cli#1755 — the same root-only defect in the test jobs.
providers/azurehas been neither tested nor linted by CI, which is why this sat unseen.
Activity