code review 7314046: net/http/httptest: add examples

net/http/httptest: add examples

[kisielk, 2013-02-06 05:38:24]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go/

[adg, 2013-02-07 22:21:43]

Nice examples

https://codereview.appspot.com/7314046/diff/4001/src/pkg/net/http/httptest/ex...
File src/pkg/net/http/httptest/example_test.go (right):

https://codereview.appspot.com/7314046/diff/4001/src/pkg/net/http/httptest/ex...
src/pkg/net/http/httptest/example_test.go:17: http.Error(w, req.URL.Path+":
something went horribly wrong.", http.StatusInternalServerError)
remove the period from the message

https://codereview.appspot.com/7314046/diff/4001/src/pkg/net/http/httptest/ex...
src/pkg/net/http/httptest/example_test.go:29: //Output: 500 - /foo: something
went horribly wrong.
"// Output:"

add the space.

https://codereview.appspot.com/7314046/diff/4001/src/pkg/net/http/httptest/ex...
src/pkg/net/http/httptest/example_test.go:50: //Output: Hello, client
ditto

[bradfitz, 2013-02-07 22:31:14]

https://codereview.appspot.com/7314046/diff/4001/src/pkg/net/http/httptest/ex...
File src/pkg/net/http/httptest/example_test.go (right):

https://codereview.appspot.com/7314046/diff/4001/src/pkg/net/http/httptest/ex...
src/pkg/net/http/httptest/example_test.go:36: server :=
httptest.NewServer(http.HandlerFunc(handler))
in all the net/http tests, we never make a variable for "handler" even... just:

server := httptest.NewServer(http.HandlerFunc(w http.ResponseWriter, req
*http.Request) {
       ....
})
defer server.Close()

We also normally say "ts", but "server" is fine.

https://codereview.appspot.com/7314046/diff/4001/src/pkg/net/http/httptest/ex...
src/pkg/net/http/httptest/example_test.go:41: log.Fatal("server did not
respond")
how do you know? maybe there was no DNS and you never even reached the server.
don't throw out err. it tries to say something useful.

[kisielk, 2013-02-08 03:09:44]

Thanks for the feedback, PTAL.

https://codereview.appspot.com/7314046/diff/4001/src/pkg/net/http/httptest/ex...
File src/pkg/net/http/httptest/example_test.go (right):

https://codereview.appspot.com/7314046/diff/4001/src/pkg/net/http/httptest/ex...
src/pkg/net/http/httptest/example_test.go:17: http.Error(w, req.URL.Path+":
something went horribly wrong.", http.StatusInternalServerError)
On 2013/02/07 22:21:43, adg wrote:
> remove the period from the message

Done

https://codereview.appspot.com/7314046/diff/4001/src/pkg/net/http/httptest/ex...
src/pkg/net/http/httptest/example_test.go:29: //Output: 500 - /foo: something
went horribly wrong.
On 2013/02/07 22:21:43, adg wrote:
> "// Output:"
> 
> add the space.

Done

https://codereview.appspot.com/7314046/diff/4001/src/pkg/net/http/httptest/ex...
src/pkg/net/http/httptest/example_test.go:36: server :=
httptest.NewServer(http.HandlerFunc(handler))
On 2013/02/07 22:31:14, bradfitz wrote:
> in all the net/http tests, we never make a variable for "handler" even...
just:
> 
> server := httptest.NewServer(http.HandlerFunc(w http.ResponseWriter, req
> *http.Request) {
>        ....
> })
> defer server.Close()
> 
> We also normally say "ts", but "server" is fine.

I took a look at how this was used in net/http and reworked it in to a similar
style.

https://codereview.appspot.com/7314046/diff/4001/src/pkg/net/http/httptest/ex...
src/pkg/net/http/httptest/example_test.go:41: log.Fatal("server did not
respond")
On 2013/02/07 22:31:14, bradfitz wrote:
> how do you know? maybe there was no DNS and you never even reached the server.
> don't throw out err. it tries to say something useful.

Totally, not sure why I ate the error here but not in the other spot. Fixed to
just print err.

https://codereview.appspot.com/7314046/diff/4001/src/pkg/net/http/httptest/ex...
src/pkg/net/http/httptest/example_test.go:50: //Output: Hello, client
On 2013/02/07 22:21:43, adg wrote:
> ditto

Done

[bradfitz, 2013-02-08 03:51:55]

https://codereview.appspot.com/7314046/diff/4002/src/pkg/net/http/httptest/ex...
File src/pkg/net/http/httptest/example_test.go (right):

https://codereview.appspot.com/7314046/diff/4002/src/pkg/net/http/httptest/ex...
src/pkg/net/http/httptest/example_test.go:17: http.Error(w, r.URL.Path+":
something went horribly wrong", http.StatusInternalServerError)
this is an XSS vector, so makes for a bad example.  http.Error doesn't escape
anything.  Just say "something failed", or use html.EscapeString.

[kisielk, 2013-02-08 04:14:08]

Hello golang-dev@googlegroups.com, adg@golang.org, bradfitz@golang.org (cc:
golang-dev@googlegroups.com),

Please take another look.

[bradfitz, 2013-02-08 17:18:09]

LGTM.

I'll reorder one line before I submit.


On Thu, Feb 7, 2013 at 8:14 PM, <kamil.kisiel@gmail.com> wrote:

> Hello golang-dev@googlegroups.com, adg@golang.org, bradfitz@golang.org
> (cc: golang-dev@googlegroups.com),
>
> Please take another look.
>
>
>
https://codereview.appspot.**com/7314046/<https://codereview.appspot.com/7314...
>

[bradfitz, 2013-02-08 17:20:55]

*** Submitted as https://code.google.com/p/go/source/detail?r=bad13530d9b3 ***

net/http/httptest: add examples

R=golang-dev, adg, bradfitz
CC=golang-dev
https://codereview.appspot.com/7314046

Committer: Brad Fitzpatrick <bradfitz@golang.org>

[remyoudompheng, 2013-07-20 21:25:00]

R=close