Merge pull request #872 from ceh/config-module-win-tests

config/module: fix test failures on Windows

config/module/detect_file.go

@@ -13,8 +13,6 @@ func (d *FileDetector) Detect(src, pwd string) (string, bool, error) {
 		return "", false, nil
 	}
 
-	// Make sure we're using "/" even on Windows. URLs are "/"-based.
-	src = filepath.ToSlash(src)
 	if !filepath.IsAbs(src) {
 		if pwd == "" {
 			return "", true, fmt.Errorf(
@@ -23,11 +21,5 @@ func (d *FileDetector) Detect(src, pwd string) (string, bool, error) {
 
 		src = filepath.Join(pwd, src)
 	}
-
+	return fmtFileURL(src), true, nil
-	// Make sure that we don't start with "/" since we add that below
-	if src[0] == '/' {
-		src = src[1:]
-	}
-
-	return fmt.Sprintf("file:///%s", src), true, nil
 }

config/module/detect_file_test.go

@@ -1,25 +1,42 @@
 package module
 
 import (
+	"runtime"
 	"testing"
 )
 
+type fileTest struct {
+	in, pwd, out string
+	err          bool
+}
+
+var fileTests = []fileTest{
+	{"./foo", "/pwd", "file:///pwd/foo", false},
+	{"./foo?foo=bar", "/pwd", "file:///pwd/foo?foo=bar", false},
+	{"foo", "/pwd", "file:///pwd/foo", false},
+}
+
+var unixFileTests = []fileTest{
+	{"/foo", "/pwd", "file:///foo", false},
+	{"/foo?bar=baz", "/pwd", "file:///foo?bar=baz", false},
+}
+
+var winFileTests = []fileTest{
+	{"/foo", "/pwd", "file:///pwd/foo", false},
+	{`C:\`, `/pwd`, `file:///C:/`, false},
+	{`C:\?bar=baz`, `/pwd`, `file:///C:/?bar=baz`, false},
+}
+
 func TestFileDetector(t *testing.T) {
-	cases := []struct {
+	if runtime.GOOS == "windows" {
-		Input  string
+		fileTests = append(fileTests, winFileTests...)
-		Output string
+	} else {
-	}{
+		fileTests = append(fileTests, unixFileTests...)
-		{"./foo", "file:///pwd/foo"},
-		{"./foo?foo=bar", "file:///pwd/foo?foo=bar"},
-		{"foo", "file:///pwd/foo"},
-		{"/foo", "file:///foo"},
-		{"/foo?bar=baz", "file:///foo?bar=baz"},
 	}
 
-	pwd := "/pwd"
 	f := new(FileDetector)
-	for i, tc := range cases {
+	for i, tc := range fileTests {
-		output, ok, err := f.Detect(tc.Input, pwd)
+		out, ok, err := f.Detect(tc.in, tc.pwd)
 		if err != nil {
 			t.Fatalf("err: %s", err)
 		}
@@ -27,36 +44,45 @@ func TestFileDetector(t *testing.T) {
 			t.Fatal("not ok")
 		}
 
-		if output != tc.Output {
+		if out != tc.out {
-			t.Fatalf("%d: bad: %#v", i, output)
+			t.Fatalf("%d: bad: %#v", i, out)
 		}
 	}
 }
 
+var noPwdFileTests = []fileTest{
+	{in: "./foo", pwd: "", out: "", err: true},
+	{in: "foo", pwd: "", out: "", err: true},
+}
+
+var noPwdUnixFileTests = []fileTest{
+	{in: "/foo", pwd: "", out: "file:///foo", err: false},
+}
+
+var noPwdWinFileTests = []fileTest{
+	{in: "/foo", pwd: "", out: "", err: true},
+	{in: `C:\`, pwd: ``, out: `file:///C:/`, err: false},
+}
+
 func TestFileDetector_noPwd(t *testing.T) {
-	cases := []struct {
+	if runtime.GOOS == "windows" {
-		Input  string
+		noPwdFileTests = append(noPwdFileTests, noPwdWinFileTests...)
-		Output string
+	} else {
-		Err    bool
+		noPwdFileTests = append(noPwdFileTests, noPwdUnixFileTests...)
-	}{
-		{"./foo", "", true},
-		{"foo", "", true},
-		{"/foo", "file:///foo", false},
 	}
 
-	pwd := ""
 	f := new(FileDetector)
-	for i, tc := range cases {
+	for i, tc := range noPwdFileTests {
-		output, ok, err := f.Detect(tc.Input, pwd)
+		out, ok, err := f.Detect(tc.in, tc.pwd)
-		if (err != nil) != tc.Err {
+		if (err != nil) != tc.err {
 			t.Fatalf("%d: err: %s", i, err)
 		}
 		if !ok {
 			t.Fatal("not ok")
 		}
 
-		if output != tc.Output {
+		if out != tc.out {
-			t.Fatalf("%d: bad: %#v", i, output)
+			t.Fatalf("%d: bad: %#v", i, out)
 		}
 	}
 }

config/module/get.go

@@ -72,7 +72,7 @@ func Get(dst, src string) error {
 		dst = tmpDir
 	}
 
-	u, err := url.Parse(src)
+	u, err := urlParse(src)
 	if err != nil {
 		return err
 	}

config/module/get_http_test.go

@@ -105,7 +105,7 @@ func TestHttpGetter_none(t *testing.T) {
 }
 
 func testHttpServer(t *testing.T) net.Listener {
-	ln, err := net.Listen("tcp", ":0")
+	ln, err := net.Listen("tcp", "127.0.0.1:0")
 	if err != nil {
 		t.Fatalf("err: %s", err)
 	}

config/module/module_test.go

@@ -39,15 +39,11 @@ func testModule(n string) string {
 	if err != nil {
 		panic(err)
 	}
-
+	return fmtFileURL(p)
-	var url url.URL
-	url.Scheme = "file"
-	url.Path = p
-	return url.String()
 }
 
 func testModuleURL(n string) *url.URL {
-	u, err := url.Parse(testModule(n))
+	u, err := urlParse(testModule(n))
 	if err != nil {
 		panic(err)
 	}

config/module/url_helper.go

@@ -0,0 +1,50 @@
+package module
+
+import (
+	"fmt"
+	"net/url"
+	"path/filepath"
+	"runtime"
+)
+
+func urlParse(rawURL string) (*url.URL, error) {
+	if runtime.GOOS == "windows" {
+		// Make sure we're using "/" on Windows. URLs are "/"-based.
+		rawURL = filepath.ToSlash(rawURL)
+	}
+	u, err := url.Parse(rawURL)
+	if err != nil {
+		return nil, err
+	}
+
+	if runtime.GOOS != "windows" {
+		return u, err
+	}
+
+	if u.Scheme != "file" {
+		return u, err
+	}
+
+	// Remove leading slash for absolute file paths on Windows.
+	// For example, url.Parse yields u.Path = "/C:/Users/user" for
+	// rawurl = "file:///C:/Users/user", which is an incorrect syntax.
+	if len(u.Path) > 2 && u.Path[0] == '/' && u.Path[2] == ':' {
+		u.Path = u.Path[1:]
+	}
+
+	return u, err
+}
+
+func fmtFileURL(path string) string {
+	if runtime.GOOS == "windows" {
+		// Make sure we're using "/" on Windows. URLs are "/"-based.
+		path = filepath.ToSlash(path)
+	}
+
+	// Make sure that we don't start with "/" since we add that below.
+	if path[0] == '/' {
+		path = path[1:]
+	}
+
+	return fmt.Sprintf("file:///%s", path)
+}