fix some of the flaky tests

CHANGELOG.md

@@ -4,10 +4,10 @@
     _⚠️ Note that this could be a slight breaking change in case you are chaining PocketBase commands and relied on the previous `0` exit status for `Command.RunE` returned errors._
     _Or in other words, if you have `./pocketbase invalid && someothercommand` and previously relied that `someothercommand` will be always executed then this is no longer the case and you'll have to adjust it or replace `&&` with `;`._
 
-- Added helper `filesystem` methods:
+- Added new `filesystem` low-level helper methods:
     - `filesystem.NewWriter(key, opts)` to allow direct file create from an `io.Reader` value.
-    - `filesystem.OnNewWriter()` low-level hook to allow listening for new/to-be-creaded files _(app level equivalent hook is also available but not exposed for now to avoid introducing breaking changes)_.
-    - `filesystem.OnDelete()` low-level hook to allow listening for deleted files _(app level equivalent hook is also available but not exposed for now to avoid introducing breaking changes)_.
+    - `filesystem.OnNewWriter()` hook to allow listening for new/to-be-creaded files _(app level hook is not exposed for now to avoid introducing breaking changes)_.
+    - `filesystem.OnDelete()` hook to allow listening for deleted files _(app level hook is not exposed for now to avoid introducing breaking changes)_.
 
 - Added new `DELETE /api/logs` endpoint and UI control to delete all logs without changing the `maxDays` retention setting.
 

apis/cron_test.go

@@ -77,10 +77,13 @@ func TestCronsRun(t *testing.T) {
 		app.Cron().Add("test", "* * * * *", func() {
 			app.Store().Set("testJobCalls", cast.ToInt(app.Store().Get("testJobCalls"))+1)
 		})
+		app.Cron().Stop()
 	}
 
 	expectedCalls := func(expected int) func(t testing.TB, app *tests.TestApp, res *http.Response) {
 		return func(t testing.TB, app *tests.TestApp, res *http.Response) {
+			time.Sleep(50 * time.Millisecond)
+
 			total := cast.ToInt(app.Store().Get("testJobCalls"))
 			if total != expected {
 				t.Fatalf("Expected total testJobCalls %d, got %d", expected, total)
@@ -93,7 +96,6 @@ func TestCronsRun(t *testing.T) {
 			Name:            "unauthorized",
 			Method:          http.MethodPost,
 			URL:             "/api/crons/test",
-			Delay:           50 * time.Millisecond,
 			BeforeTestFunc:  beforeTestFunc,
 			AfterTestFunc:   expectedCalls(0),
 			ExpectedStatus:  401,
@@ -107,7 +109,6 @@ func TestCronsRun(t *testing.T) {
 			Headers: map[string]string{
 				"Authorization": "eyJhbGciOiJIUzI1NiJ9.eyJpZCI6IjRxMXhsY2xtZmxva3UzMyIsInR5cGUiOiJhdXRoIiwiY29sbGVjdGlvbklkIjoiX3BiX3VzZXJzX2F1dGhfIiwiZXhwIjoyNTI0NjA0NDYxLCJyZWZyZXNoYWJsZSI6dHJ1ZX0.ZT3F0Z3iM-xbGgSG3LEKiEzHrPHr8t8IuHLZGGNuxLo",
 			},
-			Delay:           50 * time.Millisecond,
 			BeforeTestFunc:  beforeTestFunc,
 			AfterTestFunc:   expectedCalls(0),
 			ExpectedStatus:  403,
@@ -121,7 +122,6 @@ func TestCronsRun(t *testing.T) {
 			Headers: map[string]string{
 				"Authorization": "eyJhbGciOiJIUzI1NiJ9.eyJpZCI6InN5d2JoZWNuaDQ2cmhtMCIsInR5cGUiOiJhdXRoIiwiY29sbGVjdGlvbklkIjoicGJjXzMxNDI2MzU4MjMiLCJleHAiOjI1MjQ2MDQ0NjEsInJlZnJlc2hhYmxlIjp0cnVlfQ.UXgO3j-0BumcugrFjbd7j0M4MQvbrLggLlcu_YNGjoY",
 			},
-			Delay:           50 * time.Millisecond,
 			BeforeTestFunc:  beforeTestFunc,
 			AfterTestFunc:   expectedCalls(0),
 			ExpectedStatus:  404,
@@ -135,7 +135,6 @@ func TestCronsRun(t *testing.T) {
 			Headers: map[string]string{
 				"Authorization": "eyJhbGciOiJIUzI1NiJ9.eyJpZCI6InN5d2JoZWNuaDQ2cmhtMCIsInR5cGUiOiJhdXRoIiwiY29sbGVjdGlvbklkIjoicGJjXzMxNDI2MzU4MjMiLCJleHAiOjI1MjQ2MDQ0NjEsInJlZnJlc2hhYmxlIjp0cnVlfQ.UXgO3j-0BumcugrFjbd7j0M4MQvbrLggLlcu_YNGjoY",
 			},
-			Delay:          50 * time.Millisecond,
 			BeforeTestFunc: beforeTestFunc,
 			AfterTestFunc:  expectedCalls(1),
 			ExpectedStatus: 204,

apis/middlewares_rate_limit_test.go

@@ -1,8 +1,10 @@
 package apis_test
 
 import (
+	"fmt"
 	"net/http/httptest"
 	"testing"
+	"testing/synctest"
 	"time"
 
 	"github.com/pocketbase/pocketbase/apis"
@@ -127,27 +129,29 @@ func TestDefaultRateLimitMiddleware(t *testing.T) {
 		{"/rate/guest", 0, true, 429},
 	}
 
-	for _, s := range scenarios {
-		t.Run(s.url, func(t *testing.T) {
+	synctest.Test(t, func(t *testing.T) {
+		for i, s := range scenarios {
+			prefix := fmt.Sprintf("[%s:%d] ", s.url, i+1)
+
 			rec := httptest.NewRecorder()
 			req := httptest.NewRequest("GET", s.url, nil)
 
 			if s.authenticated {
 				auth, err := app.FindAuthRecordByEmail("users", "test@example.com")
 				if err != nil {
-					t.Fatal(err)
+					t.Fatalf(prefix+"%v", err)
 				}
 
 				token, err := auth.NewAuthToken()
 				if err != nil {
-					t.Fatal(err)
+					t.Fatalf(prefix+"%v", err)
 				}
 
 				req.Header.Add("Authorization", token)
 			}
 
 			if s.wait > 0 {
-				time.Sleep(time.Duration(s.wait) * time.Millisecond)
+				synctest.Sleep(time.Duration(s.wait) * time.Millisecond)
 			}
 
 			mux.ServeHTTP(rec, req)
@@ -155,10 +159,10 @@ func TestDefaultRateLimitMiddleware(t *testing.T) {
 			result := rec.Result()
 
 			if result.StatusCode != s.expectedStatus {
-				t.Fatalf("Expected response status %d, got %d", s.expectedStatus, result.StatusCode)
+				t.Fatalf(prefix+"Expected response status %d, got %d", s.expectedStatus, result.StatusCode)
 			}
-		})
-	}
+		}
+	})
 }
 
 func TestDefaultRateLimitMiddlewareSkipChecks(t *testing.T) {

core/base_test.go

@@ -7,6 +7,7 @@ import (
 	"os"
 	"slices"
 	"testing"
+	"testing/synctest"
 	"time"
 
 	_ "unsafe"
@@ -302,31 +303,34 @@ func TestBaseAppLoggerWrites(t *testing.T) {
 	})
 
 	t.Run("test batch logs writes", func(t *testing.T) {
-		app.Settings().Logs.MaxDays = 1
+		synctest.Test(t, func(t *testing.T) {
+			app.Settings().Logs.MaxDays = 1
 
-		for i := 0; i < logsThreshold-1; i++ {
+			for i := 0; i < logsThreshold-1; i++ {
+				app.Logger().Error("test")
+			}
+
+			if total := totalLogs(app, t); total != 0 {
+				t.Fatalf("Expected no logs, got %d", total)
+			}
+
+			// should trigger batch write
 			app.Logger().Error("test")
-		}
 
-		if total := totalLogs(app, t); total != 0 {
-			t.Fatalf("Expected no logs, got %d", total)
-		}
+			// should be added for the next batch write
+			app.Logger().Error("test")
 
-		// should trigger batch write
-		app.Logger().Error("test")
+			if total := totalLogs(app, t); total != logsThreshold {
+				t.Fatalf("Expected %d logs, got %d", logsThreshold, total)
+			}
 
-		// should be added for the next batch write
-		app.Logger().Error("test")
+			// wait for 3 secs to check the timer trigger
+			synctest.Sleep(3000 * time.Millisecond)
 
-		if total := totalLogs(app, t); total != logsThreshold {
-			t.Fatalf("Expected %d logs, got %d", logsThreshold, total)
-		}
-
-		// wait for ~3 secs to check the timer trigger
-		time.Sleep(3200 * time.Millisecond)
-		if total := totalLogs(app, t); total != logsThreshold+1 {
-			t.Fatalf("Expected %d logs, got %d", logsThreshold+1, total)
-		}
+			if total := totalLogs(app, t); total != logsThreshold {
+				t.Fatalf("Expected %d logs, got %d", logsThreshold, total)
+			}
+		})
 	})
 }
 

core/settings_model.go

@@ -257,13 +257,6 @@ func (s *Settings) DBExport(app App) (map[string]any, error) {
 	}
 	result["updated"] = now
 
-	// @todo remove with encoding/json/2
-	// serialize as empty array
-	//nolint:staticcheck
-	if s.settings.SuperuserIPs == nil {
-		s.settings.SuperuserIPs = []string{}
-	}
-
 	encoded, err := json.Marshal(s.settings, json.Deterministic(true))
 	if err != nil {
 		return nil, err
@@ -355,12 +348,6 @@ func (s *Settings) MarshalJSON() ([]byte, error) {
 		}
 	}
 
-	// @todo remove with encoding/json/2
-	// serialize as empty array
-	if copy.SuperuserIPs == nil {
-		copy.SuperuserIPs = []string{}
-	}
-
 	return json.Marshal(copy, json.Deterministic(true))
 }
 

tests/api.go

@@ -249,6 +249,7 @@ func (scenario *ApiScenario) test(t testing.TB) {
 			t.Errorf("Expected status code %d, got %d", scenario.ExpectedStatus, res.StatusCode)
 		}
 
+		// @todo consider removing in favour of synctest.Wait()
 		if scenario.Delay > 0 {
 			time.Sleep(scenario.Delay)
 		}

tools/cron/cron_test.go

@@ -5,6 +5,7 @@ import (
 	"slices"
 	"sync"
 	"testing"
+	"testing/synctest"
 	"time"
 )
 
@@ -254,63 +255,65 @@ func TestCronJobs(t *testing.T) {
 func TestCronStartStop(t *testing.T) {
 	t.Parallel()
 
-	var mu sync.Mutex
+	synctest.Test(t, func(t *testing.T) {
+		var mu sync.Mutex
 
-	test1 := 0
-	test2 := 0
+		test1 := 0
+		test2 := 0
 
-	c := New()
+		c := New()
 
-	c.SetInterval(250 * time.Millisecond)
+		c.SetInterval(250 * time.Millisecond)
+
+		c.Add("test1", "* * * * *", func() {
+			mu.Lock()
+			defer mu.Unlock()
+			test1++
+		})
+
+		c.Add("test2", "* * * * *", func() {
+			mu.Lock()
+			defer mu.Unlock()
+			test2++
+		})
+
+		// call twice Start to check if the previous ticker will be reseted
+		c.Start()
+		c.Start()
+
+		synctest.Sleep(500 * time.Millisecond)
+
+		// call twice Stop to ensure that the second stop is no-op
+		c.Stop()
+		c.Stop()
+
+		expectedCalls := 2
 
-	c.Add("test1", "* * * * *", func() {
 		mu.Lock()
-		defer mu.Unlock()
-		test1++
-	})
+		if test1 != expectedCalls {
+			t.Fatalf("Expected %d test1, got %d", expectedCalls, test1)
+		}
+		if test2 != expectedCalls {
+			t.Fatalf("Expected %d test2, got %d", expectedCalls, test2)
+		}
+		mu.Unlock()
+
+		// resume for 1 seconds
+		c.Start()
+
+		synctest.Sleep(1000 * time.Millisecond)
+
+		c.Stop()
+
+		expectedCalls += 4
 
-	c.Add("test2", "* * * * *", func() {
 		mu.Lock()
-		defer mu.Unlock()
-		test2++
+		if test1 != expectedCalls {
+			t.Fatalf("Expected %d test1, got %d", expectedCalls, test1)
+		}
+		if test2 != expectedCalls {
+			t.Fatalf("Expected %d test2, got %d", expectedCalls, test2)
+		}
+		mu.Unlock()
 	})
-
-	// call twice Start to check if the previous ticker will be reseted
-	c.Start()
-	c.Start()
-
-	time.Sleep(505 * time.Millisecond) // slightly larger to minimize flakiness
-
-	// call twice Stop to ensure that the second stop is no-op
-	c.Stop()
-	c.Stop()
-
-	expectedCalls := 2
-
-	mu.Lock()
-	if test1 != expectedCalls {
-		t.Fatalf("Expected %d test1, got %d", expectedCalls, test1)
-	}
-	if test2 != expectedCalls {
-		t.Fatalf("Expected %d test2, got %d", expectedCalls, test2)
-	}
-	mu.Unlock()
-
-	// resume for 1 seconds
-	c.Start()
-
-	time.Sleep(1005 * time.Millisecond) // slightly larger to minimize flakiness
-
-	c.Stop()
-
-	expectedCalls += 4
-
-	mu.Lock()
-	if test1 != expectedCalls {
-		t.Fatalf("Expected %d test1, got %d", expectedCalls, test1)
-	}
-	if test2 != expectedCalls {
-		t.Fatalf("Expected %d test2, got %d", expectedCalls, test2)
-	}
-	mu.Unlock()
 }

tools/filesystem/filesystem.go

@@ -131,8 +131,8 @@ func (s *System) Close() error {
 	return s.bucket.Close()
 }
 
-// OnNewWriter is a low level hook that is triggered on every writer initialization
-// (aka. when attempting to create a new file with [system.NewWriter], [system.Upload], etc.).
+// OnNewWriter is a low level hook that is triggered on every new writer initialization
+// (aka. when attempting to create a new file with [system.NewWriter] or [system.Upload]).
 //
 // Note that currently it doesn't trigger on [System.Copy] but this may change in future releases.
 func (s *System) OnNewWriter() *hook.Hook[*NewWriterEvent] {
@@ -144,6 +144,9 @@ func (s *System) OnNewWriter() *hook.Hook[*NewWriterEvent] {
 }
 
 // OnDelete is a low level hook that is triggered on every [System.Delete] call.
+//
+// Note that the hook doesn't fire when a file is being overwritten
+// by a new one, because in that case [System.Delete] is not invoked.
 func (s *System) OnDelete() *hook.Hook[*DeleteEvent] {
 	if s.onDelete == nil {
 		s.onDelete = &hook.Hook[*DeleteEvent]{}