Skip to content
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

Jump to bottom

net/http/httptest: make easier way to create Requests for testing #14915

Closed
encryptio opened this issue Mar 22, 2016 · 3 comments
Closed

net/http/httptest: make easier way to create Requests for testing #14915

encryptio opened this issue Mar 22, 2016 · 3 comments
Labels
FrozenDueToAge

Comments

@encryptio
Copy link
Contributor

When http.NewRequest is called, it does not fill in (http.Request).RequestURI, but http.Server does fill in (http.Request).RequestURI, which has caused issues writing tests that use a httptest.ResponseRecorder instead of a httptest.Server.

A test case:

package issue

import (
    "net/http"
    "net/http/httptest"
    "testing"
)

func TestNewRequestRequestURI(t *testing.T) {
    var httpServerRequestURI string
    h := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
        httpServerRequestURI = r.RequestURI
    })

    srv := httptest.NewServer(h)
    defer srv.Close()

    req, err := http.NewRequest("GET", srv.URL+"/path", nil)
    if err != nil {
        t.Fatal(err)
    }

    newRequestRequestURI := req.RequestURI

    resp, err := http.DefaultClient.Do(req)
    if err != nil {
        t.Fatal(err)
    }
    resp.Body.Close()

    if newRequestRequestURI != httpServerRequestURI {
        t.Fatalf("NewRequest().RequestURI = %#v, but sending to server yielded %#v",
            newRequestRequestURI, httpServerRequestURI)
    }
}

Which yields:

--- FAIL: TestNewRequestRequestURI (0.00s)
    test_test.go:33: NewRequest().RequestURI = "", but sending to server yielded "/path"
FAIL
exit status 1
FAIL    issue   0.004s

On go version go1.6 linux/amd64 and go version devel +c12e1b0 Tue Mar 22 17:09:29 2016 +0000 linux/amd64.
@minux
Copy link
Member

minux commented Mar 22, 2016

Working as intended (and documented).

The RequestURI field is not supposed to be set on client requests.
Quote https://golang.org/pkg/net/http/#Request:

    // RequestURI is the unmodified Request-URI of the
    // Request-Line (RFC 2616, Section 5.1) as sent by the client
    // to a server. Usually the URL field should be used instead.
    // It is an error to set this field in an HTTP client request.
    RequestURI string

And NewRequest is not designed to create server Requests:
Quote https://golang.org/pkg/net/http/#NewRequest

    NewRequest returns a Request suitable for use with Client.Do
    or Transport.RoundTrip. To create a request for use with testing
    a Server Handler use either ReadRequest or manually update the
    Request fields. See the Request type's documentation for the
    difference between inbound and outbound request fields.

@minux minux closed this as completed Mar 22, 2016
@bradfitz
Copy link
Contributor

For Go 1.7, I do want to make a new httptest helper to make it easier to create a Request for testing, though. I think there's already an open bug, but just in case I'll re-open this one and assign it to me.

@bradfitz bradfitz reopened this Mar 22, 2016
@bradfitz bradfitz self-assigned this Mar 22, 2016
@bradfitz bradfitz changed the title net/http: NewRequest().RequestURI is not set net/http/httptest: make easier way to create Requests for testing Mar 22, 2016
@cespare
Copy link
Contributor

cespare commented Mar 22, 2016

@bradfitz #14199 is the existing issue. Probably okay to leave this one closed.

@golang golang locked and limited conversation to collaborators Mar 23, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@bradfitz @encryptio @minux @cespare @gopherbot