ronantakizawa/codereview-bench huggingface dataset:
Repository: rclone/rclone
File: backend/googlecloudstorage/googlecloudstorage.go (Go)

Code before:
			if !f.opt.BucketPolicyOnly {
				insertBucket.PredefinedAcl(f.opt.BucketACL)
			}
			_, err = insertBucket.Context(ctx).Do()
			return shouldRetry(ctx, err)
		})
	}, nil)
}

// Rmdir deletes the bucket if the fs is at the root
//
// Returns an error if it isn't empty: Error 409: The bucket you tried
// to delete was not empty.
func (f *Fs) Rmdir(ctx context.Context, dir string) (err error) {
	// Add trailing slash to folder path
	dirFilePath := fmt.Sprintf("%s/", dir)
	if len(f.rootDirectory) != 0 {
		dirFilePath = fmt.Sprintf("%s/%s", f.rootDirectory, dirFilePath)
	}
	// Query directory marker object before attempting to remove it.
	e := f.pacer.Call(func() (bool, error) {
		_, e := f.svc.Objects.Get(f.rootBucket, dirFilePath).Context(ctx).Do()
		return shouldRetry(ctx, e)
	})
	if e == nil {
		// Using API directly due to trailing slash being removed before Remove() can get full path
		e := f.pacer.Call(func() (bool, error) {
			e := f.svc.Objects.Delete(f.rootBucket, dirFilePath).Context(ctx).Do()
			return shouldRetry(ctx, e)
		})
		if e != nil {
			return e
		}
	}
	bucket, directory := f.split(dir)
	if bucket == "" || directory != "" {
		return nil
	}
	return f.cache.Remove(bucket, func() error {
		return f.pacer.Call(func() (bool, error) {
			err = f.svc.Buckets.Delete(bucket).Context(ctx).Do()
			return shouldRetry(ctx, err)
		})
	})
}

// Precision returns the precision
func (f *Fs) Precision() time.Duration {
	return time.Nanosecond
}


Review comment (refactor):
I'd prefer to call Object.Remove here or something factored out of Object.Remove rather than duplicate this.

Code after:
	list := walk.NewListRHelper(callback)
	listR := func(bucket, directory, prefix string, addBucket bool) error {
		return f.list(ctx, bucket, directory, prefix, addBucket, true, func(remote string, object *storage.Object, isDirectory bool) error {
			entry, err := f.itemToDirEntry(ctx, remote, object, isDirectory)
			if err != nil {
				return err
			}
			return list.Add(entry)
		})
	}
	if bucket == "" {
		entries, err := f.listBuckets(ctx)
		if err != nil {
			return err
		}
		for _, entry := range entries {
			err = list.Add(entry)
			if err != nil {
				return err
			}
			bucket := entry.Remote()
			err = listR(bucket, "", f.rootDirectory, true)
			if err != nil {
				return err
			}
			// bucket must be present if listing succeeded
			f.cache.MarkOK(bucket)
		}
	} else {
		err = listR(bucket, directory, f.rootDirectory, f.rootBucket == "")
		if err != nil {
			return err
		}
		// bucket must be present if listing succeeded
		f.cache.MarkOK(bucket)
	}
	return list.Flush()
}

// Put the object into the bucket
//
// Copy the reader in to the new object which is returned.
//
// The new object may have been created if an error is returned
func (f *Fs) Put(ctx context.Context, in io.Reader, src fs.ObjectInfo, options ...fs.OpenOption) (fs.Object, error) {
	// Temporary Object under construction
	o := &Object{
		fs:     f,
		remote: src.Remote(),
	}
	return o, o.Update(ctx, in, src, options...)
Repository: rclone/rclone
File: vfs/dir.go (Go)

Code before:
	return
}

func (d *Dir) cachedNode(relativePath string) Node {
	segments := strings.Split(strings.Trim(relativePath, "/"), "/")
	var node Node = d
	for _, s := range segments {
		if s == "" {
			continue
		}
		if dir, ok := node.(*Dir); ok {
			dir.mu.Lock()
			node = dir.items[s]
			dir.mu.Unlock()

			if node != nil {
				continue
			}
		}
		return nil
	}

	return node
}

// Update the object but don't update the directory cache - for use by
// the directory cache
func (d *Dir) setObjectNoUpdate(newDir fs.Directory) {
	d.modTimeMu.Lock()
	d.modTime = newDir.ModTime(context.TODO())
	d.modTimeMu.Unlock()
	d.mu.Lock()
	d.entry = newDir
	d.mu.Unlock()
}

// Stat looks up a specific entry in the receiver.
//
// Stat should return a Node corresponding to the entry.  If the
// name does not exist in the directory, Stat should return ENOENT.
//
// Stat need not to handle the names "." and "..".
func (d *Dir) Stat(name string) (node Node, err error) {
	// fs.Debugf(path, "Dir.Stat")
	node, err = d.stat(name)
	if err != nil {
		if err != ENOENT {
			fs.Errorf(d, "Dir.Stat error: %v", err)
		}
		return nil, err
	}

Review comment (bug):
I think this comment is wrong. It is certainly confusing me!

I think you mean something like this?

```go
// Update the directory in the directory cache with a new fs.Directory.
// For use by the directory cache.
// Does not update the VFS cache.
```

Code after:
}

func (d *Dir) cachedNode(relativePath string) Node {
	segments := strings.Split(strings.Trim(relativePath, "/"), "/")
	var node Node = d
	for _, s := range segments {
		if s == "" {
			continue
		}
		if dir, ok := node.(*Dir); ok {
			dir.mu.Lock()
			node = dir.items[s]
			dir.mu.Unlock()

			if node != nil {
				continue
			}
		}
		return nil
	}

	return node
}

// Stat looks up a specific entry in the receiver.
//
// Stat should return a Node corresponding to the entry.  If the
// name does not exist in the directory, Stat should return ENOENT.
//
// Stat need not to handle the names "." and "..".
func (d *Dir) Stat(name string) (node Node, err error) {
	// fs.Debugf(path, "Dir.Stat")
	node, err = d.stat(name)
	if err != nil {
		if err != ENOENT {
			fs.Errorf(d, "Dir.Stat error: %v", err)
		}
		return nil, err
	}
	// fs.Debugf(path, "Dir.Stat OK")
	return node, nil
}

// ReadDirAll reads the contents of the directory sorted
func (d *Dir) ReadDirAll() (items Nodes, err error) {
	// fs.Debugf(d.path, "Dir.ReadDirAll")
	d.mu.Lock()
	err = d._readDir()
	if err != nil {
		fs.Debugf(d.path, "Dir.ReadDirAll error: %v", err)
		d.mu.Unlock()
Repository: rclone/rclone
File: vfs/vfs.go (Go)

Code before:
	// Create root directory
	vfs.root = newDir(vfs, f, nil, fsDir)

	// Start polling function
	features := vfs.f.Features()
	if do := features.ChangeNotify; do != nil {
		vfs.pollChan = make(chan time.Duration)
		do(context.TODO(), vfs.root.changeNotify, vfs.pollChan)
		vfs.pollChan <- vfs.Opt.PollInterval
	} else if vfs.Opt.PollInterval > 0 {
		fs.Infof(f, "poll-interval is not supported by this remote")
	}

	// Warn if can't stream
	if !vfs.Opt.ReadOnly && vfs.Opt.CacheMode < vfscommon.CacheModeWrites && features.PutStream == nil {
		fs.Logf(f, "--vfs-cache-mode writes or full is recommended for this remote as it can't stream")
	}

	// Pin the Fs into the cache so that when we use cache.NewFs
	// with the same remote string we get this one. The Pin is
	// removed when the vfs is finalized
	cache.PinUntilFinalized(f, vfs)
	
	if vfs.Opt.Refresh {
		fs.Debugf(f, "Refreshing VFS directory cache")
		vfs.root.readDirTree()
	}

	// This can take some time so do it after the Pin
	vfs.SetCacheMode(vfs.Opt.CacheMode)

	return vfs
}

// Stats returns info about the VFS
func (vfs *VFS) Stats() (out rc.Params) {
	out = make(rc.Params)
	out["fs"] = fs.ConfigString(vfs.f)
	out["opt"] = vfs.Opt
	out["inUse"] = atomic.LoadInt32(&vfs.inUse)

	var (
		dirs  int
		files int
	)
	vfs.root.walk(func(d *Dir) {
		dirs++
		files += len(d.items)
	})
	inf := make(rc.Params)
	out["metadataCache"] = inf

Review comment (suggestion):
This is still synchronous. Run it in a goroutine by adding `go` in front of the statement to make it async.

Code after:
	// Create root directory
	vfs.root = newDir(vfs, f, nil, fsDir)

	// Start polling function
	features := vfs.f.Features()
	if do := features.ChangeNotify; do != nil {
		vfs.pollChan = make(chan time.Duration)
		do(context.TODO(), vfs.root.changeNotify, vfs.pollChan)
		vfs.pollChan <- vfs.Opt.PollInterval
	} else if vfs.Opt.PollInterval > 0 {
		fs.Infof(f, "poll-interval is not supported by this remote")
	}

	// Warn if can't stream
	if !vfs.Opt.ReadOnly && vfs.Opt.CacheMode < vfscommon.CacheModeWrites && features.PutStream == nil {
		fs.Logf(f, "--vfs-cache-mode writes or full is recommended for this remote as it can't stream")
	}

	// Pin the Fs into the cache so that when we use cache.NewFs
	// with the same remote string we get this one. The Pin is
	// removed when the vfs is finalized
	cache.PinUntilFinalized(f, vfs)

	// Refresh the dircache if required
	if vfs.Opt.Refresh {
		go vfs.refresh()
	}

	// This can take some time so do it after the Pin
	vfs.SetCacheMode(vfs.Opt.CacheMode)

	return vfs
}

// refresh the directory cache for all directories
func (vfs *VFS) refresh() {
	fs.Debugf(vfs.f, "Refreshing VFS directory cache")
	err := vfs.root.readDirTree()
	if err != nil {
		fs.Errorf(vfs.f, "Error refreshing VFS directory cache: %v", err)
	}
}

// Stats returns info about the VFS
func (vfs *VFS) Stats() (out rc.Params) {
	out = make(rc.Params)
	out["fs"] = fs.ConfigString(vfs.f)
	out["opt"] = vfs.Opt
	out["inUse"] = vfs.inUse.Load()

	var (
Repository: rclone/rclone
File: fs/operations/operations.go (Go)

Code before:
						for _, option := range ci.UploadHeaders {
							options = append(options, option)
						}
						if ci.MetadataSet != nil {
							options = append(options, fs.MetadataOption(ci.MetadataSet))
						}
						if doUpdate {
							actionTaken = "Copied (replaced existing)"
							err = dst.Update(ctx, in, wrappedSrc, options...)
						} else {
							actionTaken = "Copied (new)"
							dst, err = f.Put(ctx, in, wrappedSrc, options...)
						}
						closeErr := in.Close()
						if err == nil {
							newDst = dst
							err = closeErr
						}
					}
				}
			}

			// Move the copied file to its real destination path.
			if copyRename {
				if err == nil {
					if doMove := f.Features().Move; doMove != nil {
						fs.Debugf(newDst, "move to new destination")
						dst, err = doMove(ctx, newDst, remote)
						if err == nil {
							newDst = dst
						}
					}
				}
				// TODO if the above download failed, we're left behind
				// with a .partial-rngstring file.  What's a good way to
				// clean it up in this retry-loop?
			}
		}
		tries++
		if tries >= maxTries {
			break
		}
		// Retry if err returned a retry error
		if fserrors.ContextError(ctx, &err) {
			break
		}
		var retry bool
		if fserrors.IsRetryError(err) || fserrors.ShouldRetry(err) {
			retry = true
		} else if t, ok := pacer.IsRetryAfter(err); ok {
			fs.Debugf(src, "Sleeping for %v (as indicated by the server) to obey Retry-After error: %v", t, err)

Review comment (suggestion):
We tested for Move or Copy above and you are only using Move.

Perhaps we should limit it to Move only so change the above test as all of the backends in question implement Move directly.

Code after:
						var meta fs.Metadata
						if ci.Metadata {
							meta, err = fs.GetMetadata(ctx, src)
							if err != nil {
								fs.Errorf(src, "Failed to read metadata: %v", err)
							}
						}
						// NB Rcat closes in0
						dst, err = Rcat(ctx, f, remotePartial, in0, src.ModTime(ctx), meta)
						newDst = dst
					} else {
						in := tr.Account(ctx, in0).WithBuffer() // account and buffer the transfer
						var wrappedSrc fs.ObjectInfo = src
						// We try to pass the original object if possible
						if src.Remote() != remotePartial {
							wrappedSrc = fs.NewOverrideRemote(src, remotePartial)
						}
						options := []fs.OpenOption{hashOption}
						for _, option := range ci.UploadHeaders {
							options = append(options, option)
						}
						if ci.MetadataSet != nil {
							options = append(options, fs.MetadataOption(ci.MetadataSet))
						}
						if doUpdate && inplace {
							err = dst.Update(ctx, in, wrappedSrc, options...)
						} else {
							dst, err = f.Put(ctx, in, wrappedSrc, options...)
						}
						if doUpdate {
							actionTaken = "Copied (replaced existing)"
						} else {
							actionTaken = "Copied (new)"
						}
						closeErr := in.Close()
						if err == nil {
							newDst = dst
							err = closeErr
						}
					}
				}
			}
		}
		tries++
		if tries >= maxTries {
			break
		}
		// Retry if err returned a retry error
		if fserrors.ContextError(ctx, &err) {
			break
		}
Repository: rclone/rclone
File: fs/filter/filter.go (Go)

Code before:
		rules = append(rules, "--- Metadata filter rules ---")
		for _, metaRule := range f.metaRules.rules {
			rules = append(rules, metaRule.String())
		}
	}
	return strings.Join(rules, "\n")
}

// HaveFilesFrom returns true if --files-from has been supplied
func (f *Filter) HaveFilesFrom() bool {
	return f.files != nil
}

var errFilesFromNotSet = errors.New("--files-from not set so can't use Filter.ListR")

// MakeListR makes function to return all the files set using --files-from
func (f *Filter) MakeListR(ctx context.Context, NewObject func(ctx context.Context, remote string) (fs.Object, error)) fs.ListRFn {
	return func(ctx context.Context, dir string, callback fs.ListRCallback) error {
		ci := fs.GetConfig(ctx)
		if !f.HaveFilesFrom() {
			return errFilesFromNotSet
		}
		var (
			checkers = ci.Checkers
			remotes  = make(chan string, checkers)
			g, gCtx  = errgroup.WithContext(ctx)
		)
		for i := 0; i < checkers; i++ {
			g.Go(func() (err error) {
				var entries = make(fs.DirEntries, 1)
				for remote := range remotes {
					entries[0], err = NewObject(ctx, remote)
					if err == fs.ErrorObjectNotFound {
						// Skip files that are not found
					} else if err != nil {
						return err
					} else {
						err = callback(entries)
						if err != nil {
							return err
						}
					}
				}
				return nil
			})
		}
		go func() {
			defer close(remotes)
			for remote := range f.files {
				select {
				case remotes <- remote:

Review comment (suggestion):
If we are making a gCtx then we should use it instead of ctx in call to `NewObject` below

Code after:
		rules = append(rules, "--- Metadata filter rules ---")
		for _, metaRule := range f.metaRules.rules {
			rules = append(rules, metaRule.String())
		}
	}
	return strings.Join(rules, "\n")
}

// HaveFilesFrom returns true if --files-from has been supplied
func (f *Filter) HaveFilesFrom() bool {
	return f.files != nil
}

var errFilesFromNotSet = errors.New("--files-from not set so can't use Filter.ListR")

// MakeListR makes function to return all the files set using --files-from
func (f *Filter) MakeListR(ctx context.Context, NewObject func(ctx context.Context, remote string) (fs.Object, error)) fs.ListRFn {
	return func(ctx context.Context, dir string, callback fs.ListRCallback) error {
		ci := fs.GetConfig(ctx)
		if !f.HaveFilesFrom() {
			return errFilesFromNotSet
		}
		var (
			checkers = ci.Checkers
			remotes  = make(chan string, checkers)
			g, gCtx  = errgroup.WithContext(ctx)
		)
		for i := 0; i < checkers; i++ {
			g.Go(func() (err error) {
				var entries = make(fs.DirEntries, 1)
				for remote := range remotes {
					entries[0], err = NewObject(gCtx, remote)
					if err == fs.ErrorObjectNotFound {
						// Skip files that are not found
					} else if err != nil {
						return err
					} else {
						err = callback(entries)
						if err != nil {
							return err
						}
					}
				}
				return nil
			})
		}
	outer:
		for remote := range f.files {
			select {
			case remotes <- remote:
			case <-gCtx.Done():
Repository: rclone/rclone
File: fs/filter/filter.go (Go)

Code before:
		}
		var (
			checkers = ci.Checkers
			remotes  = make(chan string, checkers)
			g, gCtx  = errgroup.WithContext(ctx)
		)
		for i := 0; i < checkers; i++ {
			g.Go(func() (err error) {
				var entries = make(fs.DirEntries, 1)
				for remote := range remotes {
					entries[0], err = NewObject(ctx, remote)
					if err == fs.ErrorObjectNotFound {
						// Skip files that are not found
					} else if err != nil {
						return err
					} else {
						err = callback(entries)
						if err != nil {
							return err
						}
					}
				}
				return nil
			})
		}
		go func() {
			defer close(remotes)
			for remote := range f.files {
				select {
				case remotes <- remote:
					continue
				case <-gCtx.Done():
					return
				}
			}
		}()
		return g.Wait()
	}
}

// UsesDirectoryFilters returns true if the filter uses directory
// filters and false if it doesn't.
//
// This is used in deciding whether to walk directories or use ListR
func (f *Filter) UsesDirectoryFilters() bool {
	if len(f.dirRules.rules) == 0 {
		return false
	}
	rule := f.dirRules.rules[0]
	re := rule.Regexp.String()
	if rule.Include && re == "^.*$" {

Review comment (suggestion):
I think this can be written without the extra go routine which I think makes the logic a bit simpler

```go
	outer:
		for remote := range f.files {
			select {
			case remotes <- remote:
			case <-gCtx.Done():
				break outer
			}
		}
		close(remotes)
```

Code after:
		}
		var (
			checkers = ci.Checkers
			remotes  = make(chan string, checkers)
			g, gCtx  = errgroup.WithContext(ctx)
		)
		for i := 0; i < checkers; i++ {
			g.Go(func() (err error) {
				var entries = make(fs.DirEntries, 1)
				for remote := range remotes {
					entries[0], err = NewObject(gCtx, remote)
					if err == fs.ErrorObjectNotFound {
						// Skip files that are not found
					} else if err != nil {
						return err
					} else {
						err = callback(entries)
						if err != nil {
							return err
						}
					}
				}
				return nil
			})
		}
	outer:
		for remote := range f.files {
			select {
			case remotes <- remote:
			case <-gCtx.Done():
				break outer
			}
		}
		close(remotes)
		return g.Wait()
	}
}

// UsesDirectoryFilters returns true if the filter uses directory
// filters and false if it doesn't.
//
// This is used in deciding whether to walk directories or use ListR
func (f *Filter) UsesDirectoryFilters() bool {
	if len(f.dirRules.rules) == 0 {
		return false
	}
	rule := f.dirRules.rules[0]
	re := rule.Regexp.String()
	if rule.Include && re == "^.*$" {
		return false
	}
Repository: rclone/rclone
File: fs/config/rc_test.go (Go)

Code before:
		require.NoError(t, err)
		require.NotNil(t, out)

		require.NotNil(t, out[testName])
		config := out[testName].(rc.Params)

		assert.Equal(t, "local", config["type"])
		assert.Equal(t, "sausage", config["test_key"])
	})

	t.Run("Get", func(t *testing.T) {
		call := rc.Calls.Get("config/get")
		assert.NotNil(t, call)
		in := rc.Params{
			"name": testName,
		}
		out, err := call.Fn(context.Background(), in)
		require.NoError(t, err)
		require.NotNil(t, out)

		assert.Equal(t, "local", out["type"])
		assert.Equal(t, "sausage", out["test_key"])
	})

	t.Run("ListRemotes", func(t *testing.T) {
		_ = os.Setenv("RCLONE_CONFIG_MY-LOCAL_TYPE", "local")
		call := rc.Calls.Get("config/listremotes")
		assert.NotNil(t, call)
		in := rc.Params{}
		out, err := call.Fn(context.Background(), in)
		require.NoError(t, err)
		require.NotNil(t, out)

		var remotes []string
		err = out.GetStruct("remotes", &remotes)
		require.NoError(t, err)

		assert.Contains(t, remotes, testName)
		assert.Contains(t, remotes, "my-local")
	})

	t.Run("Update", func(t *testing.T) {
		call := rc.Calls.Get("config/update")
		assert.NotNil(t, call)
		in := rc.Params{
			"name": testName,
			"parameters": rc.Params{
				"test_key":  "rutabaga",
				"test_key2": "cabbage",
			},
		}

Review comment (suggestion):
I would tidy this env var up at the end of the function otherwise it might interfere with other tests.

I'd also check the error from os.Setenv which is very easy to do in the test functions.

Something like this (untested)


```go
assert.NoError(t, os.Setenv("RCLONE_CONFIG_MY-LOCAL_TYPE", "local"))
defer func() {
    assert.NoError(t, os.Unsetenv("RCLONE_CONFIG_MY-LOCAL_TYPE"))
}()
```


Code after:
		require.NoError(t, err)
		require.NotNil(t, out)

		require.NotNil(t, out[testName])
		config := out[testName].(rc.Params)

		assert.Equal(t, "local", config["type"])
		assert.Equal(t, "sausage", config["test_key"])
	})

	t.Run("Get", func(t *testing.T) {
		call := rc.Calls.Get("config/get")
		assert.NotNil(t, call)
		in := rc.Params{
			"name": testName,
		}
		out, err := call.Fn(context.Background(), in)
		require.NoError(t, err)
		require.NotNil(t, out)

		assert.Equal(t, "local", out["type"])
		assert.Equal(t, "sausage", out["test_key"])
	})

	t.Run("ListRemotes", func(t *testing.T) {
		assert.NoError(t, os.Setenv("RCLONE_CONFIG_MY-LOCAL_TYPE", "local"))
		defer func() {
			assert.NoError(t, os.Unsetenv("RCLONE_CONFIG_MY-LOCAL_TYPE"))
		}()
		call := rc.Calls.Get("config/listremotes")
		assert.NotNil(t, call)
		in := rc.Params{}
		out, err := call.Fn(context.Background(), in)
		require.NoError(t, err)
		require.NotNil(t, out)

		var remotes []string
		err = out.GetStruct("remotes", &remotes)
		require.NoError(t, err)

		assert.Contains(t, remotes, testName)
		assert.Contains(t, remotes, "my-local")
	})

	t.Run("Update", func(t *testing.T) {
		call := rc.Calls.Get("config/update")
		assert.NotNil(t, call)
		in := rc.Params{
			"name": testName,
			"parameters": rc.Params{
				"test_key":  "rutabaga",
Repository: rclone/rclone
File: fs/accounting/stats_test.go (Go)

Code before:
		total := s.totalDuration()
		s.mu.Unlock()

		assert.Equal(t, startTime.Sub(time1)/time.Second, total/time.Second)
	})
}

func TestRemoteStats(t *testing.T) {
	ctx := context.Background()
	startTime := time.Now()
	time1 := startTime.Add(-40 * time.Second)
	time2 := time1.Add(10 * time.Second)

	t.Run("Single completed transfer", func(t *testing.T) {
		s := NewStats(ctx)
		tr1 := &Transfer{
			startedAt:   time1,
			completedAt: time2,
		}
		s.AddTransfer(tr1)
		rs, err := s.RemoteStats()

		require.NoError(t, err)
		assert.Equal(t, float64(10), rs["transferTime"])
		assert.Greater(t, rs["elapsedTime"], float64(0))
	})
}

// make time ranges from string description for testing
func makeTimeRanges(t *testing.T, in []string) timeRanges {
	trs := make(timeRanges, len(in))
	for i, Range := range in {
		var start, end int64
		n, err := fmt.Sscanf(Range, "%d-%d", &start, &end)
		require.NoError(t, err)
		require.Equal(t, 2, n)
		trs[i] = timeRange{time.Unix(start, 0), time.Unix(end, 0)}
	}
	return trs
}

func (trs timeRanges) toStrings() (out []string) {
	out = []string{}
	for _, tr := range trs {
		out = append(out, fmt.Sprintf("%d-%d", tr.start.Unix(), tr.end.Unix()))
	}
	return out
}

func TestTimeRangeMerge(t *testing.T) {
	for _, test := range []struct {

Review comment (bug):
This failed on Windows - I guess this is a flaky assertion? Wasn't expecting it to ever be 0. I could add in a time.Sleep(1ms) before calling RemoteStats() to ensure we don't have a race condition here

Code after:
		total := s.totalDuration()
		s.mu.Unlock()

		assert.Equal(t, startTime.Sub(time1)/time.Second, total/time.Second)
	})
}

func TestRemoteStats(t *testing.T) {
	ctx := context.Background()
	startTime := time.Now()
	time1 := startTime.Add(-40 * time.Second)
	time2 := time1.Add(10 * time.Second)

	t.Run("Single completed transfer", func(t *testing.T) {
		s := NewStats(ctx)
		tr1 := &Transfer{
			startedAt:   time1,
			completedAt: time2,
		}
		s.AddTransfer(tr1)
		time.Sleep(time.Millisecond)
		rs, err := s.RemoteStats()

		require.NoError(t, err)
		assert.Equal(t, float64(10), rs["transferTime"])
		assert.Greater(t, rs["elapsedTime"], float64(0))
	})
}

// make time ranges from string description for testing
func makeTimeRanges(t *testing.T, in []string) timeRanges {
	trs := make(timeRanges, len(in))
	for i, Range := range in {
		var start, end int64
		n, err := fmt.Sscanf(Range, "%d-%d", &start, &end)
		require.NoError(t, err)
		require.Equal(t, 2, n)
		trs[i] = timeRange{time.Unix(start, 0), time.Unix(end, 0)}
	}
	return trs
}

func (trs timeRanges) toStrings() (out []string) {
	out = []string{}
	for _, tr := range trs {
		out = append(out, fmt.Sprintf("%d-%d", tr.start.Unix(), tr.end.Unix()))
	}
	return out
}

func TestTimeRangeMerge(t *testing.T) {