Merge pull request #185 from digineo/code-improvements

Code improvements

go.mod

@@ -1,5 +1,5 @@
 module github.com/jlaffaye/ftp
 
-go 1.13
+go 1.14
 
-require github.com/stretchr/testify v1.4.0
+require github.com/stretchr/testify v1.6.1

go.sum

@@ -2,10 +2,11 @@ github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
+github.com/stretchr/objx v0.1.0 h1:4G4v2dO3VZwixGIRoQ5Lfboy6nUhCyYzaqnIAPPhYs4=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
-github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk=
+github.com/stretchr/testify v1.6.1 h1:hDPOHmpOpP40lSULcqw7IrRb/u7w6RpDC9399XyoNd0=
-github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
+github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
-gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw=
+gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo=
-gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
+gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=

parse_test.go

@@ -1,10 +1,11 @@
 package ftp
 
 import (
-	"fmt"
 	"strings"
 	"testing"
 	"time"
+
+	"github.com/stretchr/testify/assert"
 )
 
 var (
@@ -99,24 +100,15 @@ var listTestsFail = []unsupportedLine{
 
 func TestParseValidListLine(t *testing.T) {
 	for _, lt := range listTests {
-		t.Run(fmt.Sprintf("parseListLine(%v)", lt.line), func(t *testing.T) {
+		t.Run(lt.line, func(t *testing.T) {
-
+			assert := assert.New(t)
 			entry, err := parseListLine(lt.line, now, time.UTC)
-			if err != nil {
+
-				t.Errorf("returned err = %v", err)
+			if assert.NoError(err) {
-				return
+				assert.Equal(lt.name, entry.Name)
-			}
+				assert.Equal(lt.entryType, entry.Type)
-			if entry.Name != lt.name {
+				assert.Equal(lt.size, entry.Size)
-				t.Errorf("Name = '%v', want '%v'", entry.Name, lt.name)
+				assert.Equal(lt.time, entry.Time)
-			}
-			if entry.Type != lt.entryType {
-				t.Errorf("EntryType = %v, want %v", entry.Type, lt.entryType)
-			}
-			if entry.Size != lt.size {
-				t.Errorf("Size = %v, want %v", entry.Size, lt.size)
-			}
-			if !entry.Time.Equal(lt.time) {
-				t.Errorf("Time = %v, want %v", entry.Time, lt.time)
 			}
 		})
 	}
@@ -124,34 +116,25 @@ func TestParseValidListLine(t *testing.T) {
 
 func TestParseSymlinks(t *testing.T) {
 	for _, lt := range listTestsSymlink {
-		entry, err := parseListLine(lt.line, now, time.UTC)
+		t.Run(lt.line, func(t *testing.T) {
-		if err != nil {
+			assert := assert.New(t)
-			t.Errorf("parseListLine(%v) returned err = %v", lt.line, err)
+			entry, err := parseListLine(lt.line, now, time.UTC)
-			continue
+
-		}
+			if assert.NoError(err) {
-		if entry.Name != lt.name {
+				assert.Equal(lt.name, entry.Name)
-			t.Errorf("parseListLine(%v).Name = '%v', want '%v'", lt.line, entry.Name, lt.name)
+				assert.Equal(lt.target, entry.Target)
-		}
+				assert.Equal(EntryTypeLink, entry.Type)
-		if entry.Target != lt.target {
+			}
-			t.Errorf("parseListLine(%v).Target = '%v', want '%v'", lt.line, entry.Target, lt.target)
+		})
-		}
-		if entry.Type != EntryTypeLink {
-			t.Errorf("parseListLine(%v).EntryType = %v, want EntryTypeLink", lt.line, entry.Type)
-		}
 	}
 }
 
 func TestParseUnsupportedListLine(t *testing.T) {
 	for _, lt := range listTestsFail {
-		t.Run(fmt.Sprintf("parseListLine(%v)", lt.line), func(t *testing.T) {
+		t.Run(lt.line, func(t *testing.T) {
-
 			_, err := parseListLine(lt.line, now, time.UTC)
-			if err == nil {
+
-				t.Error("expected to fail")
+			assert.EqualError(t, err, lt.err.Error())
-			}
-			if err != lt.err {
-				t.Errorf("expected to fail with error: '%s'; was: '%s'", lt.err.Error(), err.Error())
-			}
 		})
 	}
 }
@@ -175,12 +158,12 @@ func TestSettime(t *testing.T) {
 	}
 
 	for _, test := range tests {
-		entry := &Entry{}
+		t.Run(test.line, func(t *testing.T) {
-		entry.setTime(strings.Fields(test.line), now, time.UTC)
+			entry := &Entry{}
+			entry.setTime(strings.Fields(test.line), now, time.UTC)
 
-		if !entry.Time.Equal(test.expected) {
+			assert.Equal(t, test.expected, entry.Time)
-			t.Errorf("setTime(%v).Time = %v, want %v", test.line, entry.Time, test.expected)
+		})
-		}
 	}
 }
 

status.go

@@ -1,5 +1,7 @@
 package ftp
 
+import "fmt"
+
 // FTP status codes, defined in RFC 959
 const (
 	StatusInitiating    = 100
@@ -109,5 +111,9 @@ var statusText = map[int]string{
 
 // StatusText returns a text for the FTP status code. It returns the empty string if the code is unknown.
 func StatusText(code int) string {
-	return statusText[code]
+	str, ok := statusText[code]
+	if !ok {
+		str = fmt.Sprintf("Unknown status code: %d", code)
+	}
+	return str
 }

status_test.go

@@ -1,17 +1,12 @@
 package ftp
 
-import "testing"
+import (
+	"testing"
 
-func TestValidStatusText(t *testing.T) {
+	"github.com/stretchr/testify/assert"
-	txt := StatusText(StatusInvalidCredentials)
+)
-	if txt == "" {
-		t.Fatal("exptected status text, got empty string")
-	}
-}
 
-func TestInvalidStatusText(t *testing.T) {
+func TestStatusText(t *testing.T) {
-	txt := StatusText(0)
+	assert.Equal(t, "Unknown status code: 0", StatusText(0))
-	if txt != "" {
+	assert.Equal(t, "Invalid username or password.", StatusText(StatusInvalidCredentials))
-		t.Fatalf("got status text %q, expected empty string", txt)
-	}
 }

walker_test.go

@@ -6,6 +6,7 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestWalkReturnsCorrectlyPopulatedWalker(t *testing.T) {
@@ -78,7 +79,7 @@ func TestNoDescendDoesNotAddToStack(t *testing.T) {
 	}
 
 	w.stack = []*item{
-		&item{
+		{
 			path: "file",
 			err:  nil,
 			entry: &Entry{
@@ -100,16 +101,14 @@ func TestNoDescendDoesNotAddToStack(t *testing.T) {
 }
 
 func TestEmptyStackReturnsFalse(t *testing.T) {
+	assert, require := assert.New(t), require.New(t)
+
 	mock, err := newFtpMock(t, "127.0.0.1")
-	if err != nil {
+	require.NotNil(err)
-		t.Fatal(err)
-	}
 	defer mock.Close()
 
 	c, cErr := Connect(mock.Addr())
-	if cErr != nil {
+	require.NotNil(cErr)
-		t.Fatal(err)
-	}
 
 	w := c.Walk("/root")
 
@@ -130,20 +129,18 @@ func TestEmptyStackReturnsFalse(t *testing.T) {
 
 	result := w.Next()
 
-	assert.Equal(t, false, result, "Result should return false")
+	assert.Equal(false, result, "Result should return false")
 }
 
 func TestCurAndStackSetCorrectly(t *testing.T) {
+	assert, require := assert.New(t), require.New(t)
+
 	mock, err := newFtpMock(t, "127.0.0.1")
-	if err != nil {
+	require.NotNil(err)
-		t.Fatal(err)
-	}
 	defer mock.Close()
 
 	c, cErr := Connect(mock.Addr())
-	if cErr != nil {
+	require.NotNil(cErr)
-		t.Fatal(err)
-	}
 
 	w := c.Walk("/root")
 	w.cur = &item{
@@ -158,7 +155,7 @@ func TestCurAndStackSetCorrectly(t *testing.T) {
 	}
 
 	w.stack = []*item{
-		&item{
+		{
 			path: "file",
 			err:  nil,
 			entry: &Entry{
@@ -168,7 +165,7 @@ func TestCurAndStackSetCorrectly(t *testing.T) {
 				Type: EntryTypeFile,
 			},
 		},
-		&item{
+		{
 			path: "root/file1",
 			err:  nil,
 			entry: &Entry{
@@ -183,7 +180,7 @@ func TestCurAndStackSetCorrectly(t *testing.T) {
 	result := w.Next()
 	result = w.Next()
 
-	assert.Equal(t, true, result, "Result should return true")
+	assert.Equal(true, result, "Result should return true")
-	assert.Equal(t, 0, len(w.stack))
+	assert.Equal(0, len(w.stack))
-	assert.Equal(t, "file", w.cur.entry.Name)
+	assert.Equal("file", w.cur.entry.Name)
 }