diff --git a/internal/configuration/setup/Setup_test.go b/internal/configuration/setup/Setup_test.go index 2bd326fc..cdeb5783 100644 --- a/internal/configuration/setup/Setup_test.go +++ b/internal/configuration/setup/Setup_test.go @@ -729,7 +729,11 @@ func TestIsErrorAddressAlreadyInUse(t *testing.T) { l, err := net.Listen("tcp", "127.0.0.1:19888") test.IsNil(t, err) srv2 := http.Server{ - Addr: ":19888", + // Must bind to the exact same address as the listener above, not just + // the same port on all interfaces - on Windows, binding to the + // wildcard address does not reliably conflict with an existing + // listener bound to a specific address on the same port. + Addr: "127.0.0.1:19888", } httpError := make(chan error) go func() { diff --git a/internal/storage/FileServing.go b/internal/storage/FileServing.go index 64ce9f70..248ba963 100644 --- a/internal/storage/FileServing.go +++ b/internal/storage/FileServing.go @@ -488,6 +488,8 @@ func generateHashAndEncrypt(fileContent io.Reader, fileHeader *multipart.FileHea helper.Check(err) err = encryption.Encrypt(&encInfo, tempFile, tempFileEnc) helper.Check(err) + err = tempFile.Close() + helper.Check(err) err = os.Remove(tempFile.Name()) helper.Check(err) hash.Write([]byte(configuration.Get().Authentication.SaltFiles)) diff --git a/internal/storage/FileServing_test.go b/internal/storage/FileServing_test.go index d8369d62..fa32e949 100644 --- a/internal/storage/FileServing_test.go +++ b/internal/storage/FileServing_test.go @@ -9,8 +9,8 @@ import ( "net/textproto" "os" "strings" + "sync" "testing" - "testing/synctest" "time" "github.com/forceu/gokapi/internal/configuration" @@ -18,6 +18,7 @@ import ( "github.com/forceu/gokapi/internal/configuration/database" "github.com/forceu/gokapi/internal/encryption" "github.com/forceu/gokapi/internal/helper" + "github.com/forceu/gokapi/internal/logging/serverstats" "github.com/forceu/gokapi/internal/models" "github.com/forceu/gokapi/internal/storage/chunking" "github.com/forceu/gokapi/internal/storage/filesystem/s3filesystem/aws" @@ -30,6 +31,13 @@ func TestMain(m *testing.M) { testconfiguration.Create(true) configuration.Load() configuration.ConnectDatabase() + // Initialise the traffic-stats save timer before any test calls ServeFile, so that + // no test's background AddTraffic call sees a zero-value LastUpdate (which reads as + // "5+ minutes since the last save") and triggers a real, unawaited SQLite write. Left + // uninitialised, a write can still be in flight when TestParallelDownloads starts, + // where it can collide with the mutex-serialized IncreaseDownloadCount writes and + // livelock SQLite's busy-handler retry loop under synctest's deterministic fake clock. + serverstats.Init() var testserver *httptest.Server if testconfiguration.UseMockS3Server() { testserver = testconfiguration.StartS3TestServer() @@ -918,38 +926,45 @@ func TestParallelDownloads(t *testing.T) { } database.SaveMetaData(singleDownloadFile) - synctest.Test(t, func(t *testing.T) { - const workers = 50 - results := make(chan bool, workers) - - for i := 0; i < workers; i++ { - go func() { - w := httptest.NewRecorder() - r := httptest.NewRequest("GET", "/"+singleDownloadFile.Id, nil) - - // The mutex inside ServeFile should serialize the decrement logic. - success := ServeFile(singleDownloadFile, w, r, false, true, false, true) - results <- success - }() - } - - synctest.Wait() - close(results) - - var successCount int - var failureCount int + const workers = 50 + results := make(chan bool, workers) + var wg sync.WaitGroup + + for i := 0; i < workers; i++ { + wg.Add(1) + go func() { + defer wg.Done() + w := httptest.NewRecorder() + r := httptest.NewRequest("GET", "/"+singleDownloadFile.Id, nil) + + // The mutex inside ServeFile should serialize the decrement logic. + success := ServeFile(singleDownloadFile, w, r, false, true, false, true) + results <- success + }() + } - for res := range results { - if res { - successCount++ - } else { - failureCount++ - } + // A plain WaitGroup is used here rather than testing/synctest: this test spawns + // real goroutines that hit the real SQLite database, and synctest's deterministic + // fake clock has no way to gracefully coexist with unpredictable real-world I/O + // delays (e.g. antivirus briefly locking the database file on write) - a single + // external stall can turn into an apparent livelock, since retries governed by the + // fake clock can spin far faster than the real delay actually resolves. + wg.Wait() + close(results) + + var successCount int + var failureCount int + + for res := range results { + if res { + successCount++ + } else { + failureCount++ } + } - test.IsEqualInt(t, successCount, allowedDownloads) - test.IsEqualInt(t, failureCount, workers-allowedDownloads) - }) + test.IsEqualInt(t, successCount, allowedDownloads) + test.IsEqualInt(t, failureCount, workers-allowedDownloads) } func TestServeFilesAsZipSanitisation(t *testing.T) { diff --git a/internal/storage/chunking/Chunking_test.go b/internal/storage/chunking/Chunking_test.go index 1ac183ab..1b9fa54b 100644 --- a/internal/storage/chunking/Chunking_test.go +++ b/internal/storage/chunking/Chunking_test.go @@ -8,6 +8,7 @@ import ( "net/textproto" "net/url" "os" + "runtime" "strings" "testing" @@ -271,10 +272,17 @@ func TestGetFileByChunkId(t *testing.T) { file, err := GetFileByChunkId("testchunk") test.IsEqualString(t, file.Name(), "test/data/chunk-testchunk") test.IsNil(t, err) - err = os.Chmod("test/data/chunk-testchunk", 0222) - _, err = GetFileByChunkId("testchunk") - test.IsNotNil(t, err) - err = os.Remove(file.Name()) + err = file.Close() + test.IsNil(t, err) + if runtime.GOOS != "windows" { + // Windows has no POSIX permission bits, so chmod cannot simulate an + // unreadable file there the way it can on Unix-like systems. + err = os.Chmod("test/data/chunk-testchunk", 0222) + test.IsNil(t, err) + _, err = GetFileByChunkId("testchunk") + test.IsNotNil(t, err) + } + err = os.Remove("test/data/chunk-testchunk") test.IsNil(t, err) } diff --git a/internal/storage/filesystem/localstorage/Localstorage_test.go b/internal/storage/filesystem/localstorage/Localstorage_test.go index abe30074..cd203fd0 100644 --- a/internal/storage/filesystem/localstorage/Localstorage_test.go +++ b/internal/storage/filesystem/localstorage/Localstorage_test.go @@ -42,13 +42,13 @@ func TestLocalStorageDriver_Init(t *testing.T) { FilePrefix: "tpref", }) test.IsEqualBool(t, ok, true) - test.IsEqualString(t, driver.getPath(), "test/") + test.IsEqualString(t, driver.getPath(), "test"+string(os.PathSeparator)) ok = driver.Init(Config{ - DataPath: "test2/", + DataPath: "test2" + string(os.PathSeparator), FilePrefix: "", }) test.IsEqualBool(t, ok, true) - test.IsEqualString(t, driver.getPath(), "test2/") + test.IsEqualString(t, driver.getPath(), "test2"+string(os.PathSeparator)) defer test.ExpectPanic(t) driver.Init(struct { invalid string @@ -72,7 +72,7 @@ func TestLocalStorageDriver_IsAvailable(t *testing.T) { func TestGetDataPath(t *testing.T) { driver := getTestDriver(t) initDriver(t, driver) - test.IsEqualString(t, driver.getPath(), "test/data/") + test.IsEqualString(t, driver.getPath(), "test/data"+string(os.PathSeparator)) driver.dataPath = "" defer test.ExpectPanic(t) driver.getPath() diff --git a/internal/storage/filesystem/s3filesystem/aws/Aws_test.go b/internal/storage/filesystem/s3filesystem/aws/Aws_test.go index 96514890..d10c523b 100644 --- a/internal/storage/filesystem/s3filesystem/aws/Aws_test.go +++ b/internal/storage/filesystem/s3filesystem/aws/Aws_test.go @@ -81,6 +81,7 @@ func TestUploadToAws(t *testing.T) { location, err := Upload(file, testFile) test.IsNil(t, err) test.IsNotEmpty(t, location) + file.Close() os.Remove("test") } @@ -93,6 +94,7 @@ func TestDownloadFromAws(t *testing.T) { test.FileExists(t, "test") content, _ := os.ReadFile("test") test.IsEqualString(t, string(content), "testfile-content") + file.Close() os.Remove("test") } diff --git a/internal/webserver/ssl/Ssl.go b/internal/webserver/ssl/Ssl.go index 5a4ca343..e1a85950 100644 --- a/internal/webserver/ssl/Ssl.go +++ b/internal/webserver/ssl/Ssl.go @@ -65,6 +65,7 @@ func getDaysRemaining() int { certificate, _ := GetCertificateLocations() file, err := os.Open(certificate) helper.Check(err) + defer file.Close() certContent, err := io.ReadAll(file) helper.Check(err) pemContent, _ := pem.Decode(certContent)