httptest: introduce TempServer, clean up tests

This also breaks fs_test into two parts
as the range tests test http's private httpRange
and I had to change the fs_test package from
"http" to "http_test" to use httptest which otherwise
has a cyclic depedency back on http.

Aside: we should start exposing the Range
stuff in the future.

R=rsc
CC=golang-dev
https://golang.org/cl/4261047

5 files changed, 120 insertions, 93 deletions

diff --git a/src/pkg/http/fs_test.go b/src/pkg/http/fs_test.go
index a8b67e3f08..a89c76d0bf 100644
--- a/src/pkg/http/fs_test.go
+++ b/src/pkg/http/fs_test.go
@@ -2,89 +2,22 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-package http
+package http_test
 
 import (
 	"fmt"
+	. "http"
+	"http/httptest"
 	"io/ioutil"
-	"net"
 	"os"
-	"sync"
 	"testing"
 )
 
-var ParseRangeTests = []struct {
-	s      string
-	length int64
-	r      []httpRange
-}{
-	{"", 0, nil},
-	{"foo", 0, nil},
-	{"bytes=", 0, nil},
-	{"bytes=5-4", 10, nil},
-	{"bytes=0-2,5-4", 10, nil},
-	{"bytes=0-9", 10, []httpRange{{0, 10}}},
-	{"bytes=0-", 10, []httpRange{{0, 10}}},
-	{"bytes=5-", 10, []httpRange{{5, 5}}},
-	{"bytes=0-20", 10, []httpRange{{0, 10}}},
-	{"bytes=15-,0-5", 10, nil},
-	{"bytes=-5", 10, []httpRange{{5, 5}}},
-	{"bytes=-15", 10, []httpRange{{0, 10}}},
-	{"bytes=0-499", 10000, []httpRange{{0, 500}}},
-	{"bytes=500-999", 10000, []httpRange{{500, 500}}},
-	{"bytes=-500", 10000, []httpRange{{9500, 500}}},
-	{"bytes=9500-", 10000, []httpRange{{9500, 500}}},
-	{"bytes=0-0,-1", 10000, []httpRange{{0, 1}, {9999, 1}}},
-	{"bytes=500-600,601-999", 10000, []httpRange{{500, 101}, {601, 399}}},
-	{"bytes=500-700,601-999", 10000, []httpRange{{500, 201}, {601, 399}}},
-}
-
-func TestParseRange(t *testing.T) {
-	for _, test := range ParseRangeTests {
-		r := test.r
-		ranges, err := parseRange(test.s, test.length)
-		if err != nil && r != nil {
-			t.Errorf("parseRange(%q) returned error %q", test.s, err)
-		}
-		if len(ranges) != len(r) {
-			t.Errorf("len(parseRange(%q)) = %d, want %d", test.s, len(ranges), len(r))
-			continue
-		}
-		for i := range r {
-			if ranges[i].start != r[i].start {
-				t.Errorf("parseRange(%q)[%d].start = %d, want %d", test.s, i, ranges[i].start, r[i].start)
-			}
-			if ranges[i].length != r[i].length {
-				t.Errorf("parseRange(%q)[%d].length = %d, want %d", test.s, i, ranges[i].length, r[i].length)
-			}
-		}
-	}
-}
-
 const (
 	testFile       = "testdata/file"
 	testFileLength = 11
 )
 
-var (
-	serverOnce sync.Once
-	serverAddr string
-)
-
-func startServer(t *testing.T) {
-	serverOnce.Do(func() {
-		HandleFunc("/ServeFile", func(w ResponseWriter, r *Request) {
-			ServeFile(w, r, "testdata/file")
-		})
-		l, err := net.Listen("tcp", "127.0.0.1:0")
-		if err != nil {
-			t.Fatal("listen:", err)
-		}
-		serverAddr = l.Addr().String()
-		go Serve(l, nil)
-	})
-}
-
 var ServeFileRangeTests = []struct {
 	start, end int
 	r          string
@@ -99,7 +32,11 @@ var ServeFileRangeTests = []struct {
 }
 
 func TestServeFile(t *testing.T) {
-	startServer(t)
+	ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) {
+		ServeFile(w, r, "testdata/file")
+	}))
+	defer ts.Close()
+
 	var err os.Error
 
 	file, err := ioutil.ReadFile(testFile)
@@ -110,7 +47,7 @@ func TestServeFile(t *testing.T) {
 	// set up the Request (re-used for all tests)
 	var req Request
 	req.Header = make(Header)
-	if req.URL, err = ParseURL("http://" + serverAddr + "/ServeFile"); err != nil {
+	if req.URL, err = ParseURL(ts.URL); err != nil {
 		t.Fatal("ParseURL:", err)
 	}
 	req.Method = "GET"
@@ -149,7 +86,7 @@ func TestServeFile(t *testing.T) {
 }
 
 func getBody(t *testing.T, req Request) (*Response, []byte) {
-	r, err := send(&req, DefaultTransport)
+	r, err := DefaultClient.Do(&req)
 	if err != nil {
 		t.Fatal(req.URL.String(), "send:", err)
 	}
diff --git a/src/pkg/http/httptest/Makefile b/src/pkg/http/httptest/Makefile
index fa5ec1db00..eb35d8aec6 100644
--- a/src/pkg/http/httptest/Makefile
+++ b/src/pkg/http/httptest/Makefile
@@ -7,5 +7,6 @@ include ../../../Make.inc
 TARG=http/httptest
 GOFILES=\
 	recorder.go\
+	server.go\
 
 include ../../../Make.pkg
diff --git a/src/pkg/http/httptest/server.go b/src/pkg/http/httptest/server.go
new file mode 100644
index 0000000000..5c5c746743
--- /dev/null
+++ b/src/pkg/http/httptest/server.go
@@ -0,0 +1,42 @@
+// Copyright 2011 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Implementation of TempServer
+
+package httptest
+
+import (
+	"fmt"
+	"http"
+	"net"
+)
+
+// A Server is an HTTP server listening on a system-chosen port on the
+// local loopback interface, for use in end-to-end HTTP tests.
+type Server struct {
+	URL      string // base URL of form http://ipaddr:port with no trailing slash
+	Listener net.Listener
+}
+
+// NewServer starts and returns a new Server.
+// The caller should call Close when finished, to shut it down.
+func NewServer(handler http.Handler) *Server {
+	ts := new(Server)
+	l, err := net.Listen("tcp", "127.0.0.1:0")
+	if err != nil {
+		if l, err = net.Listen("tcp6", "[::1]:0"); err != nil {
+			panic(fmt.Sprintf("httptest: failed to listen on a port: %v", err))
+		}
+	}
+	ts.Listener = l
+	ts.URL = "http://" + l.Addr().String()
+	server := &http.Server{Handler: handler}
+	go server.Serve(l)
+	return ts
+}
+
+// Close shuts down the temporary server.
+func (s *Server) Close() {
+	s.Listener.Close()
+}
diff --git a/src/pkg/http/range_test.go b/src/pkg/http/range_test.go
new file mode 100644
index 0000000000..5274a81fa3
--- /dev/null
+++ b/src/pkg/http/range_test.go
@@ -0,0 +1,57 @@
+// Copyright 2011 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package http
+
+import (
+	"testing"
+)
+
+var ParseRangeTests = []struct {
+	s      string
+	length int64
+	r      []httpRange
+}{
+	{"", 0, nil},
+	{"foo", 0, nil},
+	{"bytes=", 0, nil},
+	{"bytes=5-4", 10, nil},
+	{"bytes=0-2,5-4", 10, nil},
+	{"bytes=0-9", 10, []httpRange{{0, 10}}},
+	{"bytes=0-", 10, []httpRange{{0, 10}}},
+	{"bytes=5-", 10, []httpRange{{5, 5}}},
+	{"bytes=0-20", 10, []httpRange{{0, 10}}},
+	{"bytes=15-,0-5", 10, nil},
+	{"bytes=-5", 10, []httpRange{{5, 5}}},
+	{"bytes=-15", 10, []httpRange{{0, 10}}},
+	{"bytes=0-499", 10000, []httpRange{{0, 500}}},
+	{"bytes=500-999", 10000, []httpRange{{500, 500}}},
+	{"bytes=-500", 10000, []httpRange{{9500, 500}}},
+	{"bytes=9500-", 10000, []httpRange{{9500, 500}}},
+	{"bytes=0-0,-1", 10000, []httpRange{{0, 1}, {9999, 1}}},
+	{"bytes=500-600,601-999", 10000, []httpRange{{500, 101}, {601, 399}}},
+	{"bytes=500-700,601-999", 10000, []httpRange{{500, 201}, {601, 399}}},
+}
+
+func TestParseRange(t *testing.T) {
+	for _, test := range ParseRangeTests {
+		r := test.r
+		ranges, err := parseRange(test.s, test.length)
+		if err != nil && r != nil {
+			t.Errorf("parseRange(%q) returned error %q", test.s, err)
+		}
+		if len(ranges) != len(r) {
+			t.Errorf("len(parseRange(%q)) = %d, want %d", test.s, len(ranges), len(r))
+			continue
+		}
+		for i := range r {
+			if ranges[i].start != r[i].start {
+				t.Errorf("parseRange(%q)[%d].start = %d, want %d", test.s, i, ranges[i].start, r[i].start)
+			}
+			if ranges[i].length != r[i].length {
+				t.Errorf("parseRange(%q)[%d].length = %d, want %d", test.s, i, ranges[i].length, r[i].length)
+			}
+		}
+	}
+}
diff --git a/src/pkg/http/serve_test.go b/src/pkg/http/serve_test.go
index 80fb829004..86d64bdbb9 100644
--- a/src/pkg/http/serve_test.go
+++ b/src/pkg/http/serve_test.go
@@ -171,13 +171,10 @@ func TestHostHandlers(t *testing.T) {
 	for _, h := range handlers {
 		Handle(h.pattern, stringHandler(h.msg))
 	}
-	l, err := net.Listen("tcp", "127.0.0.1:0") // any port
-	if err != nil {
-		t.Fatal(err)
-	}
-	defer l.Close()
-	go Serve(l, nil)
-	conn, err := net.Dial("tcp", "", l.Addr().String())
+	ts := httptest.NewServer(nil)
+	defer ts.Close()
+
+	conn, err := net.Dial("tcp", "", ts.Listener.Addr().String())
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -296,13 +293,6 @@ func TestServerTimeouts(t *testing.T) {
 
 // TestIdentityResponse verifies that a handler can unset 
 func TestIdentityResponse(t *testing.T) {
-	l, err := net.Listen("tcp", "127.0.0.1:0")
-	if err != nil {
-		t.Fatalf("failed to listen on a port: %v", err)
-	}
-	defer l.Close()
-	urlBase := "http://" + l.Addr().String() + "/"
-
 	handler := HandlerFunc(func(rw ResponseWriter, req *Request) {
 		rw.SetHeader("Content-Length", "3")
 		rw.SetHeader("Transfer-Encoding", req.FormValue("te"))
@@ -320,15 +310,15 @@ func TestIdentityResponse(t *testing.T) {
 		}
 	})
 
-	server := &Server{Handler: handler}
-	go server.Serve(l)
+	ts := httptest.NewServer(handler)
+	defer ts.Close()
 
 	// Note: this relies on the assumption (which is true) that
 	// Get sends HTTP/1.1 or greater requests.  Otherwise the
 	// server wouldn't have the choice to send back chunked
 	// responses.
 	for _, te := range []string{"", "identity"} {
-		url := urlBase + "?te=" + te
+		url := ts.URL + "/?te=" + te
 		res, _, err := Get(url)
 		if err != nil {
 			t.Fatalf("error with Get of %s: %v", url, err)
@@ -346,15 +336,15 @@ func TestIdentityResponse(t *testing.T) {
 	}
 
 	// Verify that ErrContentLength is returned
-	url := urlBase + "?overwrite=1"
-	_, _, err = Get(url)
+	url := ts.URL + "/?overwrite=1"
+	_, _, err := Get(url)
 	if err != nil {
 		t.Fatalf("error with Get of %s: %v", url, err)
 	}
 
 	// Verify that the connection is closed when the declared Content-Length
 	// is larger than what the handler wrote.
-	conn, err := net.Dial("tcp", "", l.Addr().String())
+	conn, err := net.Dial("tcp", "", ts.Listener.Addr().String())
 	if err != nil {
 		t.Fatalf("error dialing: %v", err)
 	}