lib/model: Improve test utilities (#5584)

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

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

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()