Comments (11)
I like this idea and I think we should do it. I already have migrated it to go-gh so it really just needs to be moved from the internal folder to export it.
Before making it widely available there are a couple changes I would like to see to the interface while we still can:
- Should the name change to something like
TransportMock
orRoundTripperMock
? - Stubs should have names when they are registered with
Register
. This will allow for better error messaging directly referencing the stub name. - We should remove the more obscure responders that are not used in
go-gh
, for exampleFileResponse
andScopesResponder
. Perhaps there are others there that we can remove.
These will require changes to the go-gh
test suit so I think approaching it as making the interfaces changes first and then exporting the package in a second PR is the right way to go here.
from go-gh.
- I like
TransportMock
a bit better. Seems more obvious and discoverable. - 👍
- While we're at it, would be nice to define a pattern for taking a string vs. object or map to marshal. I had started some changes like this for one of my recent PRs since I already had an object in test I could marshal, and construction of it is more straight forward in some cases. Certainly just a preformatted JSON response string in other cases is great. For HTTP this already exists, but something similar for GraphQL would be nice. Maybe some pattern like
{protocol}{"Request"|"Response"}({"String"|"JSON"})
e.g., "HTTPRequest", "HTTPResponseString", "GraphQLRequest", or "GraphQLResponseJSON".
from go-gh.
Great, TransportMock
it is.
I also had thoughts on defining a naming scheme for responders as well. I think our responders can be smarter than they currently are. In my opinion each should be able to handle both a string and a struct, if it is not a string we will try to marshal it to a JSON string. In this scenario we don't need to figure out a naming scheme for what takes in a string or a struct. I wrote up some code below on how I am envisioning the responders. Let me know what you think.
func HTTPResponse(status int, header map[string][]string, body interface{}, cb func(*http.Request)) Responder {
return func(req *http.Request) (*http.Response, error) {
var b io.Reader
if s, ok := body.(string); ok {
b = bytes.NewBufferString(s)
} else {
s, _ := json.Marshal(body)
b = bytes.NewBuffer(s)
}
if cb != nil {
cb(req)
}
return httpResponse(status, header, b, req), nil
}
}
func RESTResponse(body interface{}, cb func(map[string]interface{})) Responder {
return func(req *http.Request) (*http.Response, error) {
var b io.Reader
if s, ok := body.(string); ok {
b = bytes.NewBufferString(s)
} else {
s, _ := json.Marshal(body)
b = bytes.NewBuffer(s)
}
bodyData := make(map[string]interface{})
err := decodeJSONBody(req, &bodyData)
if err != nil {
return nil, err
}
if cb != nil {
cb(bodyData)
}
return httpResponse(200, map[string][]string{}, b, req), nil
}
}
func GQLMutation(body interface{}, cb func(map[string]interface{})) Responder {
return func(req *http.Request) (*http.Response, error) {
var b io.Reader
if s, ok := body.(string); ok {
b = bytes.NewBufferString(s)
} else {
s, _ := json.Marshal(body)
b = bytes.NewBuffer(s)
}
var bodyData struct {
Variables struct {
Input map[string]interface{}
}
}
err := decodeJSONBody(req, &bodyData)
if err != nil {
return nil, err
}
if cb != nil {
cb(bodyData.Variables.Input)
}
return httpResponse(200, map[string][]string{}, b, req), nil
}
}
func GQLQuery(body interface{}, cb func(string, map[string]interface{})) Responder {
return func(req *http.Request) (*http.Response, error) {
var b io.Reader
if s, ok := body.(string); ok {
b = bytes.NewBufferString(s)
} else {
s, _ := json.Marshal(body)
b = bytes.NewBuffer(s)
}
var bodyData struct {
Query string
Variables map[string]interface{}
}
err := decodeJSONBody(req, &bodyData)
if err != nil {
return nil, err
}
if cb != nil {
cb(bodyData.Query, bodyData.Variables)
}
return httpResponse(200, map[string][]string{}, b, req), nil
}
}
func httpResponse(status int, header map[string][]string, body io.Reader, req *http.Request) *http.Response {
if _, ok := header["Content-Type"]; !ok {
header["Content-Type"] = []string{"application/json; charset=utf-8"}
}
return &http.Response{
StatusCode: status,
Header: header,
Body: io.NopCloser(body),
Request: req,
}
}
from go-gh.
Overall, I like it, but have a couple thoughts:
- It's not always true that a REST response returns 200. Sure, someone could use the HTTP response (which are mostly RESTful anyway), but maybe don't imply a format protocol through HTTP or REST. Maybe HTTPResponse or HTTPResponseWithStatus?
- For
RESTResponse
, couldn't you avoid the performance hit and memory (granted, they're tests, so maybe 🤷♂️) by only decoding a string body and just using the given body as thebodyData
otherwise?
from go-gh.
- True. I am open to naming changes this was just quickly what I put together. I chose the current names as they matched the various clients that
go-gh
offers. - That could be a good improvement, although the
cb
function signature would have to change to take aninterface{}
argument which I am hesitant about.
from go-gh.
@heaths As part of the process of exporting httpmock
we did an investigation on other http mocking packages out there and the team concluded that moving forward with using the gock
package was the right approach for us instead of continuing using our home made httpmock
package. With that decision, we are also advocating using gock
or another third-party package for http mocking when testing gh
extensions so I am going to close this issue out.
from go-gh.
@samcoe having had a change to use gock
in https://github.com/heaths/gh-projects, there are some positive aspects (simple to set up and no hooks needed if using net/http
), but some areas that could be improved - maybe even here or in a separate package. Or just contributed back to gock
- something I'm considering but would love to brainstorm ideas.
The main issue that is lacking is trying to match a request. With your httpmock
you can match a GraphQL request with just a substring - like a query name, which I do for this very purpose - but then a callback (or any way to assert) for variables. In gock
all I've found is gock.Request.Body
or gock.Request.BodyString
which requires getting the query exactly right - newlines and all - and the variables sorted, though I guess the latter is document and cannot reasonably be changed.
Still, it'd be nice to have a way to not have to encode the entire query. Thoughts?
from go-gh.
@heaths You're right that matching on the entire query string would likely lead to brittle tests. Would it be possible to define a custom Request matcher with gock that extracts the GraphQL query name and matches on that?
from go-gh.
Seems one could using Request.AddMatcher
. Perhaps a function could be added to go-gh that takes an operation name and variable map and returns a MatchFunc
.
from go-gh.
@heaths I am not sure that we necessarily need to include this functionality in go-gh
. I
went ahead and wrote a function that does what you want and it turned out to be only ~30 lines which seems like something an extension author could implement themselves. The only tricky part which isn't exactly clear is using the TeeReader
for reading the body into a copy buffer so that any subsequent matchers can also read the body if they need to.
func matchGQLRequest(query string, vars map[string]string) func(*http.Request, *gock.Request) (bool, error) {
return func(req *http.Request, ereq *gock.Request) (bool, error) {
var bodyData struct {
Query string
Variables map[string]string
}
bodyCopy := &bytes.Buffer{}
r := io.TeeReader(req.Body, bodyCopy)
req.Body = io.NopCloser(bodyCopy)
b, err := io.ReadAll(r)
if err != nil {
return false, err
}
err = json.Unmarshal(b, &bodyData)
if err != nil {
return false, err
}
if query != "" && query != bodyData.Query {
return false, nil
}
if len(vars) != 0 {
for k, v := range vars {
bv := bodyData.Variables[k]
if v != bv {
return false, nil
}
}
}
return true, nil
}
}
It can be used like this:
func TestGQLClient(t *testing.T) {
stubConfig(t, testConfig())
t.Cleanup(gock.Off)
gock.New("https://api.github.com").
Post("/graphql").
MatchHeader("Authorization", "token abc123").
AddMatcher(matchGQLRequest("QUERY", map[string]string{"var": "test"})).
Reply(200).
JSON(`{"data":{"viewer":{"login":"hubot"}}}`)
client, err := GQLClient(nil)
assert.NoError(t, err)
vars := map[string]interface{}{"var": "test"}
res := struct{ Viewer struct{ Login string } }{}
err = client.Do("QUERY", vars, &res)
assert.NoError(t, err)
assert.True(t, gock.IsDone(), printPendingMocks(gock.Pending()))
assert.Equal(t, "hubot", res.Viewer.Login)
}
from go-gh.
It was just an idea to avoid everyone having to figure that out for themselves. Perhaps dubious, but a helper function in go-gh to make it more discoverable and easier for people to use seems like a good idea. I wasn't afraid I wouldn't figure something out, just that perhaps not everyone needs to do so as well. 🙂
from go-gh.
Related Issues (20)
- Add pagination examples
- jsonpretty should not add new lines if not indenting
- TokenForHost "authentication token not found" message could be more informative HOT 3
- Feature Request: Add support to call the `gh` command interactively HOT 3
- RestClient no longer returns an HTTPError HOT 2
- Expose `ghLookupPath` or a better way to run `gh` HOT 3
- Issue while using from remote connection HOT 6
- `asciisanitizer.Sanitizer` mishandled the `�` unicode character HOT 1
- Add the ability to introduce repository specific configurations for gh-cli and its extensions HOT 2
- Create CODEOWNERS file ensuring the cli/code-reviewers are assigned to PRs
- CVE Vulnerability CVE-2023-3978 in dependency HOT 1
- hyperlink truncated at end of row isn't closed correctly HOT 2
- Use retryable HTTP client (for 50x) errors
- Allow default host to be determined based on configuration preference HOT 5
- Rest client support for endpoint not returning a JSON HOT 3
- AlecAivazis/survey is archived HOT 3
- Support passing httptest URLs in api.ClientOptions HOT 1
- [pkg/cmd/root/root.go](url) HOT 1
- GQLCLient should return GQLError for query and mutation methods
- Add a GraphQL Mutation with an `input` object to examples HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from go-gh.