From bc27aa12cd9ffd7b64e0144cd3e7423f1dbf8ea8 Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Fri, 15 Apr 2022 11:44:06 +0800 Subject: [PATCH] all: use T.TempDir to create temporary test directory (#8280) This commit replaces `os.MkdirTemp` with `t.TempDir` in tests. The directory created by `t.TempDir` is automatically removed when the test and all its subtests complete. Prior to this commit, temporary directory created using `os.MkdirTemp` needs to be removed manually by calling `os.RemoveAll`, which is omitted in some tests. The error handling boilerplate e.g. defer func() { if err := os.RemoveAll(dir); err != nil { t.Fatal(err) } } is also tedious, but `t.TempDir` handles this for us nicely. Reference: https://pkg.go.dev/testing#T.TempDir Signed-off-by: Eng Zer Jun --- cmd/syncthing/monitor_test.go | 15 ++++++---- lib/api/api_test.go | 6 +--- lib/config/config_test.go | 13 ++------ lib/fs/basicfs_test.go | 24 +++------------ lib/fs/basicfs_windows_test.go | 2 -- lib/fs/casefs_test.go | 10 ++----- lib/fs/fakefs_test.go | 15 +--------- lib/fs/mtimefs_test.go | 14 +++------ lib/ignore/ignore_test.go | 30 ++++--------------- lib/model/folder_sendrecv_test.go | 7 +++-- lib/model/model_test.go | 46 ++++++++++++++--------------- lib/model/requests_test.go | 16 ++++------ lib/model/sharedpullerstate_test.go | 3 +- lib/model/testutils_test.go | 19 ++---------- lib/model/utils_test.go | 13 +++----- lib/osutil/atomic_test.go | 6 +--- lib/osutil/osutil_test.go | 20 +++++-------- lib/osutil/traversessymlink_test.go | 16 +++------- lib/scanner/walk_test.go | 18 ++--------- lib/versioner/simple_test.go | 7 +---- lib/versioner/staggered_test.go | 5 +--- lib/versioner/trashcan_test.go | 11 ++----- 22 files changed, 91 insertions(+), 225 deletions(-) diff --git a/cmd/syncthing/monitor_test.go b/cmd/syncthing/monitor_test.go index 6ad8e4ee2..d0bc3b033 100644 --- a/cmd/syncthing/monitor_test.go +++ b/cmd/syncthing/monitor_test.go @@ -17,14 +17,17 @@ import ( func TestRotatedFile(t *testing.T) { // Verify that log rotation happens. - dir, err := os.MkdirTemp("", "syncthing") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(dir) + dir := t.TempDir() open := func(name string) (io.WriteCloser, error) { - return os.Create(name) + f, err := os.Create(name) + t.Cleanup(func() { + if f != nil { + _ = f.Close() + } + }) + + return f, err } logName := filepath.Join(dir, "log.txt") diff --git a/lib/api/api_test.go b/lib/api/api_test.go index 7b506259c..e149824b0 100644 --- a/lib/api/api_test.go +++ b/lib/api/api_test.go @@ -1146,11 +1146,7 @@ func TestBrowse(t *testing.T) { pathSep := string(os.PathSeparator) - tmpDir, err := os.MkdirTemp("", "syncthing") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(tmpDir) + tmpDir := t.TempDir() if err := os.Mkdir(filepath.Join(tmpDir, "dir"), 0755); err != nil { t.Fatal(err) diff --git a/lib/config/config_test.go b/lib/config/config_test.go index 628099f51..36f823dc9 100644 --- a/lib/config/config_test.go +++ b/lib/config/config_test.go @@ -504,13 +504,10 @@ func TestFolderPath(t *testing.T) { } func TestFolderCheckPath(t *testing.T) { - n, err := os.MkdirTemp("", "") - if err != nil { - t.Fatal(err) - } + n := t.TempDir() testFs := fs.NewFilesystem(fs.FilesystemTypeBasic, n) - err = os.MkdirAll(filepath.Join(n, "dir", ".stfolder"), os.FileMode(0777)) + err := os.MkdirAll(filepath.Join(n, "dir", ".stfolder"), os.FileMode(0777)) if err != nil { t.Fatal(err) } @@ -602,11 +599,7 @@ func TestWindowsLineEndings(t *testing.T) { t.Skip("Windows specific") } - dir, err := os.MkdirTemp("", "syncthing-test") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(dir) + dir := t.TempDir() path := filepath.Join(dir, "config.xml") os.Remove(path) diff --git a/lib/fs/basicfs_test.go b/lib/fs/basicfs_test.go index 78dcece2a..04db3bf4b 100644 --- a/lib/fs/basicfs_test.go +++ b/lib/fs/basicfs_test.go @@ -20,17 +20,13 @@ import ( func setup(t *testing.T) (*BasicFilesystem, string) { t.Helper() - dir, err := os.MkdirTemp("", "") - if err != nil { - t.Fatal(err) - } + dir := t.TempDir() return newBasicFilesystem(dir), dir } func TestChmodFile(t *testing.T) { fs, dir := setup(t) path := filepath.Join(dir, "file") - defer os.RemoveAll(dir) defer os.Chmod(path, 0666) @@ -71,7 +67,6 @@ func TestChownFile(t *testing.T) { fs, dir := setup(t) path := filepath.Join(dir, "file") - defer os.RemoveAll(dir) defer os.Chmod(path, 0666) @@ -108,7 +103,6 @@ func TestChownFile(t *testing.T) { func TestChmodDir(t *testing.T) { fs, dir := setup(t) path := filepath.Join(dir, "dir") - defer os.RemoveAll(dir) mode := os.FileMode(0755) if runtime.GOOS == "windows" { @@ -141,7 +135,6 @@ func TestChmodDir(t *testing.T) { func TestChtimes(t *testing.T) { fs, dir := setup(t) path := filepath.Join(dir, "file") - defer os.RemoveAll(dir) fd, err := os.Create(path) if err != nil { t.Error(err) @@ -166,7 +159,6 @@ func TestChtimes(t *testing.T) { func TestCreate(t *testing.T) { fs, dir := setup(t) path := filepath.Join(dir, "file") - defer os.RemoveAll(dir) if _, err := os.Stat(path); err == nil { t.Errorf("exists?") @@ -190,7 +182,6 @@ func TestCreateSymlink(t *testing.T) { fs, dir := setup(t) path := filepath.Join(dir, "file") - defer os.RemoveAll(dir) if err := fs.CreateSymlink("blah", "file"); err != nil { t.Error(err) @@ -215,7 +206,6 @@ func TestCreateSymlink(t *testing.T) { func TestDirNames(t *testing.T) { fs, dir := setup(t) - defer os.RemoveAll(dir) // Case differences testCases := []string{ @@ -244,8 +234,7 @@ func TestDirNames(t *testing.T) { func TestNames(t *testing.T) { // Tests that all names are without the root directory. - fs, dir := setup(t) - defer os.RemoveAll(dir) + fs, _ := setup(t) expected := "file" fd, err := fs.Create(expected) @@ -284,8 +273,7 @@ func TestNames(t *testing.T) { func TestGlob(t *testing.T) { // Tests that all names are without the root directory. - fs, dir := setup(t) - defer os.RemoveAll(dir) + fs, _ := setup(t) for _, dirToCreate := range []string{ filepath.Join("a", "test", "b"), @@ -344,8 +332,7 @@ func TestGlob(t *testing.T) { } func TestUsage(t *testing.T) { - fs, dir := setup(t) - defer os.RemoveAll(dir) + fs, _ := setup(t) usage, err := fs.Usage(".") if err != nil { if runtime.GOOS == "netbsd" || runtime.GOOS == "openbsd" || runtime.GOOS == "solaris" { @@ -577,18 +564,15 @@ func TestRel(t *testing.T) { func TestBasicWalkSkipSymlink(t *testing.T) { _, dir := setup(t) - defer os.RemoveAll(dir) testWalkSkipSymlink(t, FilesystemTypeBasic, dir) } func TestWalkTraverseDirJunct(t *testing.T) { _, dir := setup(t) - defer os.RemoveAll(dir) testWalkTraverseDirJunct(t, FilesystemTypeBasic, dir) } func TestWalkInfiniteRecursion(t *testing.T) { _, dir := setup(t) - defer os.RemoveAll(dir) testWalkInfiniteRecursion(t, FilesystemTypeBasic, dir) } diff --git a/lib/fs/basicfs_windows_test.go b/lib/fs/basicfs_windows_test.go index ab2f74300..48445d7ba 100644 --- a/lib/fs/basicfs_windows_test.go +++ b/lib/fs/basicfs_windows_test.go @@ -49,7 +49,6 @@ func TestResolveWindows83(t *testing.T) { dir = fs.resolveWin83(dir) fs = newBasicFilesystem(dir) } - defer os.RemoveAll(dir) shortAbs, _ := fs.rooted("LFDATA~1") long := "LFDataTool" @@ -80,7 +79,6 @@ func TestIsWindows83(t *testing.T) { dir = fs.resolveWin83(dir) fs = newBasicFilesystem(dir) } - defer os.RemoveAll(dir) tempTop, _ := fs.rooted(TempName("baz")) tempBelow, _ := fs.rooted(filepath.Join("foo", "bar", TempName("baz"))) diff --git a/lib/fs/casefs_test.go b/lib/fs/casefs_test.go index b819197f0..1e1ab8650 100644 --- a/lib/fs/casefs_test.go +++ b/lib/fs/casefs_test.go @@ -9,7 +9,6 @@ package fs import ( "errors" "fmt" - "os" "path/filepath" "runtime" "sort" @@ -28,8 +27,7 @@ func TestRealCase(t *testing.T) { testRealCase(t, newFakeFilesystem(t.Name()+"?insens=true")) }) t.Run("actual", func(t *testing.T) { - fsys, tmpDir := setup(t) - defer os.RemoveAll(tmpDir) + fsys, _ := setup(t) testRealCase(t, fsys) }) } @@ -83,8 +81,7 @@ func TestRealCaseSensitive(t *testing.T) { testRealCaseSensitive(t, newFakeFilesystem(t.Name())) }) t.Run("actual", func(t *testing.T) { - fsys, tmpDir := setup(t) - defer os.RemoveAll(tmpDir) + fsys, _ := setup(t) testRealCaseSensitive(t, fsys) }) } @@ -124,8 +121,7 @@ func TestCaseFSStat(t *testing.T) { testCaseFSStat(t, newFakeFilesystem(t.Name()+"?insens=true")) }) t.Run("actual", func(t *testing.T) { - fsys, tmpDir := setup(t) - defer os.RemoveAll(tmpDir) + fsys, _ := setup(t) testCaseFSStat(t, fsys) }) } diff --git a/lib/fs/fakefs_test.go b/lib/fs/fakefs_test.go index 63cae9d40..3ace32470 100644 --- a/lib/fs/fakefs_test.go +++ b/lib/fs/fakefs_test.go @@ -204,7 +204,6 @@ func TestFakeFSCaseSensitive(t *testing.T) { } testDir, sensitive := createTestDir(t) - defer removeTestDir(t, testDir) if sensitive { filesystems = append(filesystems, testFS{runtime.GOOS, newBasicFilesystem(testDir)}) } @@ -240,7 +239,6 @@ func TestFakeFSCaseInsensitive(t *testing.T) { } testDir, sensitive := createTestDir(t) - defer removeTestDir(t, testDir) if !sensitive { filesystems = append(filesystems, testFS{runtime.GOOS, newBasicFilesystem(testDir)}) } @@ -251,10 +249,7 @@ func TestFakeFSCaseInsensitive(t *testing.T) { func createTestDir(t *testing.T) (string, bool) { t.Helper() - testDir, err := os.MkdirTemp("", "") - if err != nil { - t.Fatalf("could not create temporary dir for testing: %s", err) - } + testDir := t.TempDir() if fd, err := os.Create(filepath.Join(testDir, ".stfolder")); err != nil { t.Fatalf("could not create .stfolder: %s", err) @@ -273,14 +268,6 @@ func createTestDir(t *testing.T) (string, bool) { return testDir, sensitive } -func removeTestDir(t *testing.T, testDir string) { - t.Helper() - - if err := os.RemoveAll(testDir); err != nil { - t.Fatalf("could not remove test directory: %s", err) - } -} - func runTests(t *testing.T, tests []test, filesystems []testFS) { for _, filesystem := range filesystems { for _, test := range tests { diff --git a/lib/fs/mtimefs_test.go b/lib/fs/mtimefs_test.go index 6c294bd24..5b547b68c 100644 --- a/lib/fs/mtimefs_test.go +++ b/lib/fs/mtimefs_test.go @@ -82,11 +82,7 @@ func TestMtimeFS(t *testing.T) { } func TestMtimeFSWalk(t *testing.T) { - dir, err := os.MkdirTemp("", "") - if err != nil { - t.Fatal(err) - } - defer func() { _ = os.RemoveAll(dir) }() + dir := t.TempDir() mtimefs, walkFs := newMtimeFSWithWalk(dir, make(mapStore)) underlying := mtimefs.Filesystem @@ -136,11 +132,7 @@ func TestMtimeFSWalk(t *testing.T) { } func TestMtimeFSOpen(t *testing.T) { - dir, err := os.MkdirTemp("", "") - if err != nil { - t.Fatal(err) - } - defer func() { _ = os.RemoveAll(dir) }() + dir := t.TempDir() mtimefs := newMtimeFS(dir, make(mapStore)) underlying := mtimefs.Filesystem @@ -177,6 +169,8 @@ func TestMtimeFSOpen(t *testing.T) { if err != nil { t.Fatal(err) } + defer fd.Close() + info, err := fd.Stat() if err != nil { t.Fatal(err) diff --git a/lib/ignore/ignore_test.go b/lib/ignore/ignore_test.go index b1f19c5a9..0295d6cca 100644 --- a/lib/ignore/ignore_test.go +++ b/lib/ignore/ignore_test.go @@ -231,10 +231,7 @@ func TestCaseSensitivity(t *testing.T) { } func TestCaching(t *testing.T) { - dir, err := os.MkdirTemp("", "") - if err != nil { - t.Fatal(err) - } + dir := t.TempDir() fs := fs.NewFilesystem(fs.FilesystemTypeBasic, dir) @@ -424,10 +421,7 @@ flamingo *.crow ` // Caches per file, hence write the patterns to a file. - dir, err := os.MkdirTemp("", "") - if err != nil { - b.Fatal(err) - } + dir := b.TempDir() fs := fs.NewFilesystem(fs.FilesystemTypeBasic, dir) @@ -465,10 +459,7 @@ flamingo } func TestCacheReload(t *testing.T) { - dir, err := os.MkdirTemp("", "") - if err != nil { - t.Fatal(err) - } + dir := t.TempDir() fs := fs.NewFilesystem(fs.FilesystemTypeBasic, dir) @@ -988,12 +979,7 @@ func TestIssue4689(t *testing.T) { } func TestIssue4901(t *testing.T) { - dir, err := os.MkdirTemp("", "") - if err != nil { - t.Fatal(err) - } - - defer os.RemoveAll(dir) + dir := t.TempDir() stignore := ` #include unicorn-lazor-death @@ -1023,7 +1009,7 @@ func TestIssue4901(t *testing.T) { t.Fatalf(err.Error()) } - err = pats.Load(".stignore") + err := pats.Load(".stignore") if err != nil { t.Fatalf("unexpected error: %s", err.Error()) } @@ -1200,11 +1186,7 @@ func TestWindowsLineEndings(t *testing.T) { lines := "foo\nbar\nbaz\n" - dir, err := os.MkdirTemp("", "syncthing-test") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(dir) + dir := t.TempDir() ffs := fs.NewFilesystem(fs.FilesystemTypeBasic, dir) m := New(ffs) diff --git a/lib/model/folder_sendrecv_test.go b/lib/model/folder_sendrecv_test.go index 0e87bccaf..4793f74dd 100644 --- a/lib/model/folder_sendrecv_test.go +++ b/lib/model/folder_sendrecv_test.go @@ -89,7 +89,7 @@ func createEmptyFileInfo(t *testing.T, name string, fs fs.Filesystem) protocol.F // Sets up a folder and model, but makes sure the services aren't actually running. func setupSendReceiveFolder(t testing.TB, files ...protocol.FileInfo) (*testModel, *sendReceiveFolder, context.CancelFunc) { - w, fcfg, wCancel := tmpDefaultWrapper() + w, fcfg, wCancel := tmpDefaultWrapper(t) // Initialise model and stop immediately. model := setupModel(t, w) model.cancel() @@ -972,8 +972,7 @@ func TestDeleteBehindSymlink(t *testing.T) { defer cleanupSRFolder(f, m, wcfgCancel) ffs := f.Filesystem(nil) - destDir := createTmpDir() - defer os.RemoveAll(destDir) + destDir := t.TempDir() destFs := fs.NewFilesystem(fs.FilesystemTypeBasic, destDir) link := "link" @@ -1228,6 +1227,8 @@ func TestPullTempFileCaseConflict(t *testing.T) { cs := <-copyChan if _, err := cs.tempFile(); err != nil { t.Error(err) + } else { + cs.finalClose() } } diff --git a/lib/model/model_test.go b/lib/model/model_test.go index ac1df08a9..6d25a3f19 100644 --- a/lib/model/model_test.go +++ b/lib/model/model_test.go @@ -2475,7 +2475,7 @@ func TestIssue2571(t *testing.T) { t.Skip("Scanning symlinks isn't supported on windows") } - w, fcfg, wCancel := tmpDefaultWrapper() + w, fcfg, wCancel := tmpDefaultWrapper(t) defer wCancel() testFs := fcfg.Filesystem(nil) defer os.RemoveAll(testFs.URI()) @@ -2514,7 +2514,7 @@ func TestIssue4573(t *testing.T) { t.Skip("Can't make the dir inaccessible on windows") } - w, fcfg, wCancel := tmpDefaultWrapper() + w, fcfg, wCancel := tmpDefaultWrapper(t) defer wCancel() testFs := fcfg.Filesystem(nil) defer os.RemoveAll(testFs.URI()) @@ -2544,7 +2544,7 @@ func TestIssue4573(t *testing.T) { // TestInternalScan checks whether various fs operations are correctly represented // in the db after scanning. func TestInternalScan(t *testing.T) { - w, fcfg, wCancel := tmpDefaultWrapper() + w, fcfg, wCancel := tmpDefaultWrapper(t) defer wCancel() testFs := fcfg.Filesystem(nil) defer os.RemoveAll(testFs.URI()) @@ -2605,7 +2605,7 @@ func TestInternalScan(t *testing.T) { func TestCustomMarkerName(t *testing.T) { testOs := &fatalOs{t} - fcfg := testFolderConfigTmp() + fcfg := testFolderConfig(t.TempDir()) fcfg.ID = "default" fcfg.RescanIntervalS = 1 fcfg.MarkerName = "myfile" @@ -2762,9 +2762,7 @@ func TestVersionRestore(t *testing.T) { // In each file, we write the filename as the content // We verify that the content matches at the expected filenames // after the restore operation. - dir, err := os.MkdirTemp("", "") - must(t, err) - defer os.RemoveAll(dir) + dir := t.TempDir() fcfg := newFolderConfiguration(defaultCfgWrapper, "default", "default", fs.FilesystemTypeBasic, dir) fcfg.Versioning.Type = "simple" @@ -3181,7 +3179,7 @@ func TestConnCloseOnRestart(t *testing.T) { protocol.CloseTimeout = oldCloseTimeout }() - w, fcfg, wCancel := tmpDefaultWrapper() + w, fcfg, wCancel := tmpDefaultWrapper(t) defer wCancel() m := setupModel(t, w) defer cleanupModelAndRemoveDir(m, fcfg.Filesystem(nil).URI()) @@ -3218,7 +3216,7 @@ func TestConnCloseOnRestart(t *testing.T) { } func TestModTimeWindow(t *testing.T) { - w, fcfg, wCancel := tmpDefaultWrapper() + w, fcfg, wCancel := tmpDefaultWrapper(t) defer wCancel() tfs := fcfg.Filesystem(nil) fcfg.RawModTimeWindowS = 2 @@ -3347,7 +3345,7 @@ func TestNewLimitedRequestResponse(t *testing.T) { } func TestSummaryPausedNoError(t *testing.T) { - wcfg, fcfg, wcfgCancel := tmpDefaultWrapper() + wcfg, fcfg, wcfgCancel := tmpDefaultWrapper(t) defer wcfgCancel() pauseFolder(t, wcfg, fcfg.ID, true) m := setupModel(t, wcfg) @@ -3360,7 +3358,7 @@ func TestSummaryPausedNoError(t *testing.T) { } func TestFolderAPIErrors(t *testing.T) { - wcfg, fcfg, wcfgCancel := tmpDefaultWrapper() + wcfg, fcfg, wcfgCancel := tmpDefaultWrapper(t) defer wcfgCancel() pauseFolder(t, wcfg, fcfg.ID, true) m := setupModel(t, wcfg) @@ -3392,7 +3390,7 @@ func TestFolderAPIErrors(t *testing.T) { } func TestRenameSequenceOrder(t *testing.T) { - wcfg, fcfg, wcfgCancel := tmpDefaultWrapper() + wcfg, fcfg, wcfgCancel := tmpDefaultWrapper(t) defer wcfgCancel() m := setupModel(t, wcfg) defer cleanupModel(m) @@ -3463,7 +3461,7 @@ func TestRenameSequenceOrder(t *testing.T) { } func TestRenameSameFile(t *testing.T) { - wcfg, fcfg, wcfgCancel := tmpDefaultWrapper() + wcfg, fcfg, wcfgCancel := tmpDefaultWrapper(t) defer wcfgCancel() m := setupModel(t, wcfg) defer cleanupModel(m) @@ -3514,7 +3512,7 @@ func TestRenameSameFile(t *testing.T) { } func TestRenameEmptyFile(t *testing.T) { - wcfg, fcfg, wcfgCancel := tmpDefaultWrapper() + wcfg, fcfg, wcfgCancel := tmpDefaultWrapper(t) defer wcfgCancel() m := setupModel(t, wcfg) defer cleanupModel(m) @@ -3591,7 +3589,7 @@ func TestRenameEmptyFile(t *testing.T) { } func TestBlockListMap(t *testing.T) { - wcfg, fcfg, wcfgCancel := tmpDefaultWrapper() + wcfg, fcfg, wcfgCancel := tmpDefaultWrapper(t) defer wcfgCancel() m := setupModel(t, wcfg) defer cleanupModel(m) @@ -3659,7 +3657,7 @@ func TestBlockListMap(t *testing.T) { } func TestScanRenameCaseOnly(t *testing.T) { - wcfg, fcfg, wcfgCancel := tmpDefaultWrapper() + wcfg, fcfg, wcfgCancel := tmpDefaultWrapper(t) defer wcfgCancel() m := setupModel(t, wcfg) defer cleanupModel(m) @@ -3712,7 +3710,7 @@ func TestScanRenameCaseOnly(t *testing.T) { func TestClusterConfigOnFolderAdd(t *testing.T) { testConfigChangeTriggersClusterConfigs(t, false, true, nil, func(wrapper config.Wrapper) { - fcfg := testFolderConfigTmp() + fcfg := testFolderConfig(t.TempDir()) fcfg.ID = "second" fcfg.Label = "second" fcfg.Devices = []config.FolderDeviceConfiguration{{ @@ -3822,7 +3820,7 @@ func TestScanDeletedROChangedOnSR(t *testing.T) { func testConfigChangeTriggersClusterConfigs(t *testing.T, expectFirst, expectSecond bool, pre func(config.Wrapper), fn func(config.Wrapper)) { t.Helper() - wcfg, _, wcfgCancel := tmpDefaultWrapper() + wcfg, _, wcfgCancel := tmpDefaultWrapper(t) defer wcfgCancel() m := setupModel(t, wcfg) defer cleanupModel(m) @@ -3884,7 +3882,7 @@ func testConfigChangeTriggersClusterConfigs(t *testing.T, expectFirst, expectSec // That then causes these files to be considered as needed, while they are not. // https://github.com/syncthing/syncthing/issues/6961 func TestIssue6961(t *testing.T) { - wcfg, fcfg, wcfgCancel := tmpDefaultWrapper() + wcfg, fcfg, wcfgCancel := tmpDefaultWrapper(t) defer wcfgCancel() tfs := fcfg.Filesystem(nil) waiter, err := wcfg.Modify(func(cfg *config.Configuration) { @@ -3971,7 +3969,7 @@ func TestCompletionEmptyGlobal(t *testing.T) { } func TestNeedMetaAfterIndexReset(t *testing.T) { - w, fcfg, wCancel := tmpDefaultWrapper() + w, fcfg, wCancel := tmpDefaultWrapper(t) defer wCancel() addDevice2(t, w, fcfg) m := setupModel(t, w) @@ -4014,7 +4012,7 @@ func TestCcCheckEncryption(t *testing.T) { t.Skip("skipping on short testing - generating encryption tokens is slow") } - w, fcfg, wCancel := tmpDefaultWrapper() + w, fcfg, wCancel := tmpDefaultWrapper(t) defer wCancel() m := setupModel(t, w) m.cancel() @@ -4155,7 +4153,7 @@ func TestCcCheckEncryption(t *testing.T) { func TestCCFolderNotRunning(t *testing.T) { // Create the folder, but don't start it. - w, fcfg, wCancel := tmpDefaultWrapper() + w, fcfg, wCancel := tmpDefaultWrapper(t) defer wCancel() tfs := fcfg.Filesystem(nil) m := newModel(t, w, myID, "syncthing", "dev", nil) @@ -4183,7 +4181,7 @@ func TestCCFolderNotRunning(t *testing.T) { } func TestPendingFolder(t *testing.T) { - w, _, wCancel := tmpDefaultWrapper() + w, _, wCancel := tmpDefaultWrapper(t) defer wCancel() m := setupModel(t, w) defer cleanupModel(m) @@ -4263,7 +4261,7 @@ func TestDeletedNotLocallyChangedReceiveEncrypted(t *testing.T) { } func deletedNotLocallyChanged(t *testing.T, ft config.FolderType) { - w, fcfg, wCancel := tmpDefaultWrapper() + w, fcfg, wCancel := tmpDefaultWrapper(t) tfs := fcfg.Filesystem(nil) fcfg.Type = ft setFolder(t, w, fcfg) diff --git a/lib/model/requests_test.go b/lib/model/requests_test.go index 2721901a4..a50953f18 100644 --- a/lib/model/requests_test.go +++ b/lib/model/requests_test.go @@ -221,7 +221,7 @@ func TestRequestVersioningSymlinkAttack(t *testing.T) { // Sets up a folder with trashcan versioning and tries to use a // deleted symlink to escape - w, fcfg, wCancel := tmpDefaultWrapper() + w, fcfg, wCancel := tmpDefaultWrapper(t) defer wCancel() defer func() { os.RemoveAll(fcfg.Filesystem(nil).URI()) @@ -235,11 +235,7 @@ func TestRequestVersioningSymlinkAttack(t *testing.T) { // Create a temporary directory that we will use as target to see if // we can escape to it - tmpdir, err := os.MkdirTemp("", "syncthing-test") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(tmpdir) + tmpdir := t.TempDir() // We listen for incoming index updates and trigger when we see one for // the expected test file. @@ -300,7 +296,7 @@ func TestPullInvalidIgnoredSR(t *testing.T) { func pullInvalidIgnored(t *testing.T, ft config.FolderType) { w, wCancel := createTmpWrapper(defaultCfgWrapper.RawCopy()) defer wCancel() - fcfg := testFolderConfigTmp() + fcfg := testFolderConfig(t.TempDir()) fss := fcfg.Filesystem(nil) fcfg.Type = ft setFolder(t, w, fcfg) @@ -1029,7 +1025,7 @@ func TestNeedFolderFiles(t *testing.T) { // propagated upon un-ignoring. // https://github.com/syncthing/syncthing/issues/6038 func TestIgnoreDeleteUnignore(t *testing.T) { - w, fcfg, wCancel := tmpDefaultWrapper() + w, fcfg, wCancel := tmpDefaultWrapper(t) defer wCancel() m := setupModel(t, w) fss := fcfg.Filesystem(nil) @@ -1273,7 +1269,7 @@ func TestRequestIndexSenderPause(t *testing.T) { } func TestRequestIndexSenderClusterConfigBeforeStart(t *testing.T) { - w, fcfg, wCancel := tmpDefaultWrapper() + w, fcfg, wCancel := tmpDefaultWrapper(t) defer wCancel() tfs := fcfg.Filesystem(nil) dir1 := "foo" @@ -1340,7 +1336,7 @@ func TestRequestReceiveEncrypted(t *testing.T) { t.Skip("skipping on short testing - scrypt is too slow") } - w, fcfg, wCancel := tmpDefaultWrapper() + w, fcfg, wCancel := tmpDefaultWrapper(t) defer wCancel() tfs := fcfg.Filesystem(nil) fcfg.Type = config.FolderTypeReceiveEncrypted diff --git a/lib/model/sharedpullerstate_test.go b/lib/model/sharedpullerstate_test.go index 2271032bb..7b08ac2b5 100644 --- a/lib/model/sharedpullerstate_test.go +++ b/lib/model/sharedpullerstate_test.go @@ -17,8 +17,7 @@ import ( // Test creating temporary file inside read-only directory func TestReadOnlyDir(t *testing.T) { // Create a read only directory, clean it up afterwards. - tmpDir := createTmpDir() - defer os.RemoveAll(tmpDir) + tmpDir := t.TempDir() if err := os.Chmod(tmpDir, 0555); err != nil { t.Fatal(err) } diff --git a/lib/model/testutils_test.go b/lib/model/testutils_test.go index 954d64b00..aceb44062 100644 --- a/lib/model/testutils_test.go +++ b/lib/model/testutils_test.go @@ -86,20 +86,15 @@ func createTmpWrapper(cfg config.Configuration) (config.Wrapper, context.CancelF return wrapper, cancel } -func tmpDefaultWrapper() (config.Wrapper, config.FolderConfiguration, context.CancelFunc) { +func tmpDefaultWrapper(t testing.TB) (config.Wrapper, config.FolderConfiguration, context.CancelFunc) { w, cancel := createTmpWrapper(defaultCfgWrapper.RawCopy()) - fcfg := testFolderConfigTmp() + fcfg := testFolderConfig(t.TempDir()) _, _ = w.Modify(func(cfg *config.Configuration) { cfg.SetFolder(fcfg) }) return w, fcfg, cancel } -func testFolderConfigTmp() config.FolderConfiguration { - tmpDir := createTmpDir() - return testFolderConfig(tmpDir) -} - func testFolderConfig(path string) config.FolderConfiguration { cfg := newFolderConfiguration(defaultCfgWrapper, "default", "default", fs.FilesystemTypeBasic, path) cfg.FSWatcherEnabled = false @@ -116,7 +111,7 @@ func testFolderConfigFake() config.FolderConfiguration { func setupModelWithConnection(t testing.TB) (*testModel, *fakeConnection, config.FolderConfiguration, context.CancelFunc) { t.Helper() - w, fcfg, cancel := tmpDefaultWrapper() + w, fcfg, cancel := tmpDefaultWrapper(t) m, fc := setupModelWithConnectionFromWrapper(t, w) return m, fc, fcfg, cancel } @@ -213,14 +208,6 @@ func cleanupModelAndRemoveDir(m *testModel, dir string) { os.RemoveAll(dir) } -func createTmpDir() string { - tmpDir, err := os.MkdirTemp("", "syncthing_testFolder-") - if err != nil { - panic("Failed to create temporary testing dir") - } - return tmpDir -} - type alwaysChangedKey struct { fs fs.Filesystem name string diff --git a/lib/model/utils_test.go b/lib/model/utils_test.go index b9e628043..0bdd3451f 100644 --- a/lib/model/utils_test.go +++ b/lib/model/utils_test.go @@ -7,7 +7,6 @@ package model import ( - "os" "runtime" "testing" @@ -15,8 +14,7 @@ import ( ) func TestInWriteableDir(t *testing.T) { - dir := createTmpDir() - defer os.RemoveAll(dir) + dir := t.TempDir() fs := fs.NewFilesystem(fs.FilesystemTypeBasic, dir) @@ -77,8 +75,7 @@ func TestOSWindowsRemove(t *testing.T) { return } - dir := createTmpDir() - defer os.RemoveAll(dir) + dir := t.TempDir() fs := fs.NewFilesystem(fs.FilesystemTypeBasic, dir) defer fs.Chmod("testdata/windows/ro/readonlynew", 0700) @@ -115,8 +112,7 @@ func TestOSWindowsRemoveAll(t *testing.T) { return } - dir := createTmpDir() - defer os.RemoveAll(dir) + dir := t.TempDir() fs := fs.NewFilesystem(fs.FilesystemTypeBasic, dir) defer fs.Chmod("testdata/windows/ro/readonlynew", 0700) @@ -148,8 +144,7 @@ func TestInWritableDirWindowsRename(t *testing.T) { return } - dir := createTmpDir() - defer os.RemoveAll(dir) + dir := t.TempDir() fs := fs.NewFilesystem(fs.FilesystemTypeBasic, dir) defer fs.Chmod("testdata/windows/ro/readonlynew", 0700) diff --git a/lib/osutil/atomic_test.go b/lib/osutil/atomic_test.go index c9737784a..c5f56784d 100644 --- a/lib/osutil/atomic_test.go +++ b/lib/osutil/atomic_test.go @@ -61,14 +61,10 @@ func TestCreateAtomicReplaceReadOnly(t *testing.T) { func testCreateAtomicReplace(t *testing.T, oldPerms os.FileMode) { t.Helper() - testdir, err := os.MkdirTemp("", "syncthing") - if err != nil { - t.Fatal(err) - } + testdir := t.TempDir() testfile := filepath.Join(testdir, "testfile") os.RemoveAll(testdir) - defer os.RemoveAll(testdir) if err := os.Mkdir(testdir, 0755); err != nil { t.Fatal(err) diff --git a/lib/osutil/osutil_test.go b/lib/osutil/osutil_test.go index db1e608fd..9aff532e3 100644 --- a/lib/osutil/osutil_test.go +++ b/lib/osutil/osutil_test.go @@ -81,15 +81,7 @@ func TestIsDeleted(t *testing.T) { } func TestRenameOrCopy(t *testing.T) { - mustTempDir := func() string { - t.Helper() - tmpDir, err := os.MkdirTemp("", "") - if err != nil { - t.Fatal(err) - } - return tmpDir - } - sameFs := fs.NewFilesystem(fs.FilesystemTypeBasic, mustTempDir()) + sameFs := fs.NewFilesystem(fs.FilesystemTypeBasic, t.TempDir()) tests := []struct { src fs.Filesystem dst fs.Filesystem @@ -101,13 +93,13 @@ func TestRenameOrCopy(t *testing.T) { file: "file", }, { - src: fs.NewFilesystem(fs.FilesystemTypeBasic, mustTempDir()), - dst: fs.NewFilesystem(fs.FilesystemTypeBasic, mustTempDir()), + src: fs.NewFilesystem(fs.FilesystemTypeBasic, t.TempDir()), + dst: fs.NewFilesystem(fs.FilesystemTypeBasic, t.TempDir()), file: "file", }, { src: fs.NewFilesystem(fs.FilesystemTypeFake, `fake://fake/?files=1&seed=42`), - dst: fs.NewFilesystem(fs.FilesystemTypeBasic, mustTempDir()), + dst: fs.NewFilesystem(fs.FilesystemTypeBasic, t.TempDir()), file: osutil.NativeFilename(`05/7a/4d52f284145b9fe8`), }, } @@ -147,6 +139,10 @@ func TestRenameOrCopy(t *testing.T) { if fd, err := test.dst.Open("new"); err != nil { t.Fatal(err) } else { + t.Cleanup(func() { + _ = fd.Close() + }) + if buf, err := io.ReadAll(fd); err != nil { t.Fatal(err) } else if string(buf) != content { diff --git a/lib/osutil/traversessymlink_test.go b/lib/osutil/traversessymlink_test.go index 374c471fd..b3665c61d 100644 --- a/lib/osutil/traversessymlink_test.go +++ b/lib/osutil/traversessymlink_test.go @@ -17,15 +17,11 @@ import ( ) func TestTraversesSymlink(t *testing.T) { - tmpDir, err := os.MkdirTemp(".", ".test-TraversesSymlink-") - if err != nil { - panic("Failed to create temporary testing dir") - } - defer os.RemoveAll(tmpDir) + tmpDir := t.TempDir() testFs := fs.NewFilesystem(fs.FilesystemTypeBasic, tmpDir) testFs.MkdirAll("a/b/c", 0755) - if err = fs.DebugSymlinkForTestsOnly(testFs, testFs, filepath.Join("a", "b"), filepath.Join("a", "l")); err != nil { + if err := fs.DebugSymlinkForTestsOnly(testFs, testFs, filepath.Join("a", "b"), filepath.Join("a", "l")); err != nil { if runtime.GOOS == "windows" { t.Skip("Symlinks aren't working") } @@ -70,15 +66,11 @@ func TestTraversesSymlink(t *testing.T) { } func TestIssue4875(t *testing.T) { - tmpDir, err := os.MkdirTemp("", ".test-Issue4875-") - if err != nil { - panic("Failed to create temporary testing dir") - } - defer os.RemoveAll(tmpDir) + tmpDir := t.TempDir() testFs := fs.NewFilesystem(fs.FilesystemTypeBasic, tmpDir) testFs.MkdirAll(filepath.Join("a", "b", "c"), 0755) - if err = fs.DebugSymlinkForTestsOnly(testFs, testFs, filepath.Join("a", "b"), filepath.Join("a", "l")); err != nil { + if err := fs.DebugSymlinkForTestsOnly(testFs, testFs, filepath.Join("a", "b"), filepath.Join("a", "l")); err != nil { if runtime.GOOS == "windows" { t.Skip("Symlinks aren't working") } diff --git a/lib/scanner/walk_test.go b/lib/scanner/walk_test.go index c673accf4..00f2456d7 100644 --- a/lib/scanner/walk_test.go +++ b/lib/scanner/walk_test.go @@ -379,11 +379,7 @@ func TestWalkSymlinkWindows(t *testing.T) { func TestWalkRootSymlink(t *testing.T) { // Create a folder with a symlink in it - tmp, err := os.MkdirTemp("", "") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(tmp) + tmp := t.TempDir() testFs := fs.NewFilesystem(testFsType, tmp) link := "link" @@ -712,11 +708,7 @@ func TestStopWalk(t *testing.T) { } func TestIssue4799(t *testing.T) { - tmp, err := os.MkdirTemp("", "") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(tmp) + tmp := t.TempDir() fs := fs.NewFilesystem(testFsType, tmp) @@ -774,11 +766,7 @@ func TestRecurseInclude(t *testing.T) { } func TestIssue4841(t *testing.T) { - tmp, err := os.MkdirTemp("", "") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(tmp) + tmp := t.TempDir() fs := fs.NewFilesystem(testFsType, tmp) diff --git a/lib/versioner/simple_test.go b/lib/versioner/simple_test.go index 0e2f33467..2194e81d2 100644 --- a/lib/versioner/simple_test.go +++ b/lib/versioner/simple_test.go @@ -8,7 +8,6 @@ package versioner import ( "math" - "os" "path/filepath" "testing" "time" @@ -55,11 +54,7 @@ func TestSimpleVersioningVersionCount(t *testing.T) { t.Skip("Test takes some time, skipping.") } - dir, err := os.MkdirTemp("", "") - //defer os.RemoveAll(dir) - if err != nil { - t.Error(err) - } + dir := t.TempDir() cfg := config.FolderConfiguration{ FilesystemType: fs.FilesystemTypeBasic, diff --git a/lib/versioner/staggered_test.go b/lib/versioner/staggered_test.go index 360c29ceb..21122ab5e 100644 --- a/lib/versioner/staggered_test.go +++ b/lib/versioner/staggered_test.go @@ -133,10 +133,7 @@ func TestCreateVersionPath(t *testing.T) { ) // Create a test dir and file - tmpDir, err := os.MkdirTemp("", "") - if err != nil { - t.Fatal(err) - } + tmpDir := t.TempDir() if err := os.WriteFile(filepath.Join(tmpDir, archiveFile), []byte("sup"), 0644); err != nil { t.Fatal(err) } diff --git a/lib/versioner/trashcan_test.go b/lib/versioner/trashcan_test.go index e0c947be9..35bad47ce 100644 --- a/lib/versioner/trashcan_test.go +++ b/lib/versioner/trashcan_test.go @@ -8,7 +8,6 @@ package versioner import ( "io" - "os" "testing" "time" @@ -20,15 +19,9 @@ func TestTrashcanArchiveRestoreSwitcharoo(t *testing.T) { // This tests that trashcan versioner restoration correctly archives existing file, because trashcan versioner // files are untagged, archiving existing file to replace with a restored version technically should collide in // in names. - tmpDir1, err := os.MkdirTemp("", "") - if err != nil { - t.Fatal(err) - } + tmpDir1 := t.TempDir() - tmpDir2, err := os.MkdirTemp("", "") - if err != nil { - t.Fatal(err) - } + tmpDir2 := t.TempDir() cfg := config.FolderConfiguration{ FilesystemType: fs.FilesystemTypeBasic,