From b3f999e86331b3c19572a88b5e123ea961ac54fb Mon Sep 17 00:00:00 2001 From: Kristina Zabunova Date: Tue, 30 Jun 2015 10:49:00 -0700 Subject: [PATCH 1/3] Increased test coverage of httputils to 70 % Signed-off-by: Kristina Zabunova (cherry picked from commit d71817464e859cd323d6cdaf4dec5a9dd530f57e) --- pkg/httputils/httputils_test.go | 59 +++++++++++++++++++++++++++++++++ pkg/httputils/mimetype_test.go | 14 ++++++++ 2 files changed, 73 insertions(+) create mode 100644 pkg/httputils/httputils_test.go create mode 100644 pkg/httputils/mimetype_test.go diff --git a/pkg/httputils/httputils_test.go b/pkg/httputils/httputils_test.go new file mode 100644 index 0000000000..4704959859 --- /dev/null +++ b/pkg/httputils/httputils_test.go @@ -0,0 +1,59 @@ +package httputils + +import ( + "testing" +) + +func TestDownload(t *testing.T) { + _, err := Download("http://docker.com") + + if err != nil { + t.Errorf("Expected error to not exist when Download(http://docker.com)") + } + + // Expected status code = 404 + _, err = Download("http://docker.com/abc1234567") + if err == nil { + t.Errorf("Expected error to exist when Download(http://docker.com/abc1234567)") + } +} + +func TestNewHTTPRequestError(t *testing.T) { + errorMessage := "Some error message" + httpResponse, _ := Download("http://docker.com") + err := NewHTTPRequestError(errorMessage, httpResponse) + + if err.Error() != errorMessage { + t.Errorf("Expected err to equal error Message") + } +} + +func TestParseServerHeader(t *testing.T) { + serverHeader, err := ParseServerHeader("bad header") + if err.Error() != "Bad header: Failed regex match" { + t.Errorf("Should fail when header can not be parsed") + } + + serverHeader, err = ParseServerHeader("(bad header)") + if err.Error() != "Bad header: '/' missing" { + t.Errorf("Should fail when header can not be parsed") + } + + serverHeader, err = ParseServerHeader("(without/spaces)") + if err.Error() != "Bad header: Expected single space" { + t.Errorf("Should fail when header can not be parsed") + } + + serverHeader, err = ParseServerHeader("(header/with space)") + if serverHeader.App != "(header" { + t.Errorf("Expected serverHeader.App to equal \"(header\"") + } + + if serverHeader.Ver != "with" { + t.Errorf("Expected serverHeader.Ver to equal \"with\"") + } + + if serverHeader.OS != "header/with space" { + t.Errorf("Expected serverHeader.OS to equal \"header/with space\"") + } +} diff --git a/pkg/httputils/mimetype_test.go b/pkg/httputils/mimetype_test.go new file mode 100644 index 0000000000..90f0eceea1 --- /dev/null +++ b/pkg/httputils/mimetype_test.go @@ -0,0 +1,14 @@ +package httputils + +import ( + "testing" +) + +func TestDetectContentType(t *testing.T) { + input := []byte("That is just a plain text") + + contentType, _, err := DetectContentType(input) + if err != nil || contentType != "text/plain" { + t.Errorf("TestDetectContentType failed") + } +} From f753f6d597efb218d4b5786b9a4f09b6e73b48bc Mon Sep 17 00:00:00 2001 From: Kristina Zabunova Date: Wed, 1 Jul 2015 08:42:13 -0700 Subject: [PATCH 2/3] =?UTF-8?q?unit=20test=20refactor=20in=20pkg/httputils?= =?UTF-8?q?=20as=20suggested=20by=20vdemeester;=20using=20pattern=20if=20x?= =?UTF-8?q?=20:=3D=20=E2=80=A6;=20x=20=3D=3D=20nil=20{}?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Kristina Zabunova (cherry picked from commit c3f1b2a5bd4a4330bcbad401c316af90925b99ad) --- pkg/httputils/httputils_test.go | 18 ++++++++---------- pkg/httputils/mimetype_test.go | 3 +-- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/pkg/httputils/httputils_test.go b/pkg/httputils/httputils_test.go index 4704959859..221462425e 100644 --- a/pkg/httputils/httputils_test.go +++ b/pkg/httputils/httputils_test.go @@ -12,8 +12,7 @@ func TestDownload(t *testing.T) { } // Expected status code = 404 - _, err = Download("http://docker.com/abc1234567") - if err == nil { + if _, err = Download("http://docker.com/abc1234567"); err == nil { t.Errorf("Expected error to exist when Download(http://docker.com/abc1234567)") } } @@ -21,9 +20,7 @@ func TestDownload(t *testing.T) { func TestNewHTTPRequestError(t *testing.T) { errorMessage := "Some error message" httpResponse, _ := Download("http://docker.com") - err := NewHTTPRequestError(errorMessage, httpResponse) - - if err.Error() != errorMessage { + if err := NewHTTPRequestError(errorMessage, httpResponse); err.Error() != errorMessage { t.Errorf("Expected err to equal error Message") } } @@ -34,17 +31,18 @@ func TestParseServerHeader(t *testing.T) { t.Errorf("Should fail when header can not be parsed") } - serverHeader, err = ParseServerHeader("(bad header)") - if err.Error() != "Bad header: '/' missing" { + if serverHeader, err = ParseServerHeader("(bad header)"); err.Error() != "Bad header: '/' missing" { t.Errorf("Should fail when header can not be parsed") } - serverHeader, err = ParseServerHeader("(without/spaces)") - if err.Error() != "Bad header: Expected single space" { + if serverHeader, err = ParseServerHeader("(without/spaces)"); err.Error() != "Bad header: Expected single space" { t.Errorf("Should fail when header can not be parsed") } - serverHeader, err = ParseServerHeader("(header/with space)") + if serverHeader, err = ParseServerHeader("(header/with space)"); err != nil { + t.Errorf("Expected err to not exist when ParseServerHeader(\"(header/with space)\")") + } + if serverHeader.App != "(header" { t.Errorf("Expected serverHeader.App to equal \"(header\"") } diff --git a/pkg/httputils/mimetype_test.go b/pkg/httputils/mimetype_test.go index 90f0eceea1..9de433ee8c 100644 --- a/pkg/httputils/mimetype_test.go +++ b/pkg/httputils/mimetype_test.go @@ -7,8 +7,7 @@ import ( func TestDetectContentType(t *testing.T) { input := []byte("That is just a plain text") - contentType, _, err := DetectContentType(input) - if err != nil || contentType != "text/plain" { + if contentType, _, err := DetectContentType(input); err != nil || contentType != "text/plain" { t.Errorf("TestDetectContentType failed") } } From 243f98ff6de7aa16319be25c57ef1d910dfb6770 Mon Sep 17 00:00:00 2001 From: David Calavera Date: Fri, 28 Aug 2015 11:03:04 -0400 Subject: [PATCH 3/3] Fix server header parsing. Signed-off-by: David Calavera --- pkg/httputils/httputils.go | 30 ++++++++++------------ pkg/httputils/httputils_test.go | 44 +++++++++++++++++---------------- 2 files changed, 36 insertions(+), 38 deletions(-) diff --git a/pkg/httputils/httputils.go b/pkg/httputils/httputils.go index c586c0232e..0fd90ea3b1 100644 --- a/pkg/httputils/httputils.go +++ b/pkg/httputils/httputils.go @@ -5,11 +5,15 @@ import ( "fmt" "net/http" "regexp" - "strings" "github.com/docker/docker/pkg/jsonmessage" ) +var ( + headerRegexp = regexp.MustCompile(`^(?:(.+)/(.+))\s\((.+)\).*$`) + errInvalidHeader = errors.New("Bad header, should be in format `docker/version (platform)`") +) + // Download requests a given URL and returns an io.Reader. func Download(url string) (resp *http.Response, err error) { if resp, err = http.Get(url); err != nil { @@ -39,21 +43,13 @@ type ServerHeader struct { // ParseServerHeader extracts pieces from an HTTP server header // which is in the format "docker/version (os)" eg docker/1.8.0-dev (windows). func ParseServerHeader(hdr string) (*ServerHeader, error) { - re := regexp.MustCompile(`.*\((.+)\).*$`) - r := &ServerHeader{} - if matches := re.FindStringSubmatch(hdr); matches != nil { - r.OS = matches[1] - parts := strings.Split(hdr, "/") - if len(parts) != 2 { - return nil, errors.New("Bad header: '/' missing") - } - r.App = parts[0] - v := strings.Split(parts[1], " ") - if len(v) != 2 { - return nil, errors.New("Bad header: Expected single space") - } - r.Ver = v[0] - return r, nil + matches := headerRegexp.FindStringSubmatch(hdr) + if len(matches) != 4 { + return nil, errInvalidHeader } - return nil, errors.New("Bad header: Failed regex match") + return &ServerHeader{ + App: matches[1], + Ver: matches[2], + OS: matches[3], + }, nil } diff --git a/pkg/httputils/httputils_test.go b/pkg/httputils/httputils_test.go index 221462425e..61e98ba0b4 100644 --- a/pkg/httputils/httputils_test.go +++ b/pkg/httputils/httputils_test.go @@ -1,19 +1,17 @@ package httputils -import ( - "testing" -) +import "testing" func TestDownload(t *testing.T) { _, err := Download("http://docker.com") if err != nil { - t.Errorf("Expected error to not exist when Download(http://docker.com)") + t.Fatalf("Expected error to not exist when Download(http://docker.com)") } // Expected status code = 404 if _, err = Download("http://docker.com/abc1234567"); err == nil { - t.Errorf("Expected error to exist when Download(http://docker.com/abc1234567)") + t.Fatalf("Expected error to exist when Download(http://docker.com/abc1234567)") } } @@ -21,37 +19,41 @@ func TestNewHTTPRequestError(t *testing.T) { errorMessage := "Some error message" httpResponse, _ := Download("http://docker.com") if err := NewHTTPRequestError(errorMessage, httpResponse); err.Error() != errorMessage { - t.Errorf("Expected err to equal error Message") + t.Fatalf("Expected err to equal error Message") } } func TestParseServerHeader(t *testing.T) { - serverHeader, err := ParseServerHeader("bad header") - if err.Error() != "Bad header: Failed regex match" { - t.Errorf("Should fail when header can not be parsed") + if _, err := ParseServerHeader("bad header"); err != errInvalidHeader { + t.Fatalf("Should fail when header can not be parsed") } - if serverHeader, err = ParseServerHeader("(bad header)"); err.Error() != "Bad header: '/' missing" { - t.Errorf("Should fail when header can not be parsed") + if _, err := ParseServerHeader("(bad header)"); err != errInvalidHeader { + t.Fatalf("Should fail when header can not be parsed") } - if serverHeader, err = ParseServerHeader("(without/spaces)"); err.Error() != "Bad header: Expected single space" { - t.Errorf("Should fail when header can not be parsed") + if _, err := ParseServerHeader("(without/spaces)"); err != errInvalidHeader { + t.Fatalf("Should fail when header can not be parsed") } - if serverHeader, err = ParseServerHeader("(header/with space)"); err != nil { - t.Errorf("Expected err to not exist when ParseServerHeader(\"(header/with space)\")") + if _, err := ParseServerHeader("(header/with space)"); err != errInvalidHeader { + t.Fatalf("Expected err to not exist when ParseServerHeader(\"(header/with space)\")") } - if serverHeader.App != "(header" { - t.Errorf("Expected serverHeader.App to equal \"(header\"") + serverHeader, err := ParseServerHeader("foo/bar (baz)") + if err != nil { + t.Fatal(err) } - if serverHeader.Ver != "with" { - t.Errorf("Expected serverHeader.Ver to equal \"with\"") + if serverHeader.App != "foo" { + t.Fatalf("Expected serverHeader.App to equal \"foo\", got %s", serverHeader.App) } - if serverHeader.OS != "header/with space" { - t.Errorf("Expected serverHeader.OS to equal \"header/with space\"") + if serverHeader.Ver != "bar" { + t.Fatalf("Expected serverHeader.Ver to equal \"bar\", got %s", serverHeader.Ver) + } + + if serverHeader.OS != "baz" { + t.Fatalf("Expected serverHeader.OS to equal \"baz\", got %s", serverHeader.OS) } }