code review 57190045: net/http: temporarily disable the failing tests on Plan 9

net/http: temporarily disable the failing tests on Plan 9

Update Issue 7237

[0intro, 2014-01-29 20:45:30]

Hello jas (cc: golang-codereviews@googlegroups.com),

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

[0intro, 2014-01-29 20:47:21]

Jeff Sickel is currently actively working on the network poller implementation
for Plan 9. It will be used to handle deadline functions and pass the timeout
tests from the http/package. In the meantime, temporarily disabling these tests
would be useful to investigate the remaining issues.

[bradfitz, 2014-01-30 08:08:22]

Could you reference a bug number in in the skip text so we know when to
remove the skips?
 On Jan 29, 2014 9:45 PM, <0intro@gmail.com> wrote:

> Reviewers: jas,
>
> Message:
> Hello jas (cc: golang-codereviews@googlegroups.com),
>
> I'd like you to review this change to
> https://code.google.com/p/go
>
>
> Description:
> net/http: temporarily disable the failing tests on Plan 9
>
> Please review this at https://codereview.appspot.com/57190045/
>
> Affected files (+24, -0 lines):
>   M src/pkg/net/http/serve_test.go
>   M src/pkg/net/http/transport_test.go
>
>
> Index: src/pkg/net/http/serve_test.go
> ===================================================================
> --- a/src/pkg/net/http/serve_test.go
> +++ b/src/pkg/net/http/serve_test.go
> @@ -441,6 +441,9 @@
>  }
>
>  func TestServerTimeouts(t *testing.T) {
> +       if runtime.GOOS == "plan9" {
> +               t.Skipf("skipping test on %q", runtime.GOOS)
> +       }
>         defer afterTest(t)
>         reqNum := 0
>         ts := httptest.NewUnstartedServer(HandlerFunc(func(res
> ResponseWriter, req *Request) {
> @@ -517,6 +520,9 @@
>  // shouldn't cause a handler to block forever on reads (next HTTP
>  // request) that will never happen.
>  func TestOnlyWriteTimeout(t *testing.T) {
> +       if runtime.GOOS == "plan9" {
> +               t.Skipf("skipping test on %q", runtime.GOOS)
> +       }
>         defer afterTest(t)
>         var conn net.Conn
>         var afterTimeoutErrc = make(chan error, 1)
> @@ -840,6 +846,9 @@
>  }
>
>  func TestTLSHandshakeTimeout(t *testing.T) {
> +       if runtime.GOOS == "plan9" {
> +               t.Skipf("skipping test on %q", runtime.GOOS)
> +       }
>         defer afterTest(t)
>         ts := httptest.NewUnstartedServer(HandlerFunc(func(w
> ResponseWriter, r *Request) {}))
>         ts.Config.ReadTimeout = 250 * time.Millisecond
> @@ -1414,6 +1423,9 @@
>  // TestClientWriteShutdown tests that if the client shuts down the write
>  // side of their TCP connection, the server doesn't send a 400 Bad
> Request.
>  func TestClientWriteShutdown(t *testing.T) {
> +       if runtime.GOOS == "plan9" {
> +               t.Skipf("skipping test on %q", runtime.GOOS)
> +       }
>         defer afterTest(t)
>         ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r
> *Request) {}))
>         defer ts.Close()
> Index: src/pkg/net/http/transport_test.go
> ===================================================================
> --- a/src/pkg/net/http/transport_test.go
> +++ b/src/pkg/net/http/transport_test.go
> @@ -793,6 +793,9 @@
>
>  // tests that persistent goroutine connections shut down when no longer
> desired.
>  func TestTransportPersistConnLeak(t *testing.T) {
> +       if runtime.GOOS == "plan9" {
> +               t.Skipf("skipping test on %q", runtime.GOOS)
> +       }
>         defer afterTest(t)
>         gotReqCh := make(chan bool)
>         unblockCh := make(chan bool)
> @@ -859,6 +862,9 @@
>  // golang.org/issue/4531: Transport leaks goroutines when
>  // request.ContentLength is explicitly short
>  func TestTransportPersistConnLeakShortBody(t *testing.T) {
> +       if runtime.GOOS == "plan9" {
> +               t.Skipf("skipping test on %q", runtime.GOOS)
> +       }
>         defer afterTest(t)
>         ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r
> *Request) {
>         }))
> @@ -1066,6 +1072,9 @@
>  }
>
>  func TestIssue4191_InfiniteGetTimeout(t *testing.T) {
> +       if runtime.GOOS == "plan9" {
> +               t.Skipf("skipping test on %q", runtime.GOOS)
> +       }
>         defer afterTest(t)
>         const debug = false
>         mux := NewServeMux()
> @@ -1127,6 +1136,9 @@
>  }
>
>  func TestIssue4191_InfiniteGetToPutTimeout(t *testing.T) {
> +       if runtime.GOOS == "plan9" {
> +               t.Skipf("skipping test on %q", runtime.GOOS)
> +       }
>         defer afterTest(t)
>         const debug = false
>         mux := NewServeMux()
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "golang-codereviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-codereviews+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
>

[jas, 2014-01-30 08:18:27]

We'll need to generate an issue/bug number to attach it to.  Though I think we
should do a full skip audit once we get the plan9 builder to an 'ok' state.

[0intro, 2014-01-30 08:29:54]

Yes, I've just created issue 7237.

[bradfitz, 2014-01-30 08:55:56]

Cool. Now just reference that bug number in the skip text.

[0intro, 2014-01-30 09:22:28]

Please take another look.

[bradfitz, 2014-01-30 09:33:41]

LGTM but can remove the f$, %q, and runtime.GOOS before submitting

Because the issue number is specific to plan9 anyway.



On Thu, Jan 30, 2014 at 10:22 AM, <0intro@gmail.com> wrote:

> Please take another look.
>
> https://codereview.appspot.com/57190045/
>

[0intro, 2014-01-30 10:12:21]

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

net/http: temporarily disable the failing tests on Plan 9

Update Issue 7237

LGTM=bradfitz
R=jas, bradfitz
CC=golang-codereviews
https://codereview.appspot.com/57190045

[gobot, 2014-01-30 10:16:22]

This CL appears to have broken the freebsd-amd64 builder.