So this is moderately offensive. We need a way to override the call to the network in our code. Since you can't stub an exported function on a package I use the OverriddenQueryFunction variable to deal with this. Maybe there is a better way?

can't stub an exported function on a package

i thought you could do exactly this? or do you mean the unexported graphQL function?

for that, I remember the book suggesting something along the lines of the answer in this SO post: https://stackoverflow.com/questions/24622388/how-to-test-a-unexported-private-function-in-go-golang

It kind of makes sense that the test and fixtures are in the command package. But I could also see this being in a top level test directory since it is a pretty high level integration test.

Ignore the comment above! I moved the common test code into a test package. That is where test/helper and test/fixtures live now.

Since we use fmt.Printf to print to stdout, I had to capture that. An alternative would be to use fmt.Fprintf and override the file writer in our tests. I didn't go with this because this captureOutput method is pretty simple and gives us one less thing to worry about when we are writing production code.

i like this approach more 👍 given that a lot of different tests in presumably different packages will want to mock out graphql responses specifically, just having it be exported makes sense to me.

this is what i had in mind 👍

🙀

💕

We are testing the Cobra dispatcher this way by going through the root command, but I believe to test individual commands, we should pass that command directly.

I don't think we need to go through the e.g. pr list dispatcher since we already trust that Cobra works.

I tried to do it this way (testing the prCmd instead of root) but ran into problems. You can't set args on a subcommand like this cmd.SetArgs(strings.Split(s, " ")). I dug into the cobra source and I think it is because when you execute a subcommand, it actually runs the root command https://github.com/spf13/cobra/blob/master/command.go#L873-L876. So I gave up and just used the root command.

One positive aspect of testing the root command is we ensure that the subcommand was added to the root command ¯_(ツ)_/¯

I was wondering where this data comes, and then later I saw the pr.json fixture. Do you think there might be benefit to inlining the mock JSON payload in the tests themselves instead of keeping them in a separate file? That way, somebody who reads or authors a test can see the API payload and the expected output in the same file.

In theory I like your idea! I'll put it inline and see if it's helpful or annoying.

Ok, I still like the core of your idea, but the problem I ran into was I lost the json syntax coloring when I made it a string. I really want this because I use syntax coloring as a linter when I edit json. If you have a way around this I'd be 😃!

So instead I made the MockGraphQLResponse function take the entire fixture path. At least this will make it more clear where the data is coming from.

Something I was thinking about re: mocking HTTP responses:

What if, instead of replacing the whole GraphQL function, we make it so we can inject a custom RoundTripper into our HTTP client? https://golang.org/pkg/net/http/#RoundTripper

Then, the tests could set up that roundtripper to return pre-baked responses from memory (or from file) rather than activating the actual HTTP request. Just an idea! If we wanted to try that approach, it might make more sense to do it in a followup PR. 👍

That's interesting, this RoundTripper thing might work well. Are you thinking this is beneficial because we can test our response to HTTP status codes instead of just assuming we always get 200's back?