code review 4070043: http: added virtual host mappings BEFORE default path m...

src/pkg/http/serve_test.go

 // Copyright 2010 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.
 
 // End-to-end serving tests
 
 package http
 
 import (
         "bytes"

[rsc, 2011/01/26 15:29:37]

sort imports

         "os"
         "net"
         "testing"
+        "fmt"
+        "io/ioutil"
 )
 
 type dummyAddr string
 type oneConnListener struct {
         conn net.Conn
 }
 
 func (l *oneConnListener) Accept() (c net.Conn, err os.Error) {
         c = l.conn
         if c == nil {
@@ skipping 102 matching lines @@
         if req.Method != "POST" {
                 t.Errorf("For request #2's method, got %q; expected %q",
                         req.Method, "POST")
         }
 
         t.Log("Waiting for EOF.")
         if serveerr := <-servech; serveerr != os.EOF {
                 t.Errorf("Serve returned %q; expected EOF", serveerr)
         }
 }
+
+func defaultHandler(rw ResponseWriter, r *Request) {
+        rw.Write(([]byte)("Default"))

[rsc, 2011/01/26 15:29:37]

the () around []byte are unnecessary.
same in various places below
rw.Flush is also unnecessary.
ResponseWriter argument is usually named w.

+        rw.Flush()
+}
+
+func someDirHandler(rw ResponseWriter, r *Request) {
+        rw.Write(([]byte)("someDir"))
+        rw.Flush()
+}
+
+func someHostHandler(rw ResponseWriter, r *Request) {
+        rw.Write(([]byte)("someHost.com/someDir"))
+        rw.Flush()
+}
+
+func GetVHost(url, host string) (r *Response, finalURL string, err os.Error) {

[rsc, 2011/01/26 15:29:37]

This is a copy of http.Get but I don't see what's different.
Please comment what's different (or fix Get if there's a bug).

+        for redirect := 0; ; redirect++ {
+                if redirect >= 10 {
+                        err = os.ErrorString("stopped after 10 redirects")
+                        break
+                }
+                var req Request
+                if req.URL, err = ParseURL(url); err != nil {
+                        break
+                }
+                url = req.URL.String()
+                req.Host = host
+                if r, err = send(&req); err != nil {
+                        break
+                }
+                if shouldRedirect(r.StatusCode) {
+                        r.Body.Close()
+                        if url = r.GetHeader("Location"); url == "" {
+                                err = os.ErrorString(fmt.Sprintf(
+                                        "%d response missing Location header", r.StatusCode))
+                                break
+                        }
+                        continue
+                }
+                finalURL = url
+                return
+        }
+        err = &URLError{"Get", url, err}
+        return
+}
+
+func checkResponse(t *testing.T, url, host, rsp string) {
+        var r *Response
+        var e os.Error
+        if host == "" {
+                r, _, e = Get(url)
+        } else {
+                r, _, e = GetVHost(url, host)
+        }
+        if e != nil {
+                t.Errorf("Get error %v\n", e)
+        }
+        msg, e := ioutil.ReadAll(r.Body)
+        if e != nil {
+                t.Errorf("Get body error: %v\n", e)
+        }
+        s := (string)(msg)

[rsc, 2011/01/26 15:29:37]

() are unnecessary around string

+        if s != rsp {
+                t.Errorf("Get body is unexpected: %v instead of %v\n", s, rsp)
+        }
+}
+
+func TestVirtualHostHandlers(t *testing.T) {
+        HandleFunc("/", defaultHandler)
+        HandleFunc("/someDir/", someDirHandler)
+        HandleFunc("someHost.com/someDir/", someHostHandler)
+        go ListenAndServe(":12345", nil)
+        checkResponse(t, "http://localhost:12345/someDir/apage", "", "someDir")

[rsc, 2011/01/26 15:29:37]

please use a table.
see url_test.go, for example.

var virtualTests = []struct {
    url string
    host string
    data string
}{
    {"http://localhost:12345/someDir/apage", "", "someDir"},
    ...
}

+        checkResponse(t, "http://localhost:12345/otherDir/apage", "", "Default")
+        checkResponse(t, "http://localhost:12345/someDir/apage", "someHost.com",
+                "someHost.com/someDir")
+        checkResponse(t, "http://localhost:12345/someDir/apage", "otherHost.com",
+                "someDir")
+        checkResponse(t, "http://localhost:12345/aDir/apage", "otherHost.com",
+                "Default")
+}