Support passing httptest URLs in api.ClientOptions
- Dominant language
- Go
- Stars
- 438
- Forks
- 94
- Avg merge
- 5d 13h
- Merged PRs (30d)
- 6
Description
The standard library's [httptest](https://pkg.go.dev/net/http/httptest) package makes stubbing APIs easy, if the client calls the test server's URL:
```go
package main
import (
"fmt"
"net/http"
"net/http/httptest"
)
func main() {
tls := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}))
defer tls.Close()
fmt.Println("tls hostname - ", tls.URL)
// prints: tls hostname - https://127.0.0.1:42002
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}))
defer ts.Close()
fmt.Println("ts hostname - ", ts.URL)
// prints: ts hostname - http://127.0.0.1:35061
}
```
In api.ClientOptions, one of the configuration fields is `Host` https://github.com/cli/go-gh/blob/25db6b99518c88e03f71dbe9e58397c4cfb62caf/pkg/api/client_options.go#L38
for certain values the argument is returned unmodified https://github.com/cli/go-gh/blob/25db6b99518c88e03f71dbe9e58397c4cfb62caf/pkg/api/rest_client.go#L151-L154
I would like to propose adding another condition to the `restURL` function which would return the unmodified `httptest` server URL
```go
if strings.HasPrefix(hostname, "https://127.0.0.1:") || strings.HasPrefix(hostname, "http://127.0.0.1:") {
return hostname
}
```
This aligns with the conditional check on the prefix for the `pathOrURL` argument.
For reference, setting the `ts.URL` value to `Host` results in a URL similar to:
>https://http//127.0.0.1:56261/api/v3/
as it satisfies this condition https://github.com/cli/go-gh/blob/25db6b99518c88e03f71dbe9e58397c4cfb62caf/pkg/api/rest_client.go#L163
I don't know if this means my proposed solution would break with Enterprises.
In writing this all out, I also realized that passing the hostname + path to `client.Get`, rather than just the api path, solves this issue.
Contributor guide
Assessment
This issue has not been assessed yet.