Repository: peak/s5cmd (go)

Issue:
`s5cmd ls -e` output is broken
Listing prefix/object mix with the `etag` parameter results in an unaligned output.

```
Δ s5cmd ls -e 's3://bucket/tmp/a/'
                                      DIR b/
2020/03/16 13:02:07   d41d8cd98f00b204e9800998ecf8427e            0 file2.txt
```

Pull request:
Align ls -e output
Fixes #101 

Patch:
diff --git a/command/app.go b/command/app.go
index b358d29..712fbbd 100644
--- a/command/app.go
+++ b/command/app.go
@@ -71,9 +71,8 @@ var app = &cli.App{
 		log.Init(logLevel, printJSON)
 		parallel.Init(workerCount)
 
-		// validation
-		if retryCount < 1 {
-			return fmt.Errorf("retry count must be a positive value")
+		if retryCount < 0 {
+			return fmt.Errorf("retry count cannot be a negative value")
 		}
 
 		s3opts := storage.S3Options{
diff --git a/command/cmd_copy.go b/command/cmd_copy.go
index 8572b60..3e48f0e 100644
--- a/command/cmd_copy.go
+++ b/command/cmd_copy.go
@@ -593,37 +593,6 @@ func prepareUploadDestination(
 	return dsturl.Join(objname)
 }
 
-// expandSource returns the full list of objects from the given src argument.
-// If src is an expandable URL, such as directory, prefix or a glob, all
-// objects are returned by walking the source.
-func expandSource(
-	ctx context.Context,
-	client storage.Storage,
-	srcurl *url.URL,
-) (<-chan *storage.Object, error) {
-	var isDir bool
-	// if the source is local, we send a Stat call to know if  we have
-	// directory or file to walk. For remote storage, we don't want to send
-	// Stat since it doesn't have any folder semantics.
-	if !srcurl.HasGlob() && !srcurl.IsRemote() {
-		obj, err := client.Stat(ctx, srcurl)
-		if err != nil {
-			return nil, err
-		}
-		isDir = obj.Type.IsDir()
-	}
-
-	// call storage.List for only walking operations.
-	if srcurl.HasGlob() || isDir {
-		return client.List(ctx, srcurl), nil
-	}
-
-	ch := make(chan *storage.Object, 1)
-	ch <- &storage.Object{URL: srcurl}
-	close(ch)
-	return ch, nil
-}
-
 // getObject checks if the object from given url exists. If no object is
 // found, error and returning object would be nil.
 func getObject(ctx context.Context, url *url.URL) (*storage.Object, error) {
diff --git a/command/cmd_delete.go b/command/cmd_delete.go
index e9f3155..ff46d9b 100644
--- a/command/cmd_delete.go
+++ b/command/cmd_delete.go
@@ -3,7 +3,6 @@ package command
 import (
 	"context"
 	"fmt"
-	"sync"
 
 	"github.com/hashicorp/go-multierror"
 	"github.com/urfave/cli/v2"
@@ -47,7 +46,7 @@ var DeleteCommand = &cli.Command{
 			return fmt.Errorf("expected at least 1 object to remove")
 		}
 
-		return checkSources(c.Args().Slice()...)
+		return sourcesHaveSameType(c.Args().Slice()...)
 	},
 	Action: func(c *cli.Context) error {
 		return Delete(
@@ -65,7 +64,7 @@ func Delete(
 	fullCommand string,
 	src ...string,
 ) error {
-	srcurls, err := newSources(src...)
+	srcurls, err := newURLs(src...)
 	if err != nil {
 		return err
 	}
@@ -119,52 +118,8 @@ func Delete(
 	return merror
 }
 
-// expandSources is a non-blocking argument dispatcher.
-// It creates a object channel by walking and expanding the given source urls.
-// If the url has a glob, it creates a goroutine to list storage items and sends them to
-// object channel, otherwise it creates storage object from the original source.
-func expandSources(
-	ctx context.Context,
-	client storage.Storage,
-	srcurls ...*url.URL,
-) <-chan *storage.Object {
-	mergech := make(chan *storage.Object)
-	go func() {
-		defer close(mergech)
-
-		var wg sync.WaitGroup
-		var objFound bool
-
-		for _, origSrc := range srcurls {
-			wg.Add(1)
-			go func(origSrc *url.URL) {
-				defer wg.Done()
-				objch, err := expandSource(ctx, client, origSrc)
-				if err != nil {
-					mergech <- &storage.Object{Err: err}
-					return
-				}
-				for object := range objch {
-					if object.Err == storage.ErrNoObjectFound {
-						continue
-					}
-					mergech <- object
-					objFound = true
-				}
-			}(origSrc)
-		}
-
-		wg.Wait()
-		if !objFound {
-			mergech <- &storage.Object{Err: storage.ErrNoObjectFound}
-		}
-	}()
-
-	return mergech
-}
-
-// newSources creates ObjectURL list from given source strings.
-func newSources(sources ...string) ([]*url.URL, error) {
+// newSources creates object URL list from given sources.
+func newURLs(sources ...string) ([]*url.URL, error) {
 	var urls []*url.URL
 	for _, src := range sources {
 		srcurl, err := url.New(src)
@@ -176,9 +131,8 @@ func newSources(sources ...string) ([]*url.URL, error) {
 	return urls, nil
 }
 
-// checkSources check if given sources share same objurlType and gives
-// error if it contains both local and remote targets.
-func checkSources(sources ...string) error {
+// sourcesHaveSameType check if given sources share the same object types.
+func sourcesHaveSameType(sources ...string) error {
 	var hasRemote, hasLocal bool
 	for _, src := range sources {
 		srcurl, err := url.New(src)
diff --git a/command/cmd_list.go b/command/cmd_list.go
index 84b781f..ffbd7b1 100644
--- a/command/cmd_list.go
+++ b/command/cmd_list.go
@@ -158,12 +158,18 @@ func (l ListMessage) humanize() string {
 }
 
 const (
-	listFormat = "%19s %1s %-6s %12s %s"
 	dateFormat = "2006/01/02 15:04:05"
 )
 
 // String returns the string representation of ListMessage.
 func (l ListMessage) String() string {
+	var listFormat = "%19s %1s %-1s %12s %s"
+	var etag string
+	if l.showEtag {
+		etag = l.Object.Etag
+		listFormat = "%19s %1s %-38s %12s %s"
+	}
+
 	if l.Object.Type.IsDir() {
 		s := fmt.Sprintf(
 			listFormat,
@@ -176,11 +182,6 @@ func (l ListMessage) String() string {
 		return s
 	}
 
-	var etag string
-	if l.showEtag {
-		etag = l.Object.Etag
-	}
-
 	s := fmt.Sprintf(
 		listFormat,
 		l.Object.ModTime.Format(dateFormat),
diff --git a/command/expand.go b/command/expand.go
new file mode 100644
index 0000000..b18c19b
--- /dev/null
+++ b/command/expand.go
@@ -0,0 +1,87 @@
+package command
+
+import (
+	"context"
+	"sync"
+
+	"github.com/peak/s5cmd/storage"
+	"github.com/peak/s5cmd/storage/url"
+)
+
+// expandSource returns the full list of objects from the given src argument.
+// If src is an expandable URL, such as directory, prefix or a glob, all
+// objects are returned by walking the source.
+func expandSource(
+	ctx context.Context,
+	client storage.Storage,
+	srcurl *url.URL,
+) (<-chan *storage.Object, error) {
+	var isDir bool
+	// if the source is local, we send a Stat call to know if  we have
+	// directory or file to walk. For remote storage, we don't want to send
+	// Stat since it doesn't have any folder semantics.
+	if !srcurl.HasGlob() && !srcurl.IsRemote() {
+		obj, err := client.Stat(ctx, srcurl)
+		if err != nil {
+			return nil, err
+		}
+		isDir = obj.Type.IsDir()
+	}
+
+	// call storage.List for only walking operations.
+	if srcurl.HasGlob() || isDir {
+		return client.List(ctx, srcurl), nil
+	}
+
+	ch := make(chan *storage.Object, 1)
+	ch <- &storage.Object{URL: srcurl}
+	close(ch)
+	return ch, nil
+}
+
+// expandSources is a non-blocking argument dispatcher. It creates a object
+// channel by walking and expanding the given source urls. If the url has a
+// glob, it creates a goroutine to list storage items and sends them to object
+// channel, otherwise it creates storage object from the original source.
+func expandSources(
+	ctx context.Context,
+	client storage.Storage,
+	srcurls ...*url.URL,
+) <-chan *storage.Object {
+	ch := make(chan *storage.Object)
+
+	go func() {
+		defer close(ch)
+
+		var wg sync.WaitGroup
+		var objFound bool
+
+		for _, origSrc := range srcurls {
+			wg.Add(1)
+			go func(origSrc *url.URL) {
+				defer wg.Done()
+
+				objch, err := expandSource(ctx, client, origSrc)
+				if err != nil {
+					ch <- &storage.Object{Err: err}
+					return
+				}
+
+				for object := range objch {
+					if object.Err == storage.ErrNoObjectFound {
+						continue
+					}
+					ch <- object
+					objFound = true
+				}
+			}(origSrc)
+		}
+
+		wg.Wait()
+		if !objFound {
+			ch <- &storage.Object{Err: storage.ErrNoObjectFound}
+		}
+	}()
+
+	return ch
+}


Test patch:
diff --git a/command/cmd_delete_test.go b/command/cmd_delete_test.go
index 3fe2f81..0ea8391 100644
--- a/command/cmd_delete_test.go
+++ b/command/cmd_delete_test.go
@@ -1,194 +1,10 @@
 package command
 
 import (
-	"context"
-	"reflect"
-	"sort"
 	"testing"
-
-	"github.com/stretchr/testify/mock"
-
-	"github.com/peak/s5cmd/storage"
-	"github.com/peak/s5cmd/storage/url"
 )
 
-func Test_expandSources(t *testing.T) {
-	t.Parallel()
-
-	tests := []struct {
-		name        string
-		src         map[string][]*storage.Object
-		wantObjects []string
-		wantError   error
-	}{
-		{
-			name: "merge_multiple_source_urls",
-			src: map[string][]*storage.Object{
-				"s3://bucket/key": {
-					{
-						URL: &url.URL{
-							Scheme: "s3",
-							Bucket: "bucket",
-							Path:   "key",
-						},
-					},
-				},
-				"s3://bucket/wildcard/*.txt": {
-					{
-						URL: &url.URL{
-							Scheme: "s3",
-							Bucket: "bucket",
-							Path:   "wildcard/test.txt",
-						},
-					},
-					{
-						URL: &url.URL{
-							Scheme: "s3",
-							Bucket: "bucket",
-							Path:   "wildcard/anothertest.txt",
-						},
-					},
-				},
-				"s3://bucket/dir/?/readme.md": {
-					{
-						URL: &url.URL{
-							Scheme: "s3",
-							Bucket: "bucket",
-							Path:   "dir/a/readme.md",
-						},
-					},
-					{
-						URL: &url.URL{
-							Scheme: "s3",
-							Bucket: "bucket",
-							Path:   "dir/b/readme.md",
-						},
-					},
-				},
-			},
-			wantObjects: []string{
-				"s3://bucket/dir/a/readme.md",
-				"s3://bucket/dir/b/readme.md",
-				"s3://bucket/key",
-				"s3://bucket/wildcard/anothertest.txt",
-				"s3://bucket/wildcard/test.txt",
-			},
-		},
-		{
-			name: "merge_multiple_with_empty_source",
-			src: map[string][]*storage.Object{
-				// this source has no item
-				"s3://bucket/wildcard/*.txt": {
-					{
-						Err: storage.ErrNoObjectFound,
-					},
-				},
-				"s3://bucket/*.txt": {
-					{
-						URL: &url.URL{
-							Scheme: "s3",
-							Bucket: "bucket",
-							Path:   "file1.txt",
-						},
-					},
-					{
-						URL: &url.URL{
-							Scheme: "s3",
-							Bucket: "bucket",
-							Path:   "file2.txt",
-						},
-					},
-				},
-			},
-			wantObjects: []string{
-				"s3://bucket/file1.txt",
-				"s3://bucket/file2.txt",
-			},
-		},
-		{
-			// if multiple source has no item.
-			// it will return single storage.ErrNoObjectFound error.
-			name: "no_item_found",
-			src: map[string][]*storage.Object{
-				// this source has no item
-				"s3://bucket/wildcard/*.txt": {
-					{
-						Err: storage.ErrNoObjectFound,
-					},
-				},
-				"s3://bucket/*.txt": {
-					{
-						Err: storage.ErrNoObjectFound,
-					},
-				},
-			},
-			wantError: storage.ErrNoObjectFound,
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-
-			srcurls, err := newSources(keys(tt.src)...)
-			if err != nil {
-				t.Errorf("unexpected error: %v", err)
-			}
-
-			client := &storage.MockStorage{}
-
-			for src, objects := range tt.src {
-				srcurl, err := url.New(src)
-				if err != nil {
-					t.Errorf("unexpected error: %v", err)
-				}
-
-				ch := generateObjects(objects)
-				client.On("List", mock.Anything, srcurl).Once().Return(ch)
-			}
-
-			gotChan := expandSources(context.Background(), client, srcurls...)
-
-			var objects []string
-			for obj := range gotChan {
-				if obj.Err != nil {
-					if obj.Err != tt.wantError {
-						t.Errorf("got error = %v, want %v", obj.Err, tt.wantError)
-					}
-					continue
-				}
-				objects = append(objects, obj.String())
-			}
-			// sort read objects
-			sort.Strings(objects)
-			if !reflect.DeepEqual(objects, tt.wantObjects) {
-				t.Errorf("got = %v, want %v", objects, tt.wantObjects)
-			}
-
-			client.AssertExpectations(t)
-		})
-	}
-}
-
-func keys(urls map[string][]*storage.Object) []string {
-	var urlKeys []string
-	for key := range urls {
-		urlKeys = append(urlKeys, key)
-	}
-	return urlKeys
-}
-
-func generateObjects(objects []*storage.Object) <-chan *storage.Object {
-	ch := make(chan *storage.Object)
-	go func() {
-		defer close(ch)
-		for _, object := range objects {
-			ch <- object
-		}
-	}()
-	return ch
-}
-
-func Test_checkSources(t *testing.T) {
+func TestSourcesHaveSameType(t *testing.T) {
 	t.Parallel()
 
 	tests := []struct {
@@ -200,7 +16,7 @@ func Test_checkSources(t *testing.T) {
 			name: "error_if_local_and_remote_mixed",
 			sources: []string{
 				"s3://bucket/key",
-				"filename.txy",
+				"filename.txt",
 			},
 			wantErr: true,
 		},
@@ -227,12 +43,13 @@ func Test_checkSources(t *testing.T) {
 			},
 		},
 	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
+	for _, tc := range tests {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
 
-			if err := checkSources(tt.sources...); (err != nil) != tt.wantErr {
-				t.Errorf("checkSources() error = %v, wantErr %v", err, tt.wantErr)
+			if err := sourcesHaveSameType(tc.sources...); (err != nil) != tc.wantErr {
+				t.Errorf("checkSources() error = %v, wantErr %v", err, tc.wantErr)
 			}
 		})
 	}
diff --git a/command/expand_test.go b/command/expand_test.go
new file mode 100644
index 0000000..8d6ea4b
--- /dev/null
+++ b/command/expand_test.go
@@ -0,0 +1,195 @@
+package command
+
+import (
+	"context"
+	"reflect"
+	"sort"
+	"testing"
+
+	"github.com/stretchr/testify/mock"
+
+	"github.com/peak/s5cmd/storage"
+	"github.com/peak/s5cmd/storage/url"
+)
+
+func TestExpandSources(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name        string
+		src         map[string][]*storage.Object
+		wantObjects []string
+		wantError   error
+	}{
+		{
+			name: "merge_multiple_source_urls",
+			src: map[string][]*storage.Object{
+				"s3://bucket/key": {
+					{
+						URL: &url.URL{
+							Scheme: "s3",
+							Bucket: "bucket",
+							Path:   "key",
+						},
+					},
+				},
+				"s3://bucket/wildcard/*.txt": {
+					{
+						URL: &url.URL{
+							Scheme: "s3",
+							Bucket: "bucket",
+							Path:   "wildcard/test.txt",
+						},
+					},
+					{
+						URL: &url.URL{
+							Scheme: "s3",
+							Bucket: "bucket",
+							Path:   "wildcard/anothertest.txt",
+						},
+					},
+				},
+				"s3://bucket/dir/?/readme.md": {
+					{
+						URL: &url.URL{
+							Scheme: "s3",
+							Bucket: "bucket",
+							Path:   "dir/a/readme.md",
+						},
+					},
+					{
+						URL: &url.URL{
+							Scheme: "s3",
+							Bucket: "bucket",
+							Path:   "dir/b/readme.md",
+						},
+					},
+				},
+			},
+			wantObjects: []string{
+				"s3://bucket/dir/a/readme.md",
+				"s3://bucket/dir/b/readme.md",
+				"s3://bucket/key",
+				"s3://bucket/wildcard/anothertest.txt",
+				"s3://bucket/wildcard/test.txt",
+			},
+		},
+		{
+			name: "merge_multiple_with_empty_source",
+			src: map[string][]*storage.Object{
+				// this source has no item
+				"s3://bucket/wildcard/*.txt": {
+					{
+						Err: storage.ErrNoObjectFound,
+					},
+				},
+				"s3://bucket/*.txt": {
+					{
+						URL: &url.URL{
+							Scheme: "s3",
+							Bucket: "bucket",
+							Path:   "file1.txt",
+						},
+					},
+					{
+						URL: &url.URL{
+							Scheme: "s3",
+							Bucket: "bucket",
+							Path:   "file2.txt",
+						},
+					},
+				},
+			},
+			wantObjects: []string{
+				"s3://bucket/file1.txt",
+				"s3://bucket/file2.txt",
+			},
+		},
+		{
+			// if multiple source has no item.
+			// it will return single storage.ErrNoObjectFound error.
+			name: "no_item_found",
+			src: map[string][]*storage.Object{
+				// this source has no item
+				"s3://bucket/wildcard/*.txt": {
+					{
+						Err: storage.ErrNoObjectFound,
+					},
+				},
+				"s3://bucket/*.txt": {
+					{
+						Err: storage.ErrNoObjectFound,
+					},
+				},
+			},
+			wantError: storage.ErrNoObjectFound,
+		},
+	}
+	for _, tc := range tests {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			// t.Parallel()
+
+			srcurls, err := newURLs(keys(tc.src)...)
+			if err != nil {
+				t.Errorf("unexpected error: %v", err)
+				return
+			}
+
+			client := &storage.MockStorage{}
+
+			for src, objects := range tc.src {
+				srcurl, err := url.New(src)
+				if err != nil {
+					t.Errorf("unexpected error: %v", err)
+					continue
+				}
+
+				ch := generateObjects(objects)
+
+				if src != "s3://bucket/key" {
+					client.On("List", mock.Anything, srcurl).Once().Return(ch)
+				}
+			}
+
+			gotChan := expandSources(context.Background(), client, srcurls...)
+
+			var objects []string
+			for obj := range gotChan {
+				if obj.Err != nil {
+					if obj.Err != tc.wantError {
+						t.Errorf("got error = %v, want %v", obj.Err, tc.wantError)
+					}
+					continue
+				}
+				objects = append(objects, obj.String())
+			}
+			// sort read objects
+			sort.Strings(objects)
+			if !reflect.DeepEqual(objects, tc.wantObjects) {
+				t.Errorf("got = %v, want %v", objects, tc.wantObjects)
+			}
+
+			client.AssertExpectations(t)
+		})
+	}
+}
+
+func keys(urls map[string][]*storage.Object) []string {
+	var urlKeys []string
+	for key := range urls {
+		urlKeys = append(urlKeys, key)
+	}
+	return urlKeys
+}
+
+func generateObjects(objects []*storage.Object) <-chan *storage.Object {
+	ch := make(chan *storage.Object, len(objects))
+	go func() {
+		defer close(ch)
+		for _, object := range objects {
+			ch <- object
+		}
+	}()
+	return ch
+}
diff --git a/e2e/app_test.go b/e2e/app_test.go
new file mode 100644
index 0000000..8bf46a4
--- /dev/null
+++ b/e2e/app_test.go
@@ -0,0 +1,69 @@
+package e2e
+
+import (
+	"fmt"
+	"strconv"
+	"testing"
+
+	"gotest.tools/v3/icmd"
+)
+
+func TestAppRetryCount(t *testing.T) {
+	// t.Parallel()
+
+	testcases := []struct {
+		name             string
+		retry            int
+		expectedError    error
+		expectedExitCode int
+	}{
+		{
+			name:             "retry_count_negative",
+			retry:            -1,
+			expectedError:    fmt.Errorf(`ERROR " ": retry count cannot be a negative value`),
+			expectedExitCode: 1,
+		},
+		{
+			name:             "retry_count_zero",
+			retry:            0,
+			expectedError:    nil,
+			expectedExitCode: 0,
+		},
+		{
+			name:             "retry_count_positive",
+			retry:            20,
+			expectedError:    nil,
+			expectedExitCode: 0,
+		},
+	}
+
+	for _, tc := range testcases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			_, s5cmd, cleanup := setup(t)
+			defer cleanup()
+
+			cmd := s5cmd("-r", strconv.Itoa(tc.retry))
+			result := icmd.RunCmd(cmd)
+
+			result.Assert(t, icmd.Expected{ExitCode: tc.expectedExitCode})
+
+			if tc.expectedError == nil {
+				if result.Stderr() != "" {
+					t.Fatalf("expected no error, got: %q", result.Stderr())
+				}
+				return
+			}
+
+			if result.Stderr() == "" {
+				t.Fatalf("expected error %q, got none", tc.expectedError)
+			}
+			assertLines(t, result.Stderr(), map[int]compareFunc{
+				0: equals("%v", tc.expectedError),
+				1: equals(""),
+			})
+		})
+	}
+}
diff --git a/e2e/cp_test.go b/e2e/cp_test.go
index fabfb49..b9f2dc6 100644
--- a/e2e/cp_test.go
+++ b/e2e/cp_test.go
@@ -42,38 +42,44 @@ func TestCopySingleS3ObjectToLocal(t *testing.T) {
 	)
 
 	testcases := []struct {
-		name     string
-		src      string
-		dst      string
-		expected fs.PathOp
+		name           string
+		src            string
+		dst            string
+		expected       fs.PathOp
+		expectedOutput string
 	}{
 		{
-			name:     "cp s3://bucket/object .",
-			src:      "file1.txt",
-			dst:      ".",
-			expected: fs.WithFile("file1.txt", fileContent, fs.WithMode(0644)),
+			name:           "cp s3://bucket/object .",
+			src:            "file1.txt",
+			dst:            ".",
+			expected:       fs.WithFile("file1.txt", fileContent, fs.WithMode(0644)),
+			expectedOutput: "cp s3://bucket/file1.txt file1.txt",
 		},
 		{
-			name:     "cp s3://bucket/object file",
-			src:      "file1.txt",
-			dst:      "file1.txt",
-			expected: fs.WithFile("file1.txt", fileContent, fs.WithMode(0644)),
+			name:           "cp s3://bucket/object file",
+			src:            "file1.txt",
+			dst:            "file1.txt",
+			expected:       fs.WithFile("file1.txt", fileContent, fs.WithMode(0644)),
+			expectedOutput: "cp s3://bucket/file1.txt file1.txt",
 		},
 		{
-			name:     "cp s3://bucket/object dir/",
-			src:      "file1.txt",
-			dst:      "dir/",
-			expected: fs.WithDir("dir", fs.WithFile("file1.txt", fileContent, fs.WithMode(0644))),
+			name:           "cp s3://bucket/object dir/",
+			src:            "file1.txt",
+			dst:            "dir/",
+			expected:       fs.WithDir("dir", fs.WithFile("file1.txt", fileContent, fs.WithMode(0644))),
+			expectedOutput: "cp s3://bucket/file1.txt dir/file1.txt",
 		},
 		{
-			name:     "cp s3://bucket/object dir/file",
-			src:      "file1.txt",
-			dst:      "dir/file1.txt",
-			expected: fs.WithDir("dir", fs.WithFile("file1.txt", fileContent, fs.WithMode(0644))),
+			name:           "cp s3://bucket/object dir/file",
+			src:            "file1.txt",
+			dst:            "dir/file1.txt",
+			expected:       fs.WithDir("dir", fs.WithFile("file1.txt", fileContent, fs.WithMode(0644))),
+			expectedOutput: "cp s3://bucket/file1.txt dir/file1.txt",
 		},
 	}
 
 	for _, tc := range testcases {
+		tc := tc
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
 
@@ -91,7 +97,7 @@ func TestCopySingleS3ObjectToLocal(t *testing.T) {
 			result.Assert(t, icmd.Success)
 
 			assertLines(t, result.Stdout(), map[int]compareFunc{
-				0: equals(`cp s3://%v/%v %v`, bucket, tc.src, tc.dst),
+				0: equals(tc.expectedOutput),
 				1: equals(""),
 			})
 
diff --git a/e2e/ls_test.go b/e2e/ls_test.go
index 4169e93..62994bf 100644
--- a/e2e/ls_test.go
+++ b/e2e/ls_test.go
@@ -7,6 +7,7 @@ import (
 	"gotest.tools/v3/icmd"
 )
 
+// ls
 func TestListBuckets(t *testing.T) {
 	t.Parallel()
 
@@ -35,6 +36,7 @@ func TestListBuckets(t *testing.T) {
 	})
 }
 
+// -json ls bucket
 func TestListBucketsJSON(t *testing.T) {
 	t.Parallel()
 
@@ -63,6 +65,7 @@ func TestListBucketsJSON(t *testing.T) {
 	}, jsonCheck(true))
 }
 
+// ls bucket/object
 func TestListSingleS3Object(t *testing.T) {
 	t.Parallel()
 
@@ -88,6 +91,7 @@ func TestListSingleS3Object(t *testing.T) {
 	})
 }
 
+// -json ls bucket/object
 func TestListSingleS3ObjectJSON(t *testing.T) {
 	t.Parallel()
 
@@ -113,6 +117,7 @@ func TestListSingleS3ObjectJSON(t *testing.T) {
 	}, jsonCheck(true))
 }
 
+// ls bucket/*.ext
 func TestListSingleWildcardS3Object(t *testing.T) {
 	t.Parallel()
 
@@ -136,9 +141,10 @@ func TestListSingleWildcardS3Object(t *testing.T) {
 		1: suffix("322 testfile2.txt"),
 		2: suffix("330 testfile3.txt"),
 		3: equals(""),
-	})
+	}, alignment(true))
 }
 
+// ls bucket/*/object*.ext
 func TestListMultipleWildcardS3Object(t *testing.T) {
 	t.Parallel()
 
@@ -173,9 +179,10 @@ func TestListMultipleWildcardS3Object(t *testing.T) {
 		4: suffix("312 d/foo/bar/testfile8.txt"),
 		5: suffix("309 f/txt/testfile10.txt"),
 		6: equals(""),
-	})
+	}, alignment(true))
 }
 
+// ls bucket/prefix/object*.ext
 func TestListMultipleWildcardS3ObjectWithPrefix(t *testing.T) {
 	t.Parallel()
 
@@ -207,9 +214,10 @@ func TestListMultipleWildcardS3ObjectWithPrefix(t *testing.T) {
 		1: suffix("304 testfile2.txt"),
 		2: suffix("304 testfile3.txt"),
 		3: equals(""),
-	})
+	}, alignment(true))
 }
 
+// ls bucket
 func TestListS3ObjectsAndFolders(t *testing.T) {
 	t.Parallel()
 
@@ -245,9 +253,10 @@ func TestListS3ObjectsAndFolders(t *testing.T) {
 		6: suffix("? 298 report.gz"),
 		7: suffix("? 302 testfile1.txt"),
 		8: equals(""),
-	})
+	}, alignment(true))
 }
 
+// ls bucket/prefix
 func TestListS3ObjectsAndFoldersWithPrefix(t *testing.T) {
 	t.Parallel()
 
@@ -272,9 +281,10 @@ func TestListS3ObjectsAndFoldersWithPrefix(t *testing.T) {
 		0: suffix("DIR t/"),
 		1: suffix("? 302 testfile1.txt"),
 		2: equals(""),
-	})
+	}, alignment(true))
 }
 
+// ls bucket/*/object*.ext
 func TestListNonexistingS3ObjectInGivenPrefix(t *testing.T) {
 	t.Parallel()
 
@@ -300,6 +310,7 @@ func TestListNonexistingS3ObjectInGivenPrefix(t *testing.T) {
 	}, strictLineCheck(false))
 }
 
+// ls bucket/object (nonexistent)
 func TestListNonexistingS3Object(t *testing.T) {
 	t.Parallel()
 
@@ -324,6 +335,7 @@ func TestListNonexistingS3Object(t *testing.T) {
 	}, strictLineCheck(false))
 }
 
+// ls -e bucket
 func TestListS3ObjectsWithDashE(t *testing.T) {
 	t.Parallel()
 
@@ -346,9 +358,10 @@ func TestListS3ObjectsWithDashE(t *testing.T) {
 		0: match(`^ \? \w+ \d+ testfile1.txt$`),
 		1: match(`^ \? \w+ \d+ testfile2.txt$`),
 		2: equals(""),
-	}, trimMatch(dateRe))
+	}, trimMatch(dateRe), alignment(true))
 }
 
+// ls -H bucket
 func TestListS3ObjectsWithDashH(t *testing.T) {
 	t.Parallel()
 
@@ -371,5 +384,5 @@ func TestListS3ObjectsWithDashH(t *testing.T) {
 		0: match(`^ \? 215.1K testfile1.txt$`),
 		1: match(`^ \? 264.0K testfile2.txt$`),
 		2: equals(""),
-	}, trimMatch(dateRe))
+	}, trimMatch(dateRe), alignment(true))
 }
diff --git a/e2e/util_test.go b/e2e/util_test.go
index a9fc77c..20770c8 100644
--- a/e2e/util_test.go
+++ b/e2e/util_test.go
@@ -299,6 +299,7 @@ type assertOpts struct {
 	strict      bool
 	sort        bool
 	json        bool
+	alignment   bool
 	trimRegexes []*regexp.Regexp
 }
 
@@ -322,6 +323,12 @@ func jsonCheck(v bool) func(*assertOpts) {
 	}
 }
 
+func alignment(v bool) func(*assertOpts) {
+	return func(opts *assertOpts) {
+		opts.alignment = v
+	}
+}
+
 func trimMatch(match string) func(*assertOpts) {
 	re := regexp.MustCompile(match)
 	return func(opts *assertOpts) {
@@ -344,6 +351,7 @@ func assertLines(t *testing.T, actual string, expectedlines map[int]compareFunc,
 		strict:      true,
 		sort:        false,
 		json:        false,
+		alignment:   false,
 		trimRegexes: nil,
 	}
 
@@ -351,6 +359,13 @@ func assertLines(t *testing.T, actual string, expectedlines map[int]compareFunc,
 		fn(&opts)
 	}
 
+	// check alignment before trimming spaces
+	if opts.alignment {
+		if err := checkLineAlignments(actual); err != nil {
+			t.Error(err)
+		}
+	}
+
 	for _, re := range opts.trimRegexes {
 		actual = re.ReplaceAllString(actual, "")
 	}
@@ -398,6 +413,39 @@ func assertLines(t *testing.T, actual string, expectedlines map[int]compareFunc,
 	}
 }
 
+func checkLineAlignments(actual string) error {
+	// use original string. because some characters are
+	// trimmed during line preparation and we need to check
+	// original string
+	actual = strings.TrimSuffix(actual, "\n")
+	lines := strings.Split(actual, "\n")
+
+	lineExists := len(lines) > 0
+	if !lineExists {
+		// nothing to compare
+		return nil
+	}
+
+	sort.Strings(lines)
+
+	var index int
+	for lineno, line := range lines {
+		// format:
+		// 			2020/03/26 09:14:10          1024.0M 1gb
+		//                                  	 	 DIR test/
+		//
+		// only check the alignment of Dir
+		got := strings.LastIndex(line, " ")
+		if index == 0 {
+			index = got
+		}
+		if index != got {
+			return fmt.Errorf("unaligned string, line: %v expected index: %v, got: %v", lineno, index, got)
+		}
+	}
+	return nil
+}
+
 func match(expected string) compareFunc {
 	re := regexp.MustCompile(expected)
 	return func(actual string) error {
diff --git a/storage/s3_test.go b/storage/s3_test.go
index cea41dd..f4cc100 100644
--- a/storage/s3_test.go
+++ b/storage/s3_test.go
@@ -51,6 +51,7 @@ func TestNewSessionPathStyle(t *testing.T) {
 	}
 
 	for _, tc := range testcases {
+		tc := tc
 		t.Run(tc.name, func(t *testing.T) {
 
 			opts := S3Options{Endpoint: tc.endpoint.Hostname()}
diff --git a/storage/url/url_test.go b/storage/url/url_test.go
index ca9e3dd..e1b20fb 100644
--- a/storage/url/url_test.go
+++ b/storage/url/url_test.go
@@ -31,10 +31,11 @@ func TestHasWild(t *testing.T) {
 			want: false,
 		},
 	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			if got := hasGlobCharacter(tt.s); got != tt.want {
-				t.Errorf("HasWild() = %v, want %v", got, tt.want)
+	for _, tc := range tests {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			if got := hasGlobCharacter(tc.s); got != tc.want {
+				t.Errorf("HasWild() = %v, want %v", got, tc.want)
 			}
 		})
 	}
@@ -96,19 +97,20 @@ func TestNew(t *testing.T) {
 			wantFilterRe: regexp.MustCompile(`^key/a/./test/.*?$`).String(),
 		},
 	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			got, err := New(tt.object)
-			if (err != nil) != tt.wantErr {
-				t.Errorf("ParseURL() error = %v, wantErr %v", err, tt.wantErr)
+	for _, tc := range tests {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			got, err := New(tc.object)
+			if (err != nil) != tc.wantErr {
+				t.Errorf("ParseURL() error = %v, wantErr %v", err, tc.wantErr)
 			}
-			if diff := cmp.Diff(tt.want, got, cmpopts.IgnoreUnexported(URL{})); diff != "" {
-				t.Errorf("test case %q: URL mismatch (-want +got):\n%v", tt.name, diff)
+			if diff := cmp.Diff(tc.want, got, cmpopts.IgnoreUnexported(URL{})); diff != "" {
+				t.Errorf("test case %q: URL mismatch (-want +got):\n%v", tc.name, diff)
 
 			}
-			if tt.wantFilterRe != "" {
-				if diff := cmp.Diff(tt.wantFilterRe, got.filterRegex.String()); diff != "" {
-					t.Errorf("test case %q: URL.filterRegex mismatch (-want +got):\n%v", tt.name, diff)
+			if tc.wantFilterRe != "" {
+				if diff := cmp.Diff(tc.wantFilterRe, got.filterRegex.String()); diff != "" {
+					t.Errorf("test case %q: URL.filterRegex mismatch (-want +got):\n%v", tc.name, diff)
 
 				}
 			}
@@ -149,15 +151,16 @@ func TestURLSetPrefixAndFilter(t *testing.T) {
 			},
 		},
 	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			got := tt.before
+	for _, tc := range tests {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			got := tc.before
 			if err := got.setPrefixAndFilter(); err != nil {
 				t.Errorf("unexpected error %v", err)
 			}
 
-			if !reflect.DeepEqual(got, tt.after) {
-				t.Errorf("setPrefixAndFilter() got = %v, want %v", got, tt.after)
+			if !reflect.DeepEqual(got, tc.after) {
+				t.Errorf("setPrefixAndFilter() got = %v, want %v", got, tc.after)
 			}
 		})
 	}
@@ -251,14 +254,15 @@ func TestCheckMatch(t *testing.T) {
 			},
 		},
 	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			u, err := New(tt.url)
+	for _, tc := range tests {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			u, err := New(tc.url)
 			if err != nil {
 				t.Errorf("unexpected error %v", err)
 			}
 
-			for key, matchResult := range tt.keys {
+			for key, matchResult := range tc.keys {
 				got := u.Match(key)
 				if got != matchResult.matched {
 					t.Errorf("Match() got = %v, want %v", got, matchResult.matched)
@@ -303,10 +307,11 @@ func TestParseBatch(t *testing.T) {
 			want:   "b/asset.txt",
 		},
 	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			if got := parseBatch(tt.prefix, tt.key); got != tt.want {
-				t.Errorf("parseBatch() = %v, want %v", got, tt.want)
+	for _, tc := range tests {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			if got := parseBatch(tc.prefix, tc.key); got != tc.want {
+				t.Errorf("parseBatch() = %v, want %v", got, tc.want)
 			}
 		})
 	}
@@ -356,10 +361,11 @@ func TestParseNonBatch(t *testing.T) {
 			want:   "testdir/",
 		},
 	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			if got := parseNonBatch(tt.prefix, tt.key); got != tt.want {
-				t.Errorf("parseNonBatch() = %v, want %v", got, tt.want)
+	for _, tc := range tests {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			if got := parseNonBatch(tc.prefix, tc.key); got != tc.want {
+				t.Errorf("parseNonBatch() = %v, want %v", got, tc.want)
 			}
 		})
 	}