Open csilvers opened 3 years ago
Thanks for the thoughts! I'm gonna put some time on the calendar with you and @dnerdy to think through some of this synchronously. In the meantime, some less-organized thoughts.
So first of all, I definitely support open-sourcing our mocking framework. (And possibly also gqltest!) I think there will be some work to do to make the wiring less Khan-specific, but it definitely seems worthwhile. One question is if it belongs in genqlient or in a separate project; that may depend on how much we want to integrate the two.
My next question is: how would your MockMyquery
differ from our existing tooling, other than obviously it would autogenerate things like the query-name to be matched? It's not clear to me what the advantage is -- and thus it's not clear to me what the benefit of combining the two is. (There's a cost, of course, to making these things open-source when we may not have the right API yet.) You mention validation, but I feel the code we have does that plenty well; I don't know that genqlient does any better. (I guess the generated helper can do the validation at mock-time instead of waiting for the query to be used? But that doesn't need to be one generated by genqlient; it could be a separate generator. Obviously some overhead there.)
Another thing that I wonder is how much this solves for the problems we're actually having. It seems to me like most of the problems were not with the wiring or validation, per se, but with what goes inside that js.Obj
. This leads me to a variant idea to attack that problem specifically.
The issue as I see it is that both js.Obj
and genqlient have drawbacks when it comes to mocking. For js.Obj
, the problem is the obvious lack of typing; the caller may have to know what fields to include based on what the query looks like, and there's nothing until runtime to check that. But on the other hand, genqlient isn't really intended for this and its types are cumbersome for a human to construct. Mixing the two is even worse: you neither get help from the compiler for constructing the types correctly, nor the flexibility and simplicity of the untyped approach.
The realization I had today is that on some level both problems come out of the fact that we're using client-driven APIs to build what is really a mock server. Because of the request-what-you want nature of GraphQL, servers and clients have quite different views of the types: servers get to use the simple types adapted directly from the schema, whereas clients need types with only the fields they want. (For example, compare gqlgen's generated types to genqlient's.) What if instead we use server-style types for mocking? Then we'd no longer have any of the "which fields do you use" problems; the mocking tools would just return the types the client asked for (and, if applicable, __typename
). And meanwhile we wouldn't need all this complicated system of embedding, interfaces, etc.; we'd just have a single type with all the fields.
If that sounds like gqlgen, that's because it is! (Concretely, it's like running gqlgen on your (composed) client schema, rather than just your service's (server) schema, and using that for mocks.) But I think we could do a few things differently if our goal is mocking:
Anyway, that would be a significant new project. (Honestly, I think any variation on this will be.) It doesn't have to be in genqlient; it could just read in genqlient's exported-operations file. (Arguably it could even be in gqlgen! Although in practice genqlient may be a more natural home.)
Quick summary of our meeting:
shurcooL
response structs, but now test-only) and it's a bit of duplication but oh welljs.Obj
is realistic for theseThe most popular HTTP mocking library in Go is https://github.com/jarcoal/httpmock because it offers quite a lot of convenient features. However, since every operation in GraphQL is a POST, URL munging (adding the operation name to the URL) would be necessary to disambiguate different GraphQL HTTP requests (query vs mutation) to provide the expected response. Since at Khan we use a URL munging adapter that is not currently included in genqlient, I would expect OSS community consumers of genqlient would need some alternative routing between GraphQL requests and response expectations to mock things.
For reference, this is how to use jarcoal/httpmock
:
httpmock.Reset()
httpmock.RegisterResponder(
"GET",
"https://app.terraform.io/api/v2/workspaces/workspaceId/current-state-version",
httpmock.NewBytesResponder(http.StatusOK, []byte(`{"data":{"attributes":{"hosted-state-download-url":"https://archivist.terraform.io/v1/object/test"}}}`)),
)
httpmock.RegisterResponder(
"GET",
"https://archivist.terraform.io/v1/object/test",
httpmock.NewBytesResponder(http.StatusOK, []byte(`{}`)),
)
While this is a somewhat older issue, I would also like to have a way to test my GraphQL database calls in some way. One thought I had was that, if all the generated Go functions for queries, mutations, etc. would be able to have a receiver that I could specify, and an interface be defined as well. I could then mock that interface.
Perhaps this is not the best way, but it feels like given an interface for my client code to use rather than a method without a receiver, I would have a lot of options to write a solid test framework that could either replay actual queries and responses, or I could just hard-code responses for tests.
@skandragon I just started using genqlient and wanted something like this as well to be able to decouple the RPC from the domain logic and ended up wrapping the generated code in a very slim wrapper. This way I can depend on this one interface and mock various behavior upstream without worrying about RPC. The only downside is that this needs to be updated by hand every time the generated code changes. However, it's pretty trivial to test this in isolation with a custom HTTP transport that can assert the request / response fields.
type Client interface {
GetEnvironments(context.Context, *EnvironmentFilterSpec, *Pagination) (*GetEnvironmentsResponse, error)
// ...
}
type Doer interface {
Do(r *http.Request) (*http.Response, error)
}
type client struct {
gqlClient graphql.Client
}
func NewGraphQLClient(endpoint string, doer Doer) Client {
return client{
gqlClient: graphql.NewClient(endpoint, doer),
}
}
func (c client) GetEnvironments(ctx context.Context, filter *EnvironmentFilterSpec, pagination *Pagination) (*GetEnvironmentsResponse, error) {
// Call the generated genqlient code
return GetEnvironments(ctx, c.gqlClient, filter, pagination)
}
The fezzik graphql generator does this already (see https://github.com/inigolabs/fezzik/blob/3650ea733e4e65a48bb3a4b1b092228d67362676/examples/github/gen/github/client.go#L12)
Interesting, I guess doing methods of a struct is a thing we could do. My guess is it doesn't work so well for large codebases* but as long as it's optional that's not necessarily a problem. I think there is probably a little bit of design to work out:
graphql.Client
, or do the methods still accept one?But it's probably not hard to implement; it's just an option and then some conditionals in the template. (Unlike the original suggestion in this thread, it's a very client-style API so we don't have to reimplement half of gqlgen.) Feel free to give it a go!
*Specifically, having all your GraphQL queries hanging off one struct means they all have to live together, rather than living closer to their callers.
My thought here is that for every function
Myquery
that genqlient produces code for, it also produces a functionMockMyquery
, that can be used in tests. But we do away with strong typing altogether!, and use the js.Obj system from Khan Academy. The justification for this is that, unlike for prod code, it's ok to do type-checking at runtime for tests. And specifying js.Obj{} objects is just much nicer to write than all the super-long type-names that would be provided if we specified Go types.Specifically, my thought is the API might be: MockMyquery(ctx context.Context, testClient graphql.Client, varsToMatch js.Obj, output js.Obj) and this would add a mux to testClient so whenever you queried that client on Myquery with args that match
varsToMatch
, the client would returnoutput
. (Alternately, we could have you pass in a mux yourself, and you could attach it to your test-client however you want; that matches the httplib model better and may be easier or harder to use, I'm not sure.)The logic of MockMyquery() would include code to validate the schema: to make sure that varsToMatch includes all required vars and they can all be coerced to the proper type, and likewise that the output matches the query's schema.
varsToMatch could include all the js matchers that we have at Khan, such as
js.UnorderedArray{}
. It could also include a newjs.Any
to match anything. (Because leaving a required var out of varsToMatch would be an error, rather than the existing Khan Academy logic where it's an implicit js.Any.) You could just pass injs.Any
directly forvarsToMatch
to mean "return the given output for all Myquery queries."I am thinking the codegen would also just create the test helper files to a package generated/genqlient/js/. So clients could import that and use js.Obj/etc like they do at Khan Academy already.
genqlient.tar.gz holds the relevant files we use at Khan Academy: