Repository: rclone/rclone
Pull request #5965 (2022-01-29 17:09:31)

test: use `T.TempDir` to create temporary test directory
<!--
Thank you very much for contributing code or documentation to rclone! Please
fill out the following questions to make it easier for us to review your
changes.

You do not need to check all the boxes below all at once, feel free to take
your time and add more commits. If you're done and ready for review, please
check the last box.
-->

#### What is the purpose of this change?
We can write less code by using the `T.TempDir` function from the `testing` package to create temporary directory. The directory created by `T.TempDir` is automatically removed when the test and all its subtests complete.

Reference: https://pkg.go.dev/testing#T.TempDir

#### Was the change discussed in an issue or in the forum before?
No

#### Checklist

- [x] I have read the [contribution guidelines](https://github.com/rclone/rclone/blob/master/CONTRIBUTING.md#submitting-a-new-feature-or-bug-fix).
- [ ] I have added tests for all changes in this PR if appropriate.
- [ ] I have added documentation for the changes if appropriate.
- [x] All commit messages are in [house style](https://github.com/rclone/rclone/blob/master/CONTRIBUTING.md#commit-messages).
- [x] I'm done, this Pull Request is ready for review :-)


Juneezee: > Nice cleanup! Seems its introduced with go 1.15, which is our current oldest supported version. Did you run through the integration tests as well?

Thanks for reviewing this PR! No, I didn't run through the integration tests.

Patch:
diff --git a/docs/content/docs.md b/docs/content/docs.md
index 0290de904..640d51337 100644
--- a/docs/content/docs.md
+++ b/docs/content/docs.md
@@ -1791,24 +1791,30 @@ The default is to run 4 file transfers in parallel.
 This forces rclone to skip any files which exist on the destination
 and have a modified time that is newer than the source file.
 
-This can be useful when transferring to a remote which doesn't support
-mod times directly (or when using `--use-server-modtime` to avoid extra
-API calls) as it is more accurate than a `--size-only` check and faster
-than using `--checksum`.
+This can be useful in avoiding needless transfers when transferring to
+a remote which doesn't support modification times directly (or when
+using `--use-server-modtime` to avoid extra API calls) as it is more
+accurate than a `--size-only` check and faster than using
+`--checksum`. On such remotes (or when using `--use-server-modtime`)
+the time checked will be the uploaded time.
+
+If an existing destination file has a modification time older than the
+source file's, it will be updated if the sizes are different. If the
+sizes are the same, it will be updated if the checksum is different or
+not available.
 
 If an existing destination file has a modification time equal (within
-the computed modify window precision) to the source file's, it will be
-updated if the sizes are different.  If `--checksum` is set then
-rclone will update the destination if the checksums differ too.
-
-If an existing destination file is older than the source file then
-it will be updated if the size or checksum differs from the source file.
-
-On remotes which don't support mod time directly (or when using
-`--use-server-modtime`) the time checked will be the uploaded time.
-This means that if uploading to one of these remotes, rclone will skip
-any files which exist on the destination and have an uploaded time that
-is newer than the modification time of the source file.
+the computed modify window) to the source file's, it will be updated
+if the sizes are different. The checksum will not be checked in this
+case unless the `--checksum` flag is provided.
+
+In all other cases the file will not be updated.
+
+Consider using the `--modify-window` flag to compensate for time skews
+between the source and the backend, for backends that do not support
+mod times, and instead use uploaded times. However, if the backend
+does not support checksums, note that sync'ing or copying within the
+time skew window may still result in additional transfers for safety.
 
 ### --use-mmap ###
 
diff --git a/docs/content/drive.md b/docs/content/drive.md
index 1fc67400e..23b062dc5 100644
--- a/docs/content/drive.md
+++ b/docs/content/drive.md
@@ -1339,7 +1339,8 @@ credentials", which opens the wizard), then "Create credentials"
 to the next step; if not, click on "CONFIGURE CONSENT SCREEN" button 
 (near the top right corner of the right panel), then select "External"
 and click on "CREATE"; on the next screen, enter an "Application name"
-("rclone" is OK) then click on "Save" (all other data is optional). 
+("rclone" is OK); enter "User Support Email" (your own email is OK); 
+enter "Developer Contact Email" (your own email is OK); then click on "Save" (all other data is optional). 
 Click again on "Credentials" on the left panel to go back to the 
 "Credentials" screen.
 


Test patch:
diff --git a/backend/drive/drive_internal_test.go b/backend/drive/drive_internal_test.go
index cd06d15cd..2a21fd574 100644
--- a/backend/drive/drive_internal_test.go
+++ b/backend/drive/drive_internal_test.go
@@ -422,11 +422,7 @@ func (f *Fs) InternalTestCopyID(t *testing.T) {
 	require.NoError(t, err)
 	o := obj.(*Object)
 
-	dir, err := ioutil.TempDir("", "rclone-drive-copyid-test")
-	require.NoError(t, err)
-	defer func() {
-		_ = os.RemoveAll(dir)
-	}()
+	dir := t.TempDir()
 
 	checkFile := func(name string) {
 		filePath := filepath.Join(dir, name)
@@ -491,19 +487,11 @@ func (f *Fs) InternalTestAgeQuery(t *testing.T) {
 	subFs, isDriveFs := subFsResult.(*Fs)
 	require.True(t, isDriveFs)
 
-	tempDir1, err := ioutil.TempDir("", "rclone-drive-agequery1-test")
-	require.NoError(t, err)
-	defer func() {
-		_ = os.RemoveAll(tempDir1)
-	}()
+	tempDir1 := t.TempDir()
 	tempFs1, err := fs.NewFs(defCtx, tempDir1)
 	require.NoError(t, err)
 
-	tempDir2, err := ioutil.TempDir("", "rclone-drive-agequery2-test")
-	require.NoError(t, err)
-	defer func() {
-		_ = os.RemoveAll(tempDir2)
-	}()
+	tempDir2 := t.TempDir()
 	tempFs2, err := fs.NewFs(defCtx, tempDir2)
 	require.NoError(t, err)
 
diff --git a/backend/union/union_internal_test.go b/backend/union/union_internal_test.go
index 8171f593f..22443dd05 100644
--- a/backend/union/union_internal_test.go
+++ b/backend/union/union_internal_test.go
@@ -4,8 +4,6 @@ import (
 	"bytes"
 	"context"
 	"fmt"
-	"io/ioutil"
-	"os"
 	"testing"
 	"time"
 
@@ -20,19 +18,12 @@ import (
 )
 
 // MakeTestDirs makes directories in /tmp for testing
-func MakeTestDirs(t *testing.T, n int) (dirs []string, clean func()) {
+func MakeTestDirs(t *testing.T, n int) (dirs []string) {
 	for i := 1; i <= n; i++ {
-		dir, err := ioutil.TempDir("", fmt.Sprintf("rclone-union-test-%d", n))
-		require.NoError(t, err)
+		dir := t.TempDir()
 		dirs = append(dirs, dir)
 	}
-	clean = func() {
-		for _, dir := range dirs {
-			err := os.RemoveAll(dir)
-			assert.NoError(t, err)
-		}
-	}
-	return dirs, clean
+	return dirs
 }
 
 func (f *Fs) TestInternalReadOnly(t *testing.T) {
@@ -95,8 +86,7 @@ func TestMoveCopy(t *testing.T) {
 		t.Skip("Skipping as -remote set")
 	}
 	ctx := context.Background()
-	dirs, clean := MakeTestDirs(t, 1)
-	defer clean()
+	dirs := MakeTestDirs(t, 1)
 	fsString := fmt.Sprintf(":union,upstreams='%s :memory:bucket':", dirs[0])
 	f, err := fs.NewFs(ctx, fsString)
 	require.NoError(t, err)
diff --git a/backend/union/union_test.go b/backend/union/union_test.go
index c58936e9d..8b69c4c27 100644
--- a/backend/union/union_test.go
+++ b/backend/union/union_test.go
@@ -27,8 +27,7 @@ func TestStandard(t *testing.T) {
 	if *fstest.RemoteName != "" {
 		t.Skip("Skipping as -remote set")
 	}
-	dirs, clean := union.MakeTestDirs(t, 3)
-	defer clean()
+	dirs := union.MakeTestDirs(t, 3)
 	upstreams := dirs[0] + " " + dirs[1] + " " + dirs[2]
 	name := "TestUnion"
 	fstests.Run(t, &fstests.Opt{
@@ -49,8 +48,7 @@ func TestRO(t *testing.T) {
 	if *fstest.RemoteName != "" {
 		t.Skip("Skipping as -remote set")
 	}
-	dirs, clean := union.MakeTestDirs(t, 3)
-	defer clean()
+	dirs := union.MakeTestDirs(t, 3)
 	upstreams := dirs[0] + " " + dirs[1] + ":ro " + dirs[2] + ":ro"
 	name := "TestUnionRO"
 	fstests.Run(t, &fstests.Opt{
@@ -71,8 +69,7 @@ func TestNC(t *testing.T) {
 	if *fstest.RemoteName != "" {
 		t.Skip("Skipping as -remote set")
 	}
-	dirs, clean := union.MakeTestDirs(t, 3)
-	defer clean()
+	dirs := union.MakeTestDirs(t, 3)
 	upstreams := dirs[0] + " " + dirs[1] + ":nc " + dirs[2] + ":nc"
 	name := "TestUnionNC"
 	fstests.Run(t, &fstests.Opt{
@@ -93,8 +90,7 @@ func TestPolicy1(t *testing.T) {
 	if *fstest.RemoteName != "" {
 		t.Skip("Skipping as -remote set")
 	}
-	dirs, clean := union.MakeTestDirs(t, 3)
-	defer clean()
+	dirs := union.MakeTestDirs(t, 3)
 	upstreams := dirs[0] + " " + dirs[1] + " " + dirs[2]
 	name := "TestUnionPolicy1"
 	fstests.Run(t, &fstests.Opt{
@@ -115,8 +111,7 @@ func TestPolicy2(t *testing.T) {
 	if *fstest.RemoteName != "" {
 		t.Skip("Skipping as -remote set")
 	}
-	dirs, clean := union.MakeTestDirs(t, 3)
-	defer clean()
+	dirs := union.MakeTestDirs(t, 3)
 	upstreams := dirs[0] + " " + dirs[1] + " " + dirs[2]
 	name := "TestUnionPolicy2"
 	fstests.Run(t, &fstests.Opt{
@@ -137,8 +132,7 @@ func TestPolicy3(t *testing.T) {
 	if *fstest.RemoteName != "" {
 		t.Skip("Skipping as -remote set")
 	}
-	dirs, clean := union.MakeTestDirs(t, 3)
-	defer clean()
+	dirs := union.MakeTestDirs(t, 3)
 	upstreams := dirs[0] + " " + dirs[1] + " " + dirs[2]
 	name := "TestUnionPolicy3"
 	fstests.Run(t, &fstests.Opt{
diff --git a/cmd/mountlib/rc_test.go b/cmd/mountlib/rc_test.go
index 030a89a5a..5513ce4a7 100644
--- a/cmd/mountlib/rc_test.go
+++ b/cmd/mountlib/rc_test.go
@@ -35,19 +35,14 @@ func TestRc(t *testing.T) {
 	getMountTypes := rc.Calls.Get("mount/types")
 	assert.NotNil(t, getMountTypes)
 
-	localDir, err := ioutil.TempDir("", "rclone-mountlib-localDir")
-	require.NoError(t, err)
-	defer func() { _ = os.RemoveAll(localDir) }()
-	err = ioutil.WriteFile(filepath.Join(localDir, "file.txt"), []byte("hello"), 0666)
+	localDir := t.TempDir()
+	err := ioutil.WriteFile(filepath.Join(localDir, "file.txt"), []byte("hello"), 0666)
 	require.NoError(t, err)
 
-	mountPoint, err := ioutil.TempDir("", "rclone-mountlib-mountPoint")
-	require.NoError(t, err)
+	mountPoint := t.TempDir()
 	if runtime.GOOS == "windows" {
 		// Windows requires the mount point not to exist
 		require.NoError(t, os.RemoveAll(mountPoint))
-	} else {
-		defer func() { _ = os.RemoveAll(mountPoint) }()
 	}
 
 	out, err := getMountTypes.Fn(ctx, nil)
diff --git a/cmd/serve/restic/restic_appendonly_test.go b/cmd/serve/restic/restic_appendonly_test.go
index f2090b2e4..c190da329 100644
--- a/cmd/serve/restic/restic_appendonly_test.go
+++ b/cmd/serve/restic/restic_appendonly_test.go
@@ -7,9 +7,7 @@ import (
 	"crypto/rand"
 	"encoding/hex"
 	"io"
-	"io/ioutil"
 	"net/http"
-	"os"
 	"strings"
 	"testing"
 
@@ -113,14 +111,7 @@ func TestResticHandler(t *testing.T) {
 	}
 
 	// setup rclone with a local backend in a temporary directory
-	tempdir, err := ioutil.TempDir("", "rclone-restic-test-")
-	require.NoError(t, err)
-
-	// make sure the tempdir is properly removed
-	defer func() {
-		err := os.RemoveAll(tempdir)
-		require.NoError(t, err)
-	}()
+	tempdir := t.TempDir()
 
 	// globally set append-only mode
 	prev := appendOnly
diff --git a/cmd/serve/restic/restic_privaterepos_test.go b/cmd/serve/restic/restic_privaterepos_test.go
index 7fd2a5c6b..fd59e181d 100644
--- a/cmd/serve/restic/restic_privaterepos_test.go
+++ b/cmd/serve/restic/restic_privaterepos_test.go
@@ -7,9 +7,7 @@ import (
 	"context"
 	"crypto/rand"
 	"io"
-	"io/ioutil"
 	"net/http"
-	"os"
 	"strings"
 	"testing"
 
@@ -35,14 +33,7 @@ func TestResticPrivateRepositories(t *testing.T) {
 	require.NoError(t, err)
 
 	// setup rclone with a local backend in a temporary directory
-	tempdir, err := ioutil.TempDir("", "rclone-restic-test-")
-	require.NoError(t, err)
-
-	// make sure the tempdir is properly removed
-	defer func() {
-		err := os.RemoveAll(tempdir)
-		require.NoError(t, err)
-	}()
+	tempdir := t.TempDir()
 
 	// globally set private-repos mode & test user
 	prev := privateRepos
diff --git a/cmdtest/cmdtest_test.go b/cmdtest/cmdtest_test.go
index f8c6adbbb..1d0c8487e 100644
--- a/cmdtest/cmdtest_test.go
+++ b/cmdtest/cmdtest_test.go
@@ -102,8 +102,7 @@ var envInitial []string
 // sets testConfig to testFolder/rclone.config.
 func createTestEnvironment(t *testing.T) {
 	//Set temporary folder for config and test data
-	tempFolder, err := ioutil.TempDir("", "rclone_cmdtest_")
-	require.NoError(t, err)
+	tempFolder := t.TempDir()
 	testFolder = filepath.ToSlash(tempFolder)
 
 	// Set path to temporary config file
diff --git a/fs/rc/webgui/rc_test.go b/fs/rc/webgui/rc_test.go
index 72de1b34b..4ba6b5cdb 100644
--- a/fs/rc/webgui/rc_test.go
+++ b/fs/rc/webgui/rc_test.go
@@ -2,7 +2,6 @@ package webgui
 
 import (
 	"context"
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"strings"
@@ -24,13 +23,12 @@ func init() {
 }
 
 func setCacheDir(t *testing.T) string {
-	cacheDir, err := ioutil.TempDir("", "rclone-cache-dir")
-	assert.Nil(t, err)
+	cacheDir := t.TempDir()
 	PluginsPath = filepath.Join(cacheDir, "plugins")
 	pluginsConfigPath = filepath.Join(cacheDir, "config")
 
 	loadedPlugins = newPlugins(availablePluginsJSONPath)
-	err = loadedPlugins.readFromFile()
+	err := loadedPlugins.readFromFile()
 	assert.Nil(t, err)
 	return cacheDir
 }
diff --git a/lib/file/file_test.go b/lib/file/file_test.go
index 4aef72650..933fb4ca7 100644
--- a/lib/file/file_test.go
+++ b/lib/file/file_test.go
@@ -13,15 +13,6 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-// Create a test directory then tidy up
-func testDir(t *testing.T) (string, func()) {
-	dir, err := ioutil.TempDir("", "rclone-test")
-	require.NoError(t, err)
-	return dir, func() {
-		assert.NoError(t, os.RemoveAll(dir))
-	}
-}
-
 // This lists dir and checks the listing is as expected without checking the size
 func checkListingNoSize(t *testing.T, dir string, want []string) {
 	var got []string
@@ -46,8 +37,7 @@ func checkListing(t *testing.T, dir string, want []string) {
 
 // Test we can rename an open file
 func TestOpenFileRename(t *testing.T) {
-	dir, tidy := testDir(t)
-	defer tidy()
+	dir := t.TempDir()
 
 	filepath := path.Join(dir, "file1")
 	f, err := Create(filepath)
@@ -71,8 +61,7 @@ func TestOpenFileRename(t *testing.T) {
 
 // Test we can delete an open file
 func TestOpenFileDelete(t *testing.T) {
-	dir, tidy := testDir(t)
-	defer tidy()
+	dir := t.TempDir()
 
 	filepath := path.Join(dir, "file1")
 	f, err := Create(filepath)
@@ -103,8 +92,7 @@ func TestOpenFileDelete(t *testing.T) {
 
 // Smoke test the Open, OpenFile and Create functions
 func TestOpenFileOperations(t *testing.T) {
-	dir, tidy := testDir(t)
-	defer tidy()
+	dir := t.TempDir()
 
 	filepath := path.Join(dir, "file1")
 
diff --git a/lib/file/mkdir_windows_test.go b/lib/file/mkdir_windows_test.go
index 9342f3c2e..e6daf1d49 100644
--- a/lib/file/mkdir_windows_test.go
+++ b/lib/file/mkdir_windows_test.go
@@ -15,8 +15,7 @@ import (
 
 // Basic test from golang's os/path_test.go
 func TestMkdirAll(t *testing.T) {
-	tmpDir, tidy := testDir(t)
-	defer tidy()
+	tmpDir := t.TempDir()
 
 	path := tmpDir + "/dir/./dir2"
 	err := MkdirAll(path, 0777)
@@ -99,8 +98,7 @@ func checkMkdirAllSubdirs(t *testing.T, path string, valid bool, errormsg string
 
 // Testing paths on existing drive
 func TestMkdirAllOnDrive(t *testing.T) {
-	path, tidy := testDir(t)
-	defer tidy()
+	path := t.TempDir()
 
 	dir, err := os.Stat(path)
 	require.NoError(t, err)