From b1564e53e4aea5e55506284ce39fcde0fac11db4 Mon Sep 17 00:00:00 2001 From: Simon Frei Date: Fri, 8 Mar 2019 21:29:09 +0100 Subject: [PATCH] lib/model: Improve test utilities (#5584) --- lib/model/folder_sendrecv_test.go | 62 +++++++++++------------ lib/model/model_test.go | 28 +++++------ lib/model/requests_test.go | 83 ++++++++++++++++--------------- 3 files changed, 87 insertions(+), 86 deletions(-) diff --git a/lib/model/folder_sendrecv_test.go b/lib/model/folder_sendrecv_test.go index 734d85a13..efe65e685 100644 --- a/lib/model/folder_sendrecv_test.go +++ b/lib/model/folder_sendrecv_test.go @@ -94,10 +94,10 @@ func createFile(t *testing.T, name string, fs fs.Filesystem) protocol.FileInfo { return file } -func setupSendReceiveFolder(files ...protocol.FileInfo) (*model, *sendReceiveFolder, string) { +func setupSendReceiveFolder(files ...protocol.FileInfo) (*model, *sendReceiveFolder) { w := createTmpWrapper(defaultCfg) model := newModel(w, myID, "syncthing", "dev", db.OpenMemory(), nil) - fcfg, tmpDir := testFolderConfigTmp() + fcfg := testFolderConfigTmp() model.AddFolder(fcfg) // Update index @@ -123,7 +123,7 @@ func setupSendReceiveFolder(files ...protocol.FileInfo) (*model, *sendReceiveFol // Folders are never actually started, so no initial scan will be done close(f.initialScanFinished) - return model, f, tmpDir + return model, f } // Layout of the files: (indexes from the above array) @@ -141,10 +141,10 @@ func TestHandleFile(t *testing.T) { requiredFile := existingFile requiredFile.Blocks = blocks[1:] - m, f, tmpDir := setupSendReceiveFolder(existingFile) + m, f := setupSendReceiveFolder(existingFile) defer func() { os.Remove(m.cfg.ConfigPath()) - os.Remove(tmpDir) + os.Remove(f.Filesystem().URI()) }() copyChan := make(chan copyBlocksState, 1) @@ -187,10 +187,10 @@ func TestHandleFileWithTemp(t *testing.T) { requiredFile := existingFile requiredFile.Blocks = blocks[1:] - m, f, tmpDir := setupSendReceiveFolder(existingFile) + m, f := setupSendReceiveFolder(existingFile) defer func() { os.Remove(m.cfg.ConfigPath()) - os.Remove(tmpDir) + os.Remove(f.Filesystem().URI()) }() if _, err := prepareTmpFile(f.Filesystem()); err != nil { @@ -240,10 +240,10 @@ func TestCopierFinder(t *testing.T) { requiredFile.Blocks = blocks[1:] requiredFile.Name = "file2" - m, f, tmpDir := setupSendReceiveFolder(existingFile) + m, f := setupSendReceiveFolder(existingFile) defer func() { os.Remove(m.cfg.ConfigPath()) - os.Remove(tmpDir) + os.Remove(f.Filesystem().URI()) }() if _, err := prepareTmpFile(f.Filesystem()); err != nil { @@ -306,12 +306,12 @@ func TestCopierFinder(t *testing.T) { func TestWeakHash(t *testing.T) { // Setup the model/pull environment - model, fo, tmpDir := setupSendReceiveFolder() + model, fo := setupSendReceiveFolder() + ffs := fo.Filesystem() defer func() { os.Remove(model.cfg.ConfigPath()) - os.Remove(tmpDir) + os.Remove(ffs.URI()) }() - ffs := fo.Filesystem() tempFile := fs.TempName("weakhash") var shift int64 = 10 @@ -438,10 +438,10 @@ func TestCopierCleanup(t *testing.T) { // Create a file file := setupFile("test", []int{0}) - m, _, tmpDir := setupSendReceiveFolder(file) + m, f := setupSendReceiveFolder(file) defer func() { os.Remove(m.cfg.ConfigPath()) - os.Remove(tmpDir) + os.Remove(f.Filesystem().URI()) }() file.Blocks = []protocol.BlockInfo{blocks[1]} @@ -474,10 +474,10 @@ func TestCopierCleanup(t *testing.T) { func TestDeregisterOnFailInCopy(t *testing.T) { file := setupFile("filex", []int{0, 2, 0, 0, 5, 0, 0, 8}) - m, f, tmpDir := setupSendReceiveFolder() + m, f := setupSendReceiveFolder() defer func() { os.Remove(m.cfg.ConfigPath()) - os.Remove(tmpDir) + os.Remove(f.Filesystem().URI()) }() // Set up our evet subscription early @@ -564,10 +564,10 @@ func TestDeregisterOnFailInCopy(t *testing.T) { func TestDeregisterOnFailInPull(t *testing.T) { file := setupFile("filex", []int{0, 2, 0, 0, 5, 0, 0, 8}) - m, f, tmpDir := setupSendReceiveFolder() + m, f := setupSendReceiveFolder() defer func() { os.Remove(m.cfg.ConfigPath()) - os.Remove(tmpDir) + os.Remove(f.Filesystem().URI()) }() // Set up our evet subscription early @@ -642,14 +642,14 @@ func TestDeregisterOnFailInPull(t *testing.T) { } func TestIssue3164(t *testing.T) { - m, f, tmpDir := setupSendReceiveFolder() + m, f := setupSendReceiveFolder() + ffs := f.Filesystem() + tmpDir := ffs.URI() defer func() { os.Remove(m.cfg.ConfigPath()) os.Remove(tmpDir) }() - ffs := f.Filesystem() - ignDir := filepath.Join("issue3164", "oktodelete") subDir := filepath.Join(ignDir, "foobar") if err := ffs.MkdirAll(subDir, 0777); err != nil { @@ -741,14 +741,14 @@ func TestDiffEmpty(t *testing.T) { // option is true and the permissions do not match between the file on disk and // in the db. func TestDeleteIgnorePerms(t *testing.T) { - m, f, tmpDir := setupSendReceiveFolder() + m, f := setupSendReceiveFolder() + ffs := f.Filesystem() defer func() { os.Remove(m.cfg.ConfigPath()) - os.Remove(tmpDir) + os.Remove(ffs.URI()) }() f.IgnorePerms = true - ffs := f.Filesystem() name := "deleteIgnorePerms" file, err := ffs.Create(name) if err != nil { @@ -797,7 +797,7 @@ func TestCopyOwner(t *testing.T) { // Set up a folder with the CopyParentOwner bit and backed by a fake // filesystem. - m, f, _ := setupSendReceiveFolder() + m, f := setupSendReceiveFolder() defer os.Remove(m.cfg.ConfigPath()) f.folder.FolderConfiguration = config.NewFolderConfiguration(m.id, f.ID, f.Label, fs.FilesystemTypeFake, "/TestCopyOwner") f.folder.FolderConfiguration.CopyOwnershipFromParent = true @@ -886,13 +886,13 @@ func TestCopyOwner(t *testing.T) { // TestSRConflictReplaceFileByDir checks that a conflict is created when an existing file // is replaced with a directory and versions are conflicting func TestSRConflictReplaceFileByDir(t *testing.T) { - m, f, tmpDir := setupSendReceiveFolder() + m, f := setupSendReceiveFolder() + ffs := f.Filesystem() defer func() { os.Remove(m.cfg.ConfigPath()) - os.Remove(tmpDir) + os.Remove(ffs.URI()) }() - ffs := f.Filesystem() name := "foo" // create local file @@ -921,13 +921,13 @@ func TestSRConflictReplaceFileByDir(t *testing.T) { // TestSRConflictReplaceFileByLink checks that a conflict is created when an existing file // is replaced with a link and versions are conflicting func TestSRConflictReplaceFileByLink(t *testing.T) { - m, f, tmpDir := setupSendReceiveFolder() + m, f := setupSendReceiveFolder() + ffs := f.Filesystem() defer func() { os.Remove(m.cfg.ConfigPath()) - os.Remove(tmpDir) + os.Remove(ffs.URI()) }() - ffs := f.Filesystem() name := "foo" // create local file diff --git a/lib/model/model_test.go b/lib/model/model_test.go index 2d76540d9..aba8a128d 100644 --- a/lib/model/model_test.go +++ b/lib/model/model_test.go @@ -2869,14 +2869,13 @@ func TestIssue2571(t *testing.T) { t.Skip("Scanning symlinks isn't supported on windows") } - w, tmpDir := tmpDefaultWrapper() + w, fcfg := tmpDefaultWrapper() + testFs := fcfg.Filesystem() defer func() { - os.RemoveAll(tmpDir) + os.RemoveAll(testFs.URI()) os.Remove(w.ConfigPath()) }() - testFs := fs.NewFilesystem(fs.FilesystemTypeBasic, tmpDir) - for _, dir := range []string{"toLink", "linkTarget"} { err := testFs.MkdirAll(dir, 0775) if err != nil { @@ -2919,14 +2918,13 @@ func TestIssue4573(t *testing.T) { t.Skip("Can't make the dir inaccessible on windows") } - w, tmpDir := tmpDefaultWrapper() + w, fcfg := tmpDefaultWrapper() + testFs := fcfg.Filesystem() defer func() { - os.RemoveAll(tmpDir) + os.RemoveAll(testFs.URI()) os.Remove(w.ConfigPath()) }() - testFs := fs.NewFilesystem(fs.FilesystemTypeBasic, tmpDir) - err := testFs.MkdirAll("inaccessible", 0755) if err != nil { t.Fatal(err) @@ -2959,14 +2957,13 @@ 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, tmpDir := tmpDefaultWrapper() + w, fcfg := tmpDefaultWrapper() + testFs := fcfg.Filesystem() defer func() { - os.RemoveAll(tmpDir) + os.RemoveAll(testFs.URI()) os.Remove(w.ConfigPath()) }() - testFs := fs.NewFilesystem(fs.FilesystemTypeBasic, tmpDir) - testCases := map[string]func(protocol.FileInfo) bool{ "removeDir": func(f protocol.FileInfo) bool { return !f.Deleted @@ -3157,15 +3154,14 @@ func TestRemoveDirWithContent(t *testing.T) { } func TestIssue4475(t *testing.T) { - m, conn, tmpDir, w := setupModelWithConnection() + m, conn, fcfg, w := setupModelWithConnection() + testFs := fcfg.Filesystem() defer func() { m.Stop() - os.RemoveAll(tmpDir) + os.RemoveAll(testFs.URI()) os.Remove(w.ConfigPath()) }() - testFs := fs.NewFilesystem(fs.FilesystemTypeBasic, tmpDir) - // Scenario: Dir is deleted locally and before syncing/index exchange // happens, a file is create in that dir on the remote. // This should result in the directory being recreated and added to the diff --git a/lib/model/requests_test.go b/lib/model/requests_test.go index 308bc0a00..8e8f167fc 100644 --- a/lib/model/requests_test.go +++ b/lib/model/requests_test.go @@ -28,10 +28,11 @@ func TestRequestSimple(t *testing.T) { // Verify that the model performs a request and creates a file based on // an incoming index update. - m, fc, tmpDir, w := setupModelWithConnection() + m, fc, fcfg, w := setupModelWithConnection() + tfs := fcfg.Filesystem() defer func() { m.Stop() - os.RemoveAll(tmpDir) + os.RemoveAll(tfs.URI()) os.Remove(w.ConfigPath()) }() @@ -61,7 +62,7 @@ func TestRequestSimple(t *testing.T) { <-done // Verify the contents - if err := equalContents(filepath.Join(tmpDir, "testfile"), contents); err != nil { + if err := equalContents(filepath.Join(tfs.URI(), "testfile"), contents); err != nil { t.Error("File did not sync correctly:", err) } } @@ -74,10 +75,10 @@ func TestSymlinkTraversalRead(t *testing.T) { return } - m, fc, tmpDir, w := setupModelWithConnection() + m, fc, fcfg, w := setupModelWithConnection() defer func() { m.Stop() - os.RemoveAll(tmpDir) + os.RemoveAll(fcfg.Filesystem().URI()) os.Remove(w.ConfigPath()) }() @@ -121,10 +122,10 @@ func TestSymlinkTraversalWrite(t *testing.T) { return } - m, fc, tmpDir, w := setupModelWithConnection() + m, fc, fcfg, w := setupModelWithConnection() defer func() { m.Stop() - os.RemoveAll(tmpDir) + os.RemoveAll(fcfg.Filesystem().URI()) os.Remove(w.ConfigPath()) }() @@ -184,10 +185,10 @@ func TestSymlinkTraversalWrite(t *testing.T) { func TestRequestCreateTmpSymlink(t *testing.T) { // Test that an update for a temporary file is invalidated - m, fc, tmpDir, w := setupModelWithConnection() + m, fc, fcfg, w := setupModelWithConnection() defer func() { m.Stop() - os.RemoveAll(tmpDir) + os.RemoveAll(fcfg.Filesystem().URI()) os.Remove(w.ConfigPath()) }() @@ -229,13 +230,12 @@ func TestRequestVersioningSymlinkAttack(t *testing.T) { // Sets up a folder with trashcan versioning and tries to use a // deleted symlink to escape - w, tmpDir := tmpDefaultWrapper() + w, fcfg := tmpDefaultWrapper() defer func() { - os.RemoveAll(tmpDir) + os.RemoveAll(fcfg.Filesystem().URI()) os.Remove(w.ConfigPath()) }() - fcfg := w.FolderList()[0] fcfg.Versioning = config.VersioningConfiguration{Type: "trashcan"} w.SetFolder(fcfg) @@ -248,6 +248,7 @@ func TestRequestVersioningSymlinkAttack(t *testing.T) { if err != nil { t.Fatal(err) } + defer os.RemoveAll(tmpdir) // We listen for incoming index updates and trigger when we see one for // the expected test file. @@ -304,13 +305,14 @@ func pullInvalidIgnored(t *testing.T, ft config.FolderType) { t.Helper() w := createTmpWrapper(defaultCfgWrapper.RawCopy()) - fcfg, tmpDir := testFolderConfigTmp() + fcfg := testFolderConfigTmp() + fss := fcfg.Filesystem() fcfg.Type = ft w.SetFolder(fcfg) m, fc := setupModelWithConnectionFromWrapper(w) defer func() { m.Stop() - os.RemoveAll(tmpDir) + os.RemoveAll(fss.URI()) os.Remove(w.ConfigPath()) }() @@ -319,7 +321,7 @@ func pullInvalidIgnored(t *testing.T, ft config.FolderType) { // because we might be changing the files on disk often enough that the // mtimes will be unreliable to determine change status. m.fmut.Lock() - m.folderIgnores["default"] = ignore.New(fcfg.Filesystem(), ignore.WithChangeDetector(newAlwaysChanged())) + m.folderIgnores["default"] = ignore.New(fss, ignore.WithChangeDetector(newAlwaysChanged())) m.fmut.Unlock() if err := m.SetIgnores("default", []string{"*ignored*"}); err != nil { @@ -339,7 +341,7 @@ func pullInvalidIgnored(t *testing.T, ft config.FolderType) { fc.deleteFile(invDel) fc.addFile(ign, 0644, protocol.FileInfoTypeFile, contents) fc.addFile(ignExisting, 0644, protocol.FileInfoTypeFile, contents) - if err := ioutil.WriteFile(filepath.Join(tmpDir, ignExisting), otherContents, 0644); err != nil { + if err := ioutil.WriteFile(filepath.Join(fss.URI(), ignExisting), otherContents, 0644); err != nil { panic(err) } @@ -429,10 +431,10 @@ func pullInvalidIgnored(t *testing.T, ft config.FolderType) { } func TestIssue4841(t *testing.T) { - m, fc, tmpDir, w := setupModelWithConnection() + m, fc, fcfg, w := setupModelWithConnection() defer func() { m.Stop() - os.RemoveAll(tmpDir) + os.RemoveAll(fcfg.Filesystem().URI()) os.Remove(w.ConfigPath()) }() @@ -470,7 +472,8 @@ func TestIssue4841(t *testing.T) { } func TestRescanIfHaveInvalidContent(t *testing.T) { - m, fc, tmpDir, w := setupModelWithConnection() + m, fc, fcfg, w := setupModelWithConnection() + tmpDir := fcfg.Filesystem().URI() defer func() { m.Stop() os.RemoveAll(tmpDir) @@ -538,16 +541,16 @@ func TestRescanIfHaveInvalidContent(t *testing.T) { } func TestParentDeletion(t *testing.T) { - m, fc, tmpDir, w := setupModelWithConnection() + m, fc, fcfg, w := setupModelWithConnection() + testFs := fcfg.Filesystem() defer func() { m.Stop() - os.RemoveAll(tmpDir) + os.RemoveAll(testFs.URI()) os.Remove(w.ConfigPath()) }() parent := "foo" child := filepath.Join(parent, "bar") - testFs := fs.NewFilesystem(fs.FilesystemTypeBasic, tmpDir) received := make(chan []protocol.FileInfo) fc.addFile(parent, 0777, protocol.FileInfoTypeDirectory, nil) @@ -623,10 +626,10 @@ func TestRequestSymlinkWindows(t *testing.T) { t.Skip("windows specific test") } - m, fc, tmpDir, w := setupModelWithConnection() + m, fc, fcfg, w := setupModelWithConnection() defer func() { m.Stop() - os.RemoveAll(tmpDir) + os.RemoveAll(fcfg.Filesystem().URI()) os.Remove(w.ConfigPath()) }() @@ -684,16 +687,16 @@ func TestRequestSymlinkWindows(t *testing.T) { } } -func tmpDefaultWrapper() (config.Wrapper, string) { +func tmpDefaultWrapper() (config.Wrapper, config.FolderConfiguration) { w := createTmpWrapper(defaultCfgWrapper.RawCopy()) - fcfg, tmpDir := testFolderConfigTmp() + fcfg := testFolderConfigTmp() w.SetFolder(fcfg) - return w, tmpDir + return w, fcfg } -func testFolderConfigTmp() (config.FolderConfiguration, string) { +func testFolderConfigTmp() config.FolderConfiguration { tmpDir := createTmpDir() - return testFolderConfig(tmpDir), tmpDir + return testFolderConfig(tmpDir) } func testFolderConfig(path string) config.FolderConfiguration { @@ -703,10 +706,10 @@ func testFolderConfig(path string) config.FolderConfiguration { return cfg } -func setupModelWithConnection() (*model, *fakeConnection, string, config.Wrapper) { - w, tmpDir := tmpDefaultWrapper() +func setupModelWithConnection() (*model, *fakeConnection, config.FolderConfiguration, config.Wrapper) { + w, fcfg := tmpDefaultWrapper() m, fc := setupModelWithConnectionFromWrapper(w) - return m, fc, tmpDir, w + return m, fc, fcfg, w } func setupModelWithConnectionFromWrapper(w config.Wrapper) (*model, *fakeConnection) { @@ -738,13 +741,14 @@ func equalContents(path string, contents []byte) error { } func TestRequestRemoteRenameChanged(t *testing.T) { - m, fc, tmpDir, w := setupModelWithConnection() + m, fc, fcfg, w := setupModelWithConnection() + tfs := fcfg.Filesystem() + tmpDir := tfs.URI() defer func() { m.Stop() os.RemoveAll(tmpDir) os.Remove(w.ConfigPath()) }() - tfs := fs.NewFilesystem(fs.FilesystemTypeBasic, tmpDir) done := make(chan struct{}) fc.mut.Lock() @@ -871,13 +875,14 @@ func TestRequestRemoteRenameChanged(t *testing.T) { } func TestRequestRemoteRenameConflict(t *testing.T) { - m, fc, tmpDir, w := setupModelWithConnection() + m, fc, fcfg, w := setupModelWithConnection() + tfs := fcfg.Filesystem() + tmpDir := tfs.URI() defer func() { m.Stop() os.RemoveAll(tmpDir) os.Remove(w.ConfigPath()) }() - tfs := fs.NewFilesystem(fs.FilesystemTypeBasic, tmpDir) recv := make(chan int) fc.mut.Lock() @@ -967,13 +972,13 @@ func TestRequestRemoteRenameConflict(t *testing.T) { } func TestRequestDeleteChanged(t *testing.T) { - m, fc, tmpDir, w := setupModelWithConnection() + m, fc, fcfg, w := setupModelWithConnection() + tfs := fcfg.Filesystem() defer func() { m.Stop() - os.RemoveAll(tmpDir) + os.RemoveAll(tfs.URI()) os.Remove(w.ConfigPath()) }() - tfs := fs.NewFilesystem(fs.FilesystemTypeBasic, tmpDir) done := make(chan struct{}) fc.mut.Lock()