-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Public test API to set URL params #322
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better docs 😉
testing.go
Outdated
|
||
import "net/http" | ||
|
||
// TestSetURLParam set url params |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you expand on this? Why it exists, what the normal way of setting params is; URL should also be capitalized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow thanks for a fast response! 😃 I was just thinking about that myself. I'm just looking up some other concrete examples for this sort of thing and then I'll reword it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion:
// TestSetURLParam sets the URL params for the given request, to be accessed via mux.Vars for testing route behaviour.
//
// This API should only be used for testing purposes; it provides a way to inject variables into the request context.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just wrote something but yours is nicer. Thanks again.
I think a better name is needed too. Starting it with |
…e approach to setting url vars. * rename SetURLParams to SetURLVars as this is more descriptive. * rename testing to testing_helpers as this is more descriptive.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m OK with this now. @kisielk?
As discussed in #233, it will be useful to have a test focused API to set URL params. Currently this is not possible because the route key type used to store the vars in the context is private and the setVars function is also private.
Currently there are two work arounds to address this, the first being to make pull the vars extraction up in to the middleware layer, which requires a significant refactor and the other is to spin up a server and running the request through a route to populate the context with the vars. The problem with this is that it's very heavy handed and for those running tests in hosted CI environments like travis, the extra bloat can slow down test execution potentially to the point where testing handlers no longer becomes viable.
This should allow for the testing of routes and route handlers to be done separately while also preserving the current level of indirection that has allowed for the underlying context implementation to be changed (as seen in this PR).
The added public API end point is prefixed with "Test" and can be found in
testing.go
to emphasise that this is a utility specifically meant for testing rather than in normal program flow. This notion of testing as a public API is borrowed from this slide Advanced Testing with Go.