all: Grand test refactor (fixes #8779, fixes #8799)

This fixes various test issues with Go 1.20.

- Most tests rewritten to use fakefs where possible
- Some tests that were already skipped, or dubious (invasive,
  unmaintainable, unclear what they even tested) have been removed
- Some actual code rewritten to better support testing in fakefs

Co-authored-by: Eric P <eric@kastelo.net>

54 files changed, 1127 insertions, 1588 deletions

diff --git a/.github/workflows/build-syncthing.yaml b/.github/workflows/build-syncthing.yaml
index 966e23cd5c..7b8cc91bb7 100644
--- a/.github/workflows/build-syncthing.yaml
+++ b/.github/workflows/build-syncthing.yaml
@@ -6,7 +6,7 @@ on:
 
 env:
   # The go version to use for builds.
-  GO_VERSION: "1.19.6"
+  GO_VERSION: "^1.20.3"
 
   # Optimize compatibility on the slow archictures.
   GO386: softfloat
@@ -42,7 +42,7 @@ jobs:
         runner: ["windows-latest", "ubuntu-latest", "macos-latest"]
         # The oldest version in this list should match what we have in our go.mod.
         # Variables don't seem to be supported here, or we could have done something nice.
-        go: ["1.19"] # Skip Go 1.20 for now, https://github.com/syncthing/syncthing/issues/8799
+        go: ["1.19", "1.20"]
     runs-on: ${{ matrix.runner }}
     steps:
       - name: Set git to use LF
diff --git a/lib/api/api.go b/lib/api/api.go
index 6f9451dcc5..5e41badb6a 100644
--- a/lib/api/api.go
+++ b/lib/api/api.go
@@ -775,9 +775,9 @@ func (s *service) getDBBrowse(w http.ResponseWriter, r *http.Request) {
 }
 
 func (s *service) getDBCompletion(w http.ResponseWriter, r *http.Request) {
-	var qs = r.URL.Query()
-	var folder = qs.Get("folder")    // empty means all folders
-	var deviceStr = qs.Get("device") // empty means local device ID
+	qs := r.URL.Query()
+	folder := qs.Get("folder")    // empty means all folders
+	deviceStr := qs.Get("device") // empty means local device ID
 
 	// We will check completion status for either the local device, or a
 	// specific given device ID.
@@ -814,14 +814,14 @@ func (s *service) getDBStatus(w http.ResponseWriter, r *http.Request) {
 }
 
 func (s *service) postDBOverride(_ http.ResponseWriter, r *http.Request) {
-	var qs = r.URL.Query()
-	var folder = qs.Get("folder")
+	qs := r.URL.Query()
+	folder := qs.Get("folder")
 	go s.model.Override(folder)
 }
 
 func (s *service) postDBRevert(_ http.ResponseWriter, r *http.Request) {
-	var qs = r.URL.Query()
-	var folder = qs.Get("folder")
+	qs := r.URL.Query()
+	folder := qs.Get("folder")
 	go s.model.Revert(folder)
 }
 
@@ -1015,7 +1015,7 @@ func (s *service) postSystemRestart(w http.ResponseWriter, _ *http.Request) {
 }
 
 func (s *service) postSystemReset(w http.ResponseWriter, r *http.Request) {
-	var qs = r.URL.Query()
+	qs := r.URL.Query()
 	folder := qs.Get("folder")
 
 	if len(folder) > 0 {
@@ -1210,7 +1210,6 @@ func (s *service) getSupportBundle(w http.ResponseWriter, r *http.Request) {
 			l.Warnln("Support bundle: failed to serialize usage-reporting.json.txt", err)
 		} else {
 			files = append(files, fileEntry{name: "usage-reporting.json.txt", data: usageReportingData})
-
 		}
 	}
 
@@ -1243,7 +1242,7 @@ func (s *service) getSupportBundle(w http.ResponseWriter, r *http.Request) {
 	zipFilePath := filepath.Join(locations.GetBaseDir(locations.ConfigBaseDir), zipFileName)
 
 	// Write buffer zip to local zip file (back up)
-	if err := os.WriteFile(zipFilePath, zipFilesBuffer.Bytes(), 0600); err != nil {
+	if err := os.WriteFile(zipFilePath, zipFilesBuffer.Bytes(), 0o600); err != nil {
 		l.Warnln("Support bundle: support bundle zip could not be created:", err)
 	}
 
@@ -1299,7 +1298,6 @@ func (s *service) getReport(w http.ResponseWriter, r *http.Request) {
 	} else {
 		sendJSON(w, r)
 	}
-
 }
 
 func (*service) getRandomString(w http.ResponseWriter, r *http.Request) {
@@ -1497,8 +1495,8 @@ func (s *service) postSystemUpgrade(w http.ResponseWriter, _ *http.Request) {
 
 func (s *service) makeDevicePauseHandler(paused bool) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
-		var qs = r.URL.Query()
-		var deviceStr = qs.Get("device")
+		qs := r.URL.Query()
+		deviceStr := qs.Get("device")
 
 		var msg string
 		var status int
@@ -1573,8 +1571,8 @@ func (*service) getHealth(w http.ResponseWriter, _ *http.Request) {
 }
 
 func (*service) getQR(w http.ResponseWriter, r *http.Request) {
-	var qs = r.URL.Query()
-	var text = qs.Get("text")
+	qs := r.URL.Query()
+	text := qs.Get("text")
 	code, err := qr.Encode(text, qr.M)
 	if err != nil {
 		http.Error(w, "Invalid", http.StatusInternalServerError)
@@ -1655,7 +1653,6 @@ func (s *service) getFolderErrors(w http.ResponseWriter, r *http.Request) {
 	page, perpage := getPagingParams(qs)
 
 	errors, err := s.model.FolderErrors(folder)
-
 	if err != nil {
 		http.Error(w, err.Error(), http.StatusNotFound)
 		return
@@ -1687,7 +1684,21 @@ func (*service) getSystemBrowse(w http.ResponseWriter, r *http.Request) {
 	var fsType fs.FilesystemType
 	fsType.UnmarshalText([]byte(qs.Get("filesystem")))
 
-	sendJSON(w, browseFiles(current, fsType))
+	sendJSON(w, browse(fsType, current))
+}
+
+func browse(fsType fs.FilesystemType, current string) []string {
+	if current == "" {
+		return browseRoots(fsType)
+	}
+
+	parent, base := parentAndBase(current)
+	ffs := fs.NewFilesystem(fsType, parent)
+	files := browseFiles(ffs, base)
+	for i := range files {
+		files[i] = filepath.Join(parent, files[i])
+	}
+	return files
 }
 
 const (
@@ -1708,14 +1719,18 @@ func checkPrefixMatch(s, prefix string) int {
 	return noMatch
 }
 
-func browseFiles(current string, fsType fs.FilesystemType) []string {
-	if current == "" {
-		filesystem := fs.NewFilesystem(fsType, "")
-		if roots, err := filesystem.Roots(); err == nil {
-			return roots
-		}
-		return nil
+func browseRoots(fsType fs.FilesystemType) []string {
+	filesystem := fs.NewFilesystem(fsType, "")
+	if roots, err := filesystem.Roots(); err == nil {
+		return roots
 	}
+
+	return nil
+}
+
+// parentAndBase returns the parent directory and the remaining base of the
+// path. The base may be empty if the path ends with a path separator.
+func parentAndBase(current string) (string, string) {
 	search, _ := fs.ExpandTilde(current)
 	pathSeparator := string(fs.PathSeparator)
 
@@ -1731,24 +1746,27 @@ func browseFiles(current string, fsType fs.FilesystemType) []string {
 		searchFile = filepath.Base(search)
 	}
 
-	fs := fs.NewFilesystem(fsType, searchDir)
+	return searchDir, searchFile
+}
 
-	subdirectories, _ := fs.DirNames(".")
+func browseFiles(ffs fs.Filesystem, search string) []string {
+	subdirectories, _ := ffs.DirNames(".")
+	pathSeparator := string(fs.PathSeparator)
 
 	exactMatches := make([]string, 0, len(subdirectories))
 	caseInsMatches := make([]string, 0, len(subdirectories))
 
 	for _, subdirectory := range subdirectories {
-		info, err := fs.Stat(subdirectory)
+		info, err := ffs.Stat(subdirectory)
 		if err != nil || !info.IsDir() {
 			continue
 		}
 
-		switch checkPrefixMatch(subdirectory, searchFile) {
+		switch checkPrefixMatch(subdirectory, search) {
 		case matchExact:
-			exactMatches = append(exactMatches, filepath.Join(searchDir, subdirectory)+pathSeparator)
+			exactMatches = append(exactMatches, subdirectory+pathSeparator)
 		case matchCaseIns:
-			caseInsMatches = append(caseInsMatches, filepath.Join(searchDir, subdirectory)+pathSeparator)
+			caseInsMatches = append(caseInsMatches, subdirectory+pathSeparator)
 		}
 	}
 
diff --git a/lib/api/api_test.go b/lib/api/api_test.go
index 7736b9b8c3..3cb01d7892 100644
--- a/lib/api/api_test.go
+++ b/lib/api/api_test.go
@@ -39,6 +39,7 @@ import (
 	"github.com/syncthing/syncthing/lib/model"
 	modelmocks "github.com/syncthing/syncthing/lib/model/mocks"
 	"github.com/syncthing/syncthing/lib/protocol"
+	"github.com/syncthing/syncthing/lib/rand"
 	"github.com/syncthing/syncthing/lib/svcutil"
 	"github.com/syncthing/syncthing/lib/sync"
 	"github.com/syncthing/syncthing/lib/tlsutil"
@@ -1168,45 +1169,39 @@ func TestBrowse(t *testing.T) {
 
 	pathSep := string(os.PathSeparator)
 
-	tmpDir := t.TempDir()
+	ffs := fs.NewFilesystem(fs.FilesystemTypeFake, rand.String(32)+"?nostfolder=true")
 
-	if err := os.Mkdir(filepath.Join(tmpDir, "dir"), 0755); err != nil {
-		t.Fatal(err)
-	}
-	if err := os.WriteFile(filepath.Join(tmpDir, "file"), []byte("hello"), 0644); err != nil {
-		t.Fatal(err)
-	}
-	if err := os.Mkdir(filepath.Join(tmpDir, "MiXEDCase"), 0755); err != nil {
-		t.Fatal(err)
-	}
+	_ = ffs.Mkdir("dir", 0o755)
+	_ = fs.WriteFile(ffs, "file", []byte("hello"), 0o644)
+	_ = ffs.Mkdir("MiXEDCase", 0o755)
 
 	// We expect completion to return the full path to the completed
 	// directory, with an ending slash.
-	dirPath := filepath.Join(tmpDir, "dir") + pathSep
-	mixedCaseDirPath := filepath.Join(tmpDir, "MiXEDCase") + pathSep
+	dirPath := "dir" + pathSep
+	mixedCaseDirPath := "MiXEDCase" + pathSep
 
 	cases := []struct {
 		current string
 		returns []string
 	}{
 		// The directory without slash is completed to one with slash.
-		{tmpDir, []string{tmpDir + pathSep}},
+		{"dir", []string{"dir" + pathSep}},
 		// With slash it's completed to its contents.
 		// Dirs are given pathSeps.
 		// Files are not returned.
-		{tmpDir + pathSep, []string{mixedCaseDirPath, dirPath}},
+		{"", []string{mixedCaseDirPath, dirPath}},
 		// Globbing is automatic based on prefix.
-		{tmpDir + pathSep + "d", []string{dirPath}},
-		{tmpDir + pathSep + "di", []string{dirPath}},
-		{tmpDir + pathSep + "dir", []string{dirPath}},
-		{tmpDir + pathSep + "f", nil},
-		{tmpDir + pathSep + "q", nil},
+		{"d", []string{dirPath}},
+		{"di", []string{dirPath}},
+		{"dir", []string{dirPath}},
+		{"f", nil},
+		{"q", nil},
 		// Globbing is case-insensitive
-		{tmpDir + pathSep + "mixed", []string{mixedCaseDirPath}},
+		{"mixed", []string{mixedCaseDirPath}},
 	}
 
 	for _, tc := range cases {
-		ret := browseFiles(tc.current, fs.FilesystemTypeBasic)
+		ret := browseFiles(ffs, tc.current)
 		if !util.EqualStrings(ret, tc.returns) {
 			t.Errorf("browseFiles(%q) => %q, expected %q", tc.current, ret, tc.returns)
 		}
diff --git a/lib/config/config_test.go b/lib/config/config_test.go
index a0446e6445..0b667e44d7 100644
--- a/lib/config/config_test.go
+++ b/lib/config/config_test.go
@@ -27,15 +27,21 @@ import (
 	"github.com/syncthing/syncthing/lib/events"
 	"github.com/syncthing/syncthing/lib/fs"
 	"github.com/syncthing/syncthing/lib/protocol"
+	"github.com/syncthing/syncthing/lib/rand"
 )
 
 var device1, device2, device3, device4 protocol.DeviceID
 
+var testFs fs.Filesystem
+
 func init() {
 	device1, _ = protocol.DeviceIDFromString("AIR6LPZ7K4PTTUXQSMUUCPQ5YWOEDFIIQJUG7772YQXXR5YD6AWQ")
 	device2, _ = protocol.DeviceIDFromString("GYRZZQB-IRNPV4Z-T7TC52W-EQYJ3TT-FDQW6MW-DFLMU42-SSSU6EM-FBK2VAY")
 	device3, _ = protocol.DeviceIDFromString("LGFPDIT-7SKNNJL-VJZA4FC-7QNCRKA-CE753K7-2BW5QDK-2FOZ7FR-FEP57QJ")
 	device4, _ = protocol.DeviceIDFromString("P56IOI7-MZJNU2Y-IQGDREY-DM2MGTI-MGL3BXN-PQ6W5BM-TBBZ4TJ-XZWICQ2")
+
+	testFs = fs.NewFilesystem(fs.FilesystemTypeFake, rand.String(32)+"?content=true")
+	loadTestFiles()
 }
 
 func TestDefaultValues(t *testing.T) {
@@ -144,19 +150,19 @@ func TestDefaultValues(t *testing.T) {
 
 func TestDeviceConfig(t *testing.T) {
 	for i := OldestHandledVersion; i <= CurrentVersion; i++ {
-		cfgFile := fmt.Sprintf("testdata/v%d.xml", i)
-		if _, err := os.Stat(cfgFile); os.IsNotExist(err) {
+		cfgFile := fmt.Sprintf("v%d.xml", i)
+		if _, err := testFs.Stat(cfgFile); os.IsNotExist(err) {
 			continue
 		}
 
-		os.RemoveAll(filepath.Join("testdata", DefaultMarkerName))
-		wr, wrCancel, err := copyAndLoad(cfgFile, device1)
+		testFs.RemoveAll(DefaultMarkerName)
+		wr, wrCancel, err := copyAndLoad(testFs, cfgFile, device1)
 		defer wrCancel()
 		if err != nil {
 			t.Fatal(err)
 		}
 
-		_, err = os.Stat(filepath.Join("testdata", DefaultMarkerName))
+		_, err = testFs.Stat(DefaultMarkerName)
 		if i < 6 && err != nil {
 			t.Fatal(err)
 		} else if i >= 6 && err == nil {
@@ -217,7 +223,7 @@ func TestDeviceConfig(t *testing.T) {
 			t.Errorf("%d: Incorrect version %d != %d", i, cfg.Version, CurrentVersion)
 		}
 		if diff, equal := messagediff.PrettyDiff(expectedFolders, cfg.Folders); !equal {
-			t.Errorf("%d: Incorrect Folders. Diff:\n%s", i, diff)
+			t.Errorf("%d: Incorrect Folders. Diff:\n%s\n%v", i, diff, cfg)
 		}
 		if diff, equal := messagediff.PrettyDiff(expectedDevices, cfg.Devices); !equal {
 			t.Errorf("%d: Incorrect Devices. Diff:\n%s", i, diff)
@@ -229,10 +235,10 @@ func TestDeviceConfig(t *testing.T) {
 }
 
 func TestNoListenAddresses(t *testing.T) {
-	cfg, cfgCancel, err := copyAndLoad("testdata/nolistenaddress.xml", device1)
+	cfg, cfgCancel, err := copyAndLoad(testFs, "nolistenaddress.xml", device1)
 	defer cfgCancel()
 	if err != nil {
-		t.Error(err)
+		t.Fatal(err)
 	}
 
 	expected := []string{""}
@@ -292,7 +298,7 @@ func TestOverriddenValues(t *testing.T) {
 	expectedPath := "/media/syncthing"
 
 	os.Unsetenv("STNOUPGRADE")
-	cfg, cfgCancel, err := copyAndLoad("testdata/overridenvalues.xml", device1)
+	cfg, cfgCancel, err := copyAndLoad(testFs, "overridenvalues.xml", device1)
 	defer cfgCancel()
 	if err != nil {
 		t.Error(err)
@@ -338,7 +344,7 @@ func TestDeviceAddressesDynamic(t *testing.T) {
 		},
 	}
 
-	cfg, cfgCancel, err := copyAndLoad("testdata/deviceaddressesdynamic.xml", device4)
+	cfg, cfgCancel, err := copyAndLoad(testFs, "deviceaddressesdynamic.xml", device4)
 	defer cfgCancel()
 	if err != nil {
 		t.Error(err)
@@ -384,7 +390,7 @@ func TestDeviceCompression(t *testing.T) {
 		},
 	}
 
-	cfg, cfgCancel, err := copyAndLoad("testdata/devicecompression.xml", device4)
+	cfg, cfgCancel, err := copyAndLoad(testFs, "devicecompression.xml", device4)
 	defer cfgCancel()
 	if err != nil {
 		t.Error(err)
@@ -427,7 +433,7 @@ func TestDeviceAddressesStatic(t *testing.T) {
 		},
 	}
 
-	cfg, cfgCancel, err := copyAndLoad("testdata/deviceaddressesstatic.xml", device4)
+	cfg, cfgCancel, err := copyAndLoad(testFs, "deviceaddressesstatic.xml", device4)
 	defer cfgCancel()
 	if err != nil {
 		t.Error(err)
@@ -440,7 +446,7 @@ func TestDeviceAddressesStatic(t *testing.T) {
 }
 
 func TestVersioningConfig(t *testing.T) {
-	cfg, cfgCancel, err := copyAndLoad("testdata/versioningconfig.xml", device4)
+	cfg, cfgCancel, err := copyAndLoad(testFs, "versioningconfig.xml", device4)
 	defer cfgCancel()
 	if err != nil {
 		t.Error(err)
@@ -468,7 +474,7 @@ func TestIssue1262(t *testing.T) {
 		t.Skipf("path gets converted to absolute as part of the filesystem initialization on linux")
 	}
 
-	cfg, cfgCancel, err := copyAndLoad("testdata/issue-1262.xml", device4)
+	cfg, cfgCancel, err := copyAndLoad(testFs, "issue-1262.xml", device4)
 	defer cfgCancel()
 	if err != nil {
 		t.Fatal(err)
@@ -483,7 +489,7 @@ func TestIssue1262(t *testing.T) {
 }
 
 func TestIssue1750(t *testing.T) {
-	cfg, cfgCancel, err := copyAndLoad("testdata/issue-1750.xml", device4)
+	cfg, cfgCancel, err := copyAndLoad(testFs, "issue-1750.xml", device4)
 	defer cfgCancel()
 	if err != nil {
 		t.Fatal(err)
@@ -521,20 +527,15 @@ func TestFolderPath(t *testing.T) {
 }
 
 func TestFolderCheckPath(t *testing.T) {
-	n := t.TempDir()
-	testFs := fs.NewFilesystem(fs.FilesystemTypeBasic, n)
-
-	err := os.MkdirAll(filepath.Join(n, "dir", ".stfolder"), os.FileMode(0o777))
-	if err != nil {
-		t.Fatal(err)
-	}
+	tmpFs := fs.NewFilesystem(fs.FilesystemTypeFake, rand.String(16)+"?nostfolder=true")
+	_ = tmpFs.MkdirAll(filepath.Join("dir", ".stfolder"), 0o777)
 
 	testcases := []struct {
 		path string
 		err  error
 	}{
 		{
-			path: "",
+			path: ".",
 			err:  ErrMarkerMissing,
 		},
 		{
@@ -547,35 +548,20 @@ func TestFolderCheckPath(t *testing.T) {
 		},
 	}
 
-	err = fs.DebugSymlinkForTestsOnly(testFs, testFs, "dir", "link")
-	if err == nil {
-		t.Log("running with symlink check")
-		testcases = append(testcases, struct {
-			path string
-			err  error
-		}{
-			path: "link",
-			err:  nil,
-		})
-	} else if !build.IsWindows {
-		t.Log("running without symlink check")
-		t.Fatal(err)
-	}
-
 	for _, testcase := range testcases {
 		cfg := FolderConfiguration{
-			Path:       filepath.Join(n, testcase.path),
-			MarkerName: DefaultMarkerName,
+			FilesystemType: fs.FilesystemTypeFake,
+			MarkerName:     DefaultMarkerName,
 		}
 
-		if err := cfg.CheckPath(); testcase.err != err {
-			t.Errorf("unexpected error in case %s: %s != %v", testcase.path, err, testcase.err)
+		if err := cfg.checkFilesystemPath(tmpFs, testcase.path); testcase.err != err {
+			t.Errorf("unexpected error in case; path: [%s] err [%s] expected err [%v]", testcase.path, err, testcase.err)
 		}
 	}
 }
 
 func TestNewSaveLoad(t *testing.T) {
-	path := "testdata/temp.xml"
+	path := "temp.xml"
 	os.Remove(path)
 	defer os.Remove(path)
 
@@ -657,7 +643,7 @@ func TestPrepare(t *testing.T) {
 }
 
 func TestCopy(t *testing.T) {
-	wrapper, wrapperCancel, err := copyAndLoad("testdata/example.xml", device1)
+	wrapper, wrapperCancel, err := copyAndLoad(testFs, "example.xml", device1)
 	defer wrapperCancel()
 	if err != nil {
 		t.Fatal(err)
@@ -690,14 +676,12 @@ func TestCopy(t *testing.T) {
 		t.Error("Config should have changed")
 	}
 	if !bytes.Equal(bsOrig, bsCopy) {
-		// os.WriteFile("a", bsOrig, 0644)
-		// os.WriteFile("b", bsCopy, 0644)
 		t.Error("Copy should be unchanged")
 	}
 }
 
 func TestPullOrder(t *testing.T) {
-	wrapper, wrapperCleanup, err := copyAndLoad("testdata/pullorder.xml", device1)
+	wrapper, wrapperCleanup, err := copyAndLoad(testFs, "pullorder.xml", device1)
 	defer wrapperCleanup()
 	if err != nil {
 		t.Fatal(err)
@@ -750,7 +734,7 @@ func TestPullOrder(t *testing.T) {
 }
 
 func TestLargeRescanInterval(t *testing.T) {
-	wrapper, wrapperCancel, err := copyAndLoad("testdata/largeinterval.xml", device1)
+	wrapper, wrapperCancel, err := copyAndLoad(testFs, "largeinterval.xml", device1)
 	defer wrapperCancel()
 	if err != nil {
 		t.Fatal(err)
@@ -810,7 +794,7 @@ func TestGUIPasswordHash(t *testing.T) {
 func TestDuplicateDevices(t *testing.T) {
 	// Duplicate devices should be removed
 
-	wrapper, wrapperCancel, err := copyAndLoad("testdata/dupdevices.xml", device1)
+	wrapper, wrapperCancel, err := copyAndLoad(testFs, "dupdevices.xml", device1)
 	defer wrapperCancel()
 	if err != nil {
 		t.Fatal(err)
@@ -829,7 +813,7 @@ func TestDuplicateDevices(t *testing.T) {
 func TestDuplicateFolders(t *testing.T) {
 	// Duplicate folders are a loading error
 
-	_, _Cancel, err := copyAndLoad("testdata/dupfolders.xml", device1)
+	_, _Cancel, err := copyAndLoad(testFs, "dupfolders.xml", device1)
 	defer _Cancel()
 	if err == nil || !strings.Contains(err.Error(), errFolderIDDuplicate.Error()