Skip to content

Parallelism-safe mechanism for stubbing HTTP responses#874

Merged
mislav merged 3 commits into
masterfrom
httpmock
May 8, 2020
Merged

Parallelism-safe mechanism for stubbing HTTP responses#874
mislav merged 3 commits into
masterfrom
httpmock

Conversation

@mislav

@mislav mislav commented May 6, 2020

Copy link
Copy Markdown
Contributor

This adds a thread-safe RoundTripper that can be used for mocking HTTP requests in tests. Incoming requests are matched based on their contents, not the order the stubs were registered in. This ensures that the tests keep working even if the requests themselves were parallelized and their order isn't guaranteed.

This was loosely modeled after https://pkg.go.dev/github.com/jarcoal/httpmock which I was evaluating for potential use in tests, but that one was designed for RESTful APIs and didn't look like it could handle looking inside GraphQL requests in the matching phase.

Needed for #787

mislav added 2 commits May 6, 2020 22:03
This adds a thread-safe RoundTripper that can be used for mocking HTTP
requests in tests. Incoming requests are matched based on their
contents, not the order the stubs were registered in.
@mislav mislav requested review from probablycorey and vilmibm May 6, 2020 20:13
Comment thread api/queries_repo.go Outdated
return result, nil
}

func RepoNetworkStubResponse(owner, repo, permission string) string {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This moves the non-trivial response stub more closer to the actual implementation so it can be kept in sync with RepoNetwork.

Comment thread command/pr_create_test.go
{ "data": { "repository": { "forks": { "nodes": [
] } } } }
http := initMockHTTP()
defer http.Verify(t)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional: once the test is done, verify that all response stubs were matched and fail the test otherwise

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a nice idea but I don't mind if you want to do it later

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, it's done! This demonstrates its use. It's an added level of assertion; I've only indicated that this line is "optional" because the test case would have passed without it too

Comment thread command/pr_create_test.go Outdated
] } } } }
http := initMockHTTP()
defer http.Verify(t)
http.Register(httpmock.GraphQL(`\bviewerPermission\b`), httpmock.StringResponse(api.RepoNetworkStubResponse("OWNER", "REPO", "WRITE")))

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incoming requests are now matched by their GraphQL query contents; this simple regexp is used to determine that this query is detecting the base/head repository, and we return a stubbed RepoNetwork response

Comment thread command/pr_create_test.go
Comment on lines +427 to +428
`, func(inputs map[string]interface{}) {
eq(t, inputs["repositoryId"], "REPOID")

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For GraphQL mutations we can opt to receive a parsed map of input variables which we can then check without having to repeat the JSON-parsing boilerplate.

Comment thread command/testing.go Outdated
return http
}

func initMockHTTP() *httpmock.Registry {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This supersedes initFakeHTTP, which was the obsolete sequential stubber

@vilmibm vilmibm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is great!

Comment thread command/pr_create_test.go
{ "data": { "repository": { "forks": { "nodes": [
] } } } }
http := initMockHTTP()
defer http.Verify(t)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a nice idea but I don't mind if you want to do it later

@mislav mislav merged commit 3000847 into master May 8, 2020
@mislav mislav deleted the httpmock branch May 8, 2020 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants