cmd/uplinkng: tests for cp

this adds some stuff to ultest so that the set of
files created by a test can be inspected after so
that we can write some tests for the cp command
to observe that it does what it is supposed to do.

Change-Id: I98b8fb214058140dfbb117baa7acea6a2cc340e1

cmd/uplinkng/cmd_cp.go

@@ -21,6 +21,7 @@ type cmdCp struct {
 
 	recursive bool
 	dryrun    bool
+	progress  bool
 
 	source ulloc.Location
 	dest   ulloc.Location
@@ -36,6 +37,9 @@ func (c *cmdCp) Setup(a clingy.Arguments, f clingy.Flags) {
 	c.dryrun = f.New("dryrun", "Print what operations would happen but don't execute them", false,
 		clingy.Transform(strconv.ParseBool),
 	).(bool)
+	c.progress = f.New("progress", "Show a progress bar when possible", true,
+		clingy.Transform(strconv.ParseBool),
+	).(bool)
 
 	c.source = a.New("source", "Source to copy", clingy.Transform(ulloc.Parse)).(ulloc.Location)
 	c.dest = a.New("dest", "Desination to copy", clingy.Transform(ulloc.Parse)).(ulloc.Location)
@@ -51,7 +55,7 @@ func (c *cmdCp) Execute(ctx clingy.Context) error {
 	if c.recursive {
 		return c.copyRecursive(ctx, fs)
 	}
-	return c.copyFile(ctx, fs, c.source, c.dest, true)
+	return c.copyFile(ctx, fs, c.source, c.dest, c.progress)
 }
 
 func (c *cmdCp) copyRecursive(ctx clingy.Context, fs ulfs.Filesystem) error {
@@ -99,7 +103,7 @@ func (c *cmdCp) copyFile(ctx clingy.Context, fs ulfs.Filesystem, source, dest ul
 	}
 
 	if !source.Std() && !dest.Std() {
-		fmt.Println(copyVerb(source, dest), source, "to", dest)
+		fmt.Fprintln(ctx.Stdout(), copyVerb(source, dest), source, "to", dest)
 	}
 
 	if c.dryrun {

cmd/uplinkng/cmd_cp_test.go

@@ -0,0 +1,117 @@
+// Copyright (C) 2021 Storj Labs, Inc.
+// See LICENSE for copying information.
+
+package main
+
+import (
+	"testing"
+
+	"storj.io/storj/cmd/uplinkng/ultest"
+)
+
+func TestCpDownload(t *testing.T) {
+	state := ultest.Setup(commands,
+		ultest.WithFile("sj://user/file1.txt", "remote"),
+	)
+
+	state.Succeed(t, "cp", "sj://user/file1.txt", "/home/user/file1.txt").RequireFiles(t,
+		ultest.File{Loc: "/home/user/file1.txt", Contents: "remote"},
+		ultest.File{Loc: "sj://user/file1.txt", Contents: "remote"},
+	)
+}
+
+func TestCpDownloadOverwrite(t *testing.T) {
+	state := ultest.Setup(commands,
+		ultest.WithFile("/home/user/file1.txt", "local"),
+		ultest.WithFile("sj://user/file1.txt", "remote"),
+	)
+
+	state.Succeed(t, "cp", "sj://user/file1.txt", "/home/user/file1.txt").RequireFiles(t,
+		ultest.File{Loc: "/home/user/file1.txt", Contents: "remote"},
+		ultest.File{Loc: "sj://user/file1.txt", Contents: "remote"},
+	)
+}
+
+func TestCpUpload(t *testing.T) {
+	state := ultest.Setup(commands,
+		ultest.WithFile("/home/user/file1.txt", "local"),
+		ultest.WithBucket("user"),
+	)
+
+	state.Succeed(t, "cp", "/home/user/file1.txt", "sj://user/file1.txt").RequireFiles(t,
+		ultest.File{Loc: "/home/user/file1.txt", Contents: "local"},
+		ultest.File{Loc: "sj://user/file1.txt", Contents: "local"},
+	)
+}
+
+func TestCpUploadOverwrite(t *testing.T) {
+	state := ultest.Setup(commands,
+		ultest.WithFile("/home/user/file1.txt", "local"),
+		ultest.WithFile("sj://user/file1.txt", "remote"),
+	)
+
+	state.Succeed(t, "cp", "/home/user/file1.txt", "sj://user/file1.txt").RequireFiles(t,
+		ultest.File{Loc: "/home/user/file1.txt", Contents: "local"},
+		ultest.File{Loc: "sj://user/file1.txt", Contents: "local"},
+	)
+}
+
+func TestCpRecursiveDownload(t *testing.T) {
+	state := ultest.Setup(commands,
+		ultest.WithFile("sj://user/file1.txt", "data1"),
+		ultest.WithFile("sj://user/folder1/file2.txt", "data2"),
+		ultest.WithFile("sj://user/folder1/file3.txt", "data3"),
+		ultest.WithFile("sj://user/folder2/folder3/file4.txt", "data4"),
+		ultest.WithFile("sj://user/folder2/folder3/file5.txt", "data5"),
+	)
+
+	state.Succeed(t, "cp", "sj://user", "/home/user/dest", "--recursive").RequireFiles(t,
+		ultest.File{Loc: "sj://user/file1.txt", Contents: "data1"},
+		ultest.File{Loc: "sj://user/folder1/file2.txt", Contents: "data2"},
+		ultest.File{Loc: "sj://user/folder1/file3.txt", Contents: "data3"},
+		ultest.File{Loc: "sj://user/folder2/folder3/file4.txt", Contents: "data4"},
+		ultest.File{Loc: "sj://user/folder2/folder3/file5.txt", Contents: "data5"},
+
+		ultest.File{Loc: "/home/user/dest/file1.txt", Contents: "data1"},
+		ultest.File{Loc: "/home/user/dest/folder1/file2.txt", Contents: "data2"},
+		ultest.File{Loc: "/home/user/dest/folder1/file3.txt", Contents: "data3"},
+		ultest.File{Loc: "/home/user/dest/folder2/folder3/file4.txt", Contents: "data4"},
+		ultest.File{Loc: "/home/user/dest/folder2/folder3/file5.txt", Contents: "data5"},
+	)
+
+	state.Succeed(t, "cp", "sj://user/fo", "/home/user/dest", "--recursive").RequireFiles(t,
+		ultest.File{Loc: "sj://user/file1.txt", Contents: "data1"},
+		ultest.File{Loc: "sj://user/folder1/file2.txt", Contents: "data2"},
+		ultest.File{Loc: "sj://user/folder1/file3.txt", Contents: "data3"},
+		ultest.File{Loc: "sj://user/folder2/folder3/file4.txt", Contents: "data4"},
+		ultest.File{Loc: "sj://user/folder2/folder3/file5.txt", Contents: "data5"},
+
+		ultest.File{Loc: "/home/user/dest/folder1/file2.txt", Contents: "data2"},
+		ultest.File{Loc: "/home/user/dest/folder1/file3.txt", Contents: "data3"},
+		ultest.File{Loc: "/home/user/dest/folder2/folder3/file4.txt", Contents: "data4"},
+		ultest.File{Loc: "/home/user/dest/folder2/folder3/file5.txt", Contents: "data5"},
+	)
+}
+
+func TestCpRecursiveDifficult(t *testing.T) {
+	state := ultest.Setup(commands,
+		ultest.WithFile("sj://user/dot-dot/../foo"),
+		ultest.WithFile("sj://user/dot-dot/../../foo"),
+
+		ultest.WithFile("sj://user//"),
+		ultest.WithFile("sj://user///"),
+		ultest.WithFile("sj://user////"),
+
+		ultest.WithFile("sj://user//starts-slash"),
+
+		ultest.WithFile("sj://user/ends-slash"),
+		ultest.WithFile("sj://user/ends-slash/"),
+		ultest.WithFile("sj://user/ends-slash//"),
+
+		ultest.WithFile("sj://user/mid-slash"),
+		ultest.WithFile("sj://user/mid-slash//2"),
+		ultest.WithFile("sj://user/mid-slash/1"),
+	)
+
+	_ = state
+}

cmd/uplinkng/ulloc/location.go

@@ -174,7 +174,11 @@ func (p Location) RelativeTo(target Location) (string, error) {
 	} else if !strings.HasPrefix(target.key, p.key) {
 		return "", errs.New("cannot make relative location because keys are not prefixes")
 	}
-	return target.key[len(p.key):], nil
+	idx := strings.LastIndexByte(p.key, '/')
+	if idx == -1 {
+		idx = 0
+	}
+	return target.key[idx:], nil
 }
 
 // AppendKey adds the key to the end of the existing key, separating with the

cmd/uplinkng/ultest/filesystem.go

@@ -22,23 +22,22 @@ import (
 
 type testFilesystem struct {
 	stdin   string
-	ops     []Operation
 	created int64
-	files   map[ulloc.Location]byteFileData
+	files   map[ulloc.Location]memFileData
-	pending map[ulloc.Location][]*byteWriteHandle
+	pending map[ulloc.Location][]*memWriteHandle
 	buckets map[string]struct{}
 }
 
 func newTestFilesystem() *testFilesystem {
 	return &testFilesystem{
-		files:   make(map[ulloc.Location]byteFileData),
+		files:   make(map[ulloc.Location]memFileData),
-		pending: make(map[ulloc.Location][]*byteWriteHandle),
+		pending: make(map[ulloc.Location][]*memWriteHandle),
 		buckets: make(map[string]struct{}),
 	}
 }
 
-type byteFileData struct {
+type memFileData struct {
-	data    []byte
+	contents string
 	created  int64
 }
 
@@ -46,23 +45,30 @@ func (tfs *testFilesystem) ensureBucket(name string) {
 	tfs.buckets[name] = struct{}{}
 }
 
+func (tfs *testFilesystem) Files() (files []File) {
+	for loc, mf := range tfs.files {
+		files = append(files, File{
+			Loc:      loc.String(),
+			Contents: mf.contents,
+		})
+	}
+	sort.Slice(files, func(i, j int) bool { return files[i].less(files[j]) })
+	return files
+}
+
 func (tfs *testFilesystem) Close() error {
 	return nil
 }
 
 func (tfs *testFilesystem) Open(ctx clingy.Context, loc ulloc.Location) (_ ulfs.ReadHandle, err error) {
-	defer func() { tfs.ops = append(tfs.ops, newOp("open", loc, err)) }()
+	mf, ok := tfs.files[loc]
-
-	bf, ok := tfs.files[loc]
 	if !ok {
 		return nil, errs.New("file does not exist")
 	}
-	return &byteReadHandle{Buffer: bytes.NewBuffer(bf.data)}, nil
+	return &byteReadHandle{Buffer: bytes.NewBufferString(mf.contents)}, nil
 }
 
 func (tfs *testFilesystem) Create(ctx clingy.Context, loc ulloc.Location) (_ ulfs.WriteHandle, err error) {
-	defer func() { tfs.ops = append(tfs.ops, newOp("create", loc, err)) }()
-
 	if bucket, _, ok := loc.RemoteParts(); ok {
 		if _, ok := tfs.buckets[bucket]; !ok {
 			return nil, errs.New("bucket %q does not exist", bucket)
@@ -70,7 +76,7 @@ func (tfs *testFilesystem) Create(ctx clingy.Context, loc ulloc.Location) (_ ulf
 	}
 
 	tfs.created++
-	wh := &byteWriteHandle{
+	wh := &memWriteHandle{
 		buf: bytes.NewBuffer(nil),
 		loc: loc,
 		tfs: tfs,
@@ -84,11 +90,11 @@ func (tfs *testFilesystem) Create(ctx clingy.Context, loc ulloc.Location) (_ ulf
 
 func (tfs *testFilesystem) ListObjects(ctx context.Context, prefix ulloc.Location, recursive bool) (ulfs.ObjectIterator, error) {
 	var infos []ulfs.ObjectInfo
-	for loc, bf := range tfs.files {
+	for loc, mf := range tfs.files {
 		if loc.HasPrefix(prefix) {
 			infos = append(infos, ulfs.ObjectInfo{
 				Loc:     loc,
-				Created: time.Unix(bf.created, 0),
+				Created: time.Unix(mf.created, 0),
 			})
 		}
 	}
@@ -145,7 +151,7 @@ func (b *byteReadHandle) Info() ulfs.ObjectInfo { return ulfs.ObjectInfo{} }
 // ulfs.WriteHandle
 //
 
-type byteWriteHandle struct {
+type memWriteHandle struct {
 	buf  *bytes.Buffer
 	loc  ulloc.Location
 	tfs  *testFilesystem
@@ -153,33 +159,31 @@ type byteWriteHandle struct {
 	done bool
 }
 
-func (b *byteWriteHandle) Write(p []byte) (int, error) {
+func (b *memWriteHandle) Write(p []byte) (int, error) {
 	return b.buf.Write(p)
 }
 
-func (b *byteWriteHandle) Commit() error {
+func (b *memWriteHandle) Commit() error {
 	if err := b.close(); err != nil {
 		return err
 	}
 
-	b.tfs.ops = append(b.tfs.ops, newOp("commit", b.loc, nil))
+	b.tfs.files[b.loc] = memFileData{
-	b.tfs.files[b.loc] = byteFileData{
+		contents: b.buf.String(),
-		data:    b.buf.Bytes(),
 		created:  b.cre,
 	}
 	return nil
 }
 
-func (b *byteWriteHandle) Abort() error {
+func (b *memWriteHandle) Abort() error {
 	if err := b.close(); err != nil {
 		return err
 	}
 
-	b.tfs.ops = append(b.tfs.ops, newOp("append", b.loc, nil))
 	return nil
 }
 
-func (b *byteWriteHandle) close() error {
+func (b *memWriteHandle) close() error {
 	if b.done {
 		return errs.New("already done")
 	}

cmd/uplinkng/ultest/result.go

@@ -4,6 +4,7 @@
 package ultest
 
 import (
+	"sort"
 	"strings"
 	"testing"
 
@@ -18,7 +19,7 @@ type Result struct {
 	Stderr string
 	Ok     bool
 	Err    error
-	Operations []Operation
+	Files  []File
 }
 
 // RequireSuccess fails if the Result did not observe a successful execution.
@@ -32,20 +33,32 @@ func (r Result) RequireSuccess(t *testing.T) {
 }
 
 // RequireFailure fails if the Result did not observe a failed execution.
-func (r Result) RequireFailure(t *testing.T) {
+func (r Result) RequireFailure(t *testing.T) Result {
 	require.False(t, r.Ok && r.Err == nil, "command ran with no error")
+	return r
 }
 
 // RequireStdout requires that the execution wrote to stdout the provided string.
 // Blank lines are ignored and all lines are space trimmed for the comparison.
-func (r Result) RequireStdout(t *testing.T, stdout string) {
+func (r Result) RequireStdout(t *testing.T, stdout string) Result {
 	require.Equal(t, trimNewlineSpaces(stdout), trimNewlineSpaces(r.Stdout))
+	return r
 }
 
 // RequireStderr requires that the execution wrote to stderr the provided string.
 // Blank lines are ignored and all lines are space trimmed for the comparison.
-func (r Result) RequireStderr(t *testing.T, stderr string) {
+func (r Result) RequireStderr(t *testing.T, stderr string) Result {
 	require.Equal(t, trimNewlineSpaces(stderr), trimNewlineSpaces(r.Stderr))
+	return r
+}
+
+// RequireFiles requires that the set of files provided are all of the files that
+// existed at the end of the execution.
+func (r Result) RequireFiles(t *testing.T, files ...File) Result {
+	files = append([]File(nil), files...)
+	sort.Slice(files, func(i, j int) bool { return files[i].less(files[j]) })
+	require.Equal(t, files, r.Files)
+	return r
 }
 
 func parseErrors(s string) []string {
@@ -74,18 +87,14 @@ func trimNewlineSpaces(s string) string {
 	return strings.Join(lines[:j], "\n")
 }
 
-// Operation represents some kind of filesystem operation that happened
+// File represents a file existing either locally or remotely.
-// on some location, and if the operation failed.
+type File struct {
-type Operation struct {
-	Kind  string
 	Loc      string
-	Error bool
+	Contents string
 }
 
-func newOp(kind string, loc ulloc.Location, err error) Operation {
+func (f File) less(g File) bool {
-	return Operation{
+	fl, _ := ulloc.Parse(f.Loc)
-		Kind:  kind,
+	gl, _ := ulloc.Parse(g.Loc)
-		Loc:   loc.String(),
+	return fl.Less(gl)
-		Error: err != nil,
-	}
 }

cmd/uplinkng/ultest/setup.go

@@ -10,7 +10,6 @@ import (
 
 	"github.com/stretchr/testify/require"
 	"github.com/zeebo/clingy"
-	"github.com/zeebo/errs"
 
 	"storj.io/storj/cmd/uplinkng/ulfs"
 	"storj.io/storj/cmd/uplinkng/ulloc"
@@ -57,9 +56,10 @@ func (st State) Run(t *testing.T, args ...string) Result {
 	var stdout bytes.Buffer
 	var stderr bytes.Buffer
 	var stdin bytes.Buffer
-	var ops []Operation
 	var ran bool
 
+	tfs := newTestFilesystem()
+
 	ok, err := clingy.Environment{
 		Name: "uplink-test",
 		Args: args,
@@ -69,13 +69,9 @@ func (st State) Run(t *testing.T, args ...string) Result {
 		Stderr: &stderr,
 
 		Wrap: func(ctx clingy.Context, cmd clingy.Cmd) error {
-			tfs := newTestFilesystem()
 			for _, opt := range st.opts {
-				if err := opt.fn(ctx, tfs); err != nil {
+				opt.fn(t, ctx, tfs)
-					return errs.Wrap(err)
 			}
-			}
-			tfs.ops = nil
 
 			if len(tfs.stdin) > 0 {
 				_, _ = stdin.WriteString(tfs.stdin)
@@ -88,67 +84,89 @@ func (st State) Run(t *testing.T, args ...string) Result {
 			}
 
 			ran = true
-			err := cmd.Execute(ctx)
+			return cmd.Execute(ctx)
-			ops = tfs.ops
-			return err
 		},
 	}.Run(context.Background(), st.cmds)
 
 	if ok && err == nil {
 		require.True(t, ran, "no command was executed: %q", args)
 	}
+
 	return Result{
 		Stdout: stdout.String(),
 		Stderr: stderr.String(),
 		Ok:     ok,
 		Err:    err,
-		Operations: ops,
+		Files:  tfs.Files(),
 	}
 }
 
 // ExecuteOption allows one to control the environment that a command executes in.
 type ExecuteOption struct {
-	fn func(ctx clingy.Context, tfs *testFilesystem) error
+	fn func(t *testing.T, ctx clingy.Context, tfs *testFilesystem)
+}
+
+// WithFilesystem lets one do arbitrary setup on the filesystem in a callback.
+func WithFilesystem(cb func(t *testing.T, ctx clingy.Context, fs ulfs.Filesystem)) ExecuteOption {
+	return ExecuteOption{func(t *testing.T, ctx clingy.Context, tfs *testFilesystem) {
+		cb(t, ctx, tfs)
+	}}
+}
+
+// WithBucket ensures the bucket exists.
+func WithBucket(name string) ExecuteOption {
+	return ExecuteOption{func(_ *testing.T, _ clingy.Context, tfs *testFilesystem) {
+		tfs.ensureBucket(name)
+	}}
 }
 
 // WithStdin sets the command to execute with the provided string as standard input.
 func WithStdin(stdin string) ExecuteOption {
-	return ExecuteOption{func(_ clingy.Context, tfs *testFilesystem) error {
+	return ExecuteOption{func(_ *testing.T, _ clingy.Context, tfs *testFilesystem) {
 		tfs.stdin = stdin
-		return nil
 	}}
 }
 
 // WithFile sets the command to execute with a file created at the given location.
-func WithFile(location string) ExecuteOption {
+func WithFile(location string, contents ...string) ExecuteOption {
-	return ExecuteOption{func(ctx clingy.Context, tfs *testFilesystem) error {
+	contents = append([]string(nil), contents...)
+	return ExecuteOption{func(t *testing.T, ctx clingy.Context, tfs *testFilesystem) {
 		loc, err := ulloc.Parse(location)
-		if err != nil {
+		require.NoError(t, err)
-			return err
+
-		}
 		if bucket, _, ok := loc.RemoteParts(); ok {
 			tfs.ensureBucket(bucket)
 		}
+
 		wh, err := tfs.Create(ctx, loc)
-		if err != nil {
+		require.NoError(t, err)
-			return err
+		defer func() { _ = wh.Abort() }()
+
+		for _, content := range contents {
+			_, err := wh.Write([]byte(content))
+			require.NoError(t, err)
 		}
-		return wh.Commit()
+		if len(contents) == 0 {
+			_, err := wh.Write([]byte(location))
+			require.NoError(t, err)
+		}
+
+		require.NoError(t, wh.Commit())
 	}}
 }
 
 // WithPendingFile sets the command to execute with a pending upload happening to
 // the provided location.
 func WithPendingFile(location string) ExecuteOption {
-	return ExecuteOption{func(ctx clingy.Context, tfs *testFilesystem) error {
+	return ExecuteOption{func(t *testing.T, ctx clingy.Context, tfs *testFilesystem) {
 		loc, err := ulloc.Parse(location)
-		if err != nil {
+		require.NoError(t, err)
-			return err
+
-		}
 		if bucket, _, ok := loc.RemoteParts(); ok {
 			tfs.ensureBucket(bucket)
 		}
+
 		_, err = tfs.Create(ctx, loc)
-		return err
+		require.NoError(t, err)
 	}}
 }