Public test API to set URL params#322
Merged
kisielk merged 3 commits intogorilla:masterfrom Dec 8, 2017
Merged
Conversation
elithrar
suggested changes
Dec 7, 2017
testing.go
Outdated
|
|
||
| import "net/http" | ||
|
|
||
| // TestSetURLParam set url params |
Contributor
There was a problem hiding this comment.
Could you expand on this? Why it exists, what the normal way of setting params is; URL should also be capitalized.
Contributor
Author
There was a problem hiding this comment.
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.
Contributor
There was a problem hiding this comment.
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.
Contributor
Author
There was a problem hiding this comment.
I just wrote something but yours is nicer. Thanks again.
Contributor
|
I think a better name is needed too. Starting it with |
Contributor
Author
added 2 commits
December 7, 2017 14:36
…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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.goto 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.