grafana / grafana-openapi-client-go

Grafana OpenAPI Client for Go
Apache License 2.0
38 stars 6 forks source link

Assertion: http.DefaultTransport.(*http.Transport) ... makes mocking impossible #94

Open sylnsr opened 1 month ago

sylnsr commented 1 month ago

Regarding https://github.com/grafana/grafana-openapi-client-go/blob/3ad0f7e4ee52421b606af2486d7bccd7a0776ae6/client/grafana_http_api_client.go#L416

I was using github.com/jarcoal/httpmock with the old (now deprecated) API client our app. We have a test suite must run mocking tests before running integration tests as the next step. Upon upgrading to the this new OpenAPI client, I discovered that at this point, we're running into a panic caused by that line of code:

interface conversion: net/http.RoundTripper is *github.com/jarcoal/http.MockTransport, 
not *net/http.Transport

It seems like an overly opinionated or short sighted requirement, that the transport MUST be *net/http.Transport. For one thing this breaks mocking. Looking at it another way, whats the point of allowing the user to set TransportConfig if down the line you're making an assertion that its a specific thing?

sylnsr commented 1 month ago

On a side note, for any one else running into this, the work around for the issue with that particular mocking framework is not to use httpmock.Activate() to activate the framework, but instead to use httpmock.ActivateNonDefault(&http.Client{}). (Correction, there is no solution)

However, that should not detract from the larger issue .. I still think the issue for taking a custom TransportConfig, then asserting its something specific, is misleading.