From 2f1104625df488bea7d261f079bb69a3eb41d72d Mon Sep 17 00:00:00 2001 From: Ali Rizvi-Santiago Date: Tue, 5 Apr 2016 16:51:17 -0500 Subject: [PATCH] Fixed some of the unit-tests in common/ due to the changes made in {config,download}.go config.go: Fixed some issues related to the url scheme not being lowercased which broke some of the tests. config_test.go: Removed the UNC share test for \\host\share\file since SMB support has been moved to a different uri scheme. download_test.go: Explicitly set the CopyFile configuration option for all the unit-tests that test file copying capability. Removed the UNC share testcase since it's under a different uri scheme now. Modified the file:// UNC share testcase to explicitly test the smb:// uri. Changed the incorrect t.Errorf calls to t.Logf so that the tests can pass. --- common/config.go | 13 +++++++++++-- common/config_test.go | 24 ++++++++++++++++-------- common/download_test.go | 21 +++++++++++++-------- 3 files changed, 40 insertions(+), 18 deletions(-) diff --git a/common/config.go b/common/config.go index 103c51105..bcc0e08e8 100644 --- a/common/config.go +++ b/common/config.go @@ -6,6 +6,7 @@ import ( "path/filepath" "strings" "time" + "net/url" ) // PackerKeyEnv is used to specify the key interval (delay) between keystrokes @@ -50,7 +51,7 @@ func DownloadableURL(original string) (string, error) { supported := []string{"file", "http", "https", "ftp", "smb"} found := false for _, s := range supported { - if strings.HasPrefix(original, s + "://") { + if strings.HasPrefix(strings.ToLower(original), s + "://") { found = true break } @@ -59,7 +60,15 @@ func DownloadableURL(original string) (string, error) { // If it's properly prefixed with something we support, then we don't need // to make it a uri. if found { - return original, nil + original = filepath.ToSlash(original) + + // make sure that it can be parsed though.. + uri,err := url.Parse(original) + if err != nil { return "", err } + + uri.Scheme = strings.ToLower(uri.Scheme) + + return uri.String(), nil } // If the file exists, then make it an absolute path diff --git a/common/config_test.go b/common/config_test.go index 365515109..3b1aff8cf 100644 --- a/common/config_test.go +++ b/common/config_test.go @@ -5,7 +5,6 @@ import ( "io/ioutil" "os" "path/filepath" - "runtime" "strings" "testing" ) @@ -38,6 +37,21 @@ func TestChooseString(t *testing.T) { } func TestDownloadableURL(t *testing.T) { + // Invalid URL: has hex code in host + _, err := DownloadableURL("http://what%20.com") + if err == nil { + t.Fatalf("expected err : %s", err) + } + + // Valid: http + u, err := DownloadableURL("HTTP://packer.io/path") + if err != nil { + t.Fatalf("err: %s", err) + } + + if u != "http://packer.io/path" { + t.Fatalf("bad: %s", u) + } cases := []struct { InputString string @@ -154,11 +168,7 @@ func TestDownloadableURL_FilePaths(t *testing.T) { } tfPath = filepath.Clean(tfPath) - filePrefix := "file://" - if runtime.GOOS == "windows" { - filePrefix += "/" - } // Relative filepath. We run this test in a func so that // the defers run right away. @@ -180,9 +190,7 @@ func TestDownloadableURL_FilePaths(t *testing.T) { t.Fatalf("err: %s", err) } - expected := fmt.Sprintf("%s%s", - filePrefix, - strings.Replace(tfPath, `\`, `/`, -1)) + expected := "file://" + strings.Replace(tfPath, `\`, `/`, -1) if u != expected { t.Fatalf("unexpected: %#v != %#v", u, expected) } diff --git a/common/download_test.go b/common/download_test.go index b46d81455..091d9c7d7 100644 --- a/common/download_test.go +++ b/common/download_test.go @@ -58,6 +58,7 @@ func TestDownloadClient_basic(t *testing.T) { client := NewDownloadClient(&DownloadConfig{ Url: ts.URL + "/basic.txt", TargetPath: tf.Name(), + CopyFile: true, }) path, err := client.Get() @@ -93,6 +94,7 @@ func TestDownloadClient_checksumBad(t *testing.T) { TargetPath: tf.Name(), Hash: HashForType("md5"), Checksum: checksum, + CopyFile: true, }) if _, err := client.Get(); err == nil { t.Fatal("should error") @@ -117,6 +119,7 @@ func TestDownloadClient_checksumGood(t *testing.T) { TargetPath: tf.Name(), Hash: HashForType("md5"), Checksum: checksum, + CopyFile: true, }) path, err := client.Get() if err != nil { @@ -147,6 +150,7 @@ func TestDownloadClient_checksumNoDownload(t *testing.T) { TargetPath: "./test-fixtures/root/another.txt", Hash: HashForType("md5"), Checksum: checksum, + CopyFile: true, }) path, err := client.Get() if err != nil { @@ -185,6 +189,7 @@ func TestDownloadClient_resume(t *testing.T) { client := NewDownloadClient(&DownloadConfig{ Url: ts.URL, TargetPath: tf.Name(), + CopyFile: true, }) path, err := client.Get() if err != nil { @@ -242,6 +247,7 @@ func TestDownloadClient_usesDefaultUserAgent(t *testing.T) { config := &DownloadConfig{ Url: server.URL, TargetPath: tf.Name(), + CopyFile: true, } client := NewDownloadClient(config) @@ -273,6 +279,7 @@ func TestDownloadClient_setsUserAgent(t *testing.T) { Url: server.URL, TargetPath: tf.Name(), UserAgent: "fancy user agent", + CopyFile: true, } client := NewDownloadClient(config) @@ -353,6 +360,7 @@ func TestDownloadFileUrl(t *testing.T) { if err != nil { t.Fatalf("Unable to detect working directory: %s", err) } + cwd = filepath.ToSlash(cwd) // source_path is a file path and source is a network path sourcePath := fmt.Sprintf("%s/test-fixtures/fileurl/%s", cwd, "cake") @@ -455,12 +463,10 @@ func TestFileUriTransforms(t *testing.T) { // ./relative/path -> ./relative/path // /absolute/path -> /absolute/path // c:/windows/absolute -> c:/windows/absolute - // \\host/sharename/file -> \\host/sharename/file testcases := []string{ "./%s", cwd + "/%s", volume + cwd + "/%s", - "\\\\" + host + "/" + share + "/" + cwd[1:] + "/%s", } // all regular slashed testcases @@ -471,19 +477,18 @@ func TestFileUriTransforms(t *testing.T) { if err != nil { t.Errorf("Unable to transform uri '%s' into a path : %v", uri, err) } - t.Errorf("TestFileUriTransforms : Result Path '%s'", res) + t.Logf("TestFileUriTransforms : Result Path '%s'", res) } // ...and finally the oddball windows native path - // \\host\sharename\file -> \\host/sharename/file - testpath_native := filepath.FromSlash(testpath) - testcase_native := "\\\\" + host + "\\" + share + "\\" + filepath.FromSlash(cwd[1:]) + "\\%s" - uri := "file://" + fmt.Sprintf(testcase_native, testpath_native) + // smb://host/sharename/file -> \\host\sharename\file + testcase := host + "/" + share + "/" + cwd[1:] + "/%s" + uri := "smb://" + fmt.Sprintf(testcase, testpath) t.Logf("TestFileUriTransforms : Trying Uri '%s'", uri) res,err := SimulateFileUriDownload(t, uri) if err != nil { t.Errorf("Unable to transform uri '%s' into a path", uri) return } - t.Errorf("TestFileUriTransforms : Result Path '%s'", res) + t.Logf("TestFileUriTransforms : Result Path '%s'", res) }