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

How to set URL parameters for unit tests? #167

Closed
bhirbec opened this Issue Jun 2, 2016 · 13 comments

Comments

Projects
None yet
6 participants
@bhirbec

bhirbec commented Jun 2, 2016

Hi,

I need to set up URL parameters for my unit tests. I'm using context.Set() to do so and my test looks something like this:

context.Set(r, 0, map[string]string{"param1": "123"})
req , _ := http.NewRequest()
w = := httptest.NewRecorder()
myhandler(w, req)

The HTTP handler uses mux.Vars() to retrieve the parameters but it doesn't get any. The problem is explained in great details in this post:

Trouble is, even when you use 0 as value to set context values, it is not same value that mux.Vars() reads. mux.Vars() is using varsKey (as you already saw) which is of type contextKey and not int.

Sure, contextKey is defined as:

type contextKey int

which means that it has int as underlying object, but type plays part when comparing values in go, so int(0) != contextKey(0).

I'm aware I could solve my problem by either starting a web server or by calling the mux directly (as mentioned in SO). However, I don't want to do so. I'd like to call the handler directly in my test.

I see two options to solve this:

  • add a mux.SetVar() method to explicitly set URL parameters for a given request (there's already a private setVar() method)
  • use int instead of contextKey type when defining varsKey. This way context.Set(r, 0, map[string]string{"param1": "123"}) would be a valid way to set up URL parameters.

Hope this help. Thanks for making Gorilla :)

Ben

@elithrar

This comment has been minimized.

Show comment
Hide comment
@elithrar

elithrar Jun 3, 2016

Member

Note: also discussed in #112

The short answer: using https://golang.org/pkg/net/http/httptest/#Server + setting server.URL = /your/test/url is the right way to test this. You can still test multiple handlers, use table driven tests for testing different route variables, etc - without tying your tests to mux's implementation.

The long answer: changing the type of contextKey to int would cause collisions with other packages also using the request context. The gorilla/context docs cover this. Making the key type public would also add to the API and prevent us from ever changing things.

Member

elithrar commented Jun 3, 2016

Note: also discussed in #112

The short answer: using https://golang.org/pkg/net/http/httptest/#Server + setting server.URL = /your/test/url is the right way to test this. You can still test multiple handlers, use table driven tests for testing different route variables, etc - without tying your tests to mux's implementation.

The long answer: changing the type of contextKey to int would cause collisions with other packages also using the request context. The gorilla/context docs cover this. Making the key type public would also add to the API and prevent us from ever changing things.

@elithrar elithrar added the question label Jun 3, 2016

@elithrar elithrar self-assigned this Jun 3, 2016

@bhirbec

This comment has been minimized.

Show comment
Hide comment
@bhirbec

bhirbec Jun 3, 2016

@elithrar thanks for the reply. I'm not sure to understand your solution. How does the context get populated if the mux isn't involved? Can you please provide more informations or point to an example?

bhirbec commented Jun 3, 2016

@elithrar thanks for the reply. I'm not sure to understand your solution. How does the context get populated if the mux isn't involved? Can you please provide more informations or point to an example?

@elithrar

This comment has been minimized.

Show comment
Hide comment
@elithrar

elithrar Jun 3, 2016

Member

@bhirbec You populate the context by passing the correct URL - e.g.

    r := mux.NewRouter()
    r.HandleFunc("/hello/{name}", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
        fmt.Fprintln(w, "Hello, client")
    }))

    ts := httptest.NewServer(r)
    defer ts.Close()

    // Table driven test
    names := []string{"kate", "matt", "emma"}
    for _, name := range names {
        url := ts.URL + "/hello/" + name
        resp, err := http.Get(url)
        if err != nil {
            t.Fatal(err)
        }

        if status := resp.Code; status != http.StatusOK {
            t.Fatalf("wrong status code: got %d want %d", status, http.StatusOK)
        }
    }

This way—regardless of whether you use gorilla/mux or not—you're still testing that your application handles the route variables you're passing.

Member

elithrar commented Jun 3, 2016

@bhirbec You populate the context by passing the correct URL - e.g.

    r := mux.NewRouter()
    r.HandleFunc("/hello/{name}", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
        fmt.Fprintln(w, "Hello, client")
    }))

    ts := httptest.NewServer(r)
    defer ts.Close()

    // Table driven test
    names := []string{"kate", "matt", "emma"}
    for _, name := range names {
        url := ts.URL + "/hello/" + name
        resp, err := http.Get(url)
        if err != nil {
            t.Fatal(err)
        }

        if status := resp.Code; status != http.StatusOK {
            t.Fatalf("wrong status code: got %d want %d", status, http.StatusOK)
        }
    }

This way—regardless of whether you use gorilla/mux or not—you're still testing that your application handles the route variables you're passing.

@elithrar

This comment has been minimized.

Show comment
Hide comment
@elithrar

elithrar Aug 1, 2016

Member

Closing this out for now. Re-open if any questions!

Member

elithrar commented Aug 1, 2016

Closing this out for now. Re-open if any questions!

@bkeroackdsc

This comment has been minimized.

Show comment
Hide comment
@bkeroackdsc

bkeroackdsc Aug 2, 2017

@elithrar For the love of all that is holy, please just expose a SetVars function. The implementation behind it will be opaque so it preserves separation of concerns, and it will allow us to test our handlers in isolation without having to spin up an entire HTTP server just to test a single function!

bkeroackdsc commented Aug 2, 2017

@elithrar For the love of all that is holy, please just expose a SetVars function. The implementation behind it will be opaque so it preserves separation of concerns, and it will allow us to test our handlers in isolation without having to spin up an entire HTTP server just to test a single function!

@astromahi

This comment has been minimized.

Show comment
Hide comment
@astromahi

astromahi Aug 8, 2017

It would be great help, if you guys export the setVars. It forcing me to do functional testing.

func setVars(r *http.Request, val interface{}) *http.Request {
	return contextSet(r, varsKey, val)
}

astromahi commented Aug 8, 2017

It would be great help, if you guys export the setVars. It forcing me to do functional testing.

func setVars(r *http.Request, val interface{}) *http.Request {
	return contextSet(r, varsKey, val)
}
@vincent6767

This comment has been minimized.

Show comment
Hide comment
@vincent6767

vincent6767 Jan 22, 2018

Any update on this?

vincent6767 commented Jan 22, 2018

Any update on this?

@elithrar

This comment has been minimized.

Show comment
Hide comment
@vincent6767

This comment has been minimized.

Show comment
Hide comment
@vincent6767

vincent6767 Jan 22, 2018

Yep. I have just seen them several hours ago. Thank you!

vincent6767 commented Jan 22, 2018

Yep. I have just seen them several hours ago. Thank you!

@bkeroackdsc

This comment has been minimized.

Show comment
Hide comment
@bkeroackdsc

bkeroackdsc commented Jan 22, 2018

Thanks @elithrar !

@palsivertsen

This comment has been minimized.

Show comment
Hide comment
@palsivertsen

palsivertsen Jan 25, 2018

Handy function for testing!
testing-handlers section in Readme could use this update

palsivertsen commented Jan 25, 2018

Handy function for testing!
testing-handlers section in Readme could use this update

@elithrar

This comment has been minimized.

Show comment
Hide comment
@elithrar

elithrar Jan 25, 2018

Member

@palsivertsen Would you like to PR that? :)

Member

elithrar commented Jan 25, 2018

@palsivertsen Would you like to PR that? :)

@palsivertsen

This comment has been minimized.

Show comment
Hide comment
@palsivertsen

palsivertsen commented Jan 26, 2018

@elithrar Sure. See pull #342

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment