diff --git a/.golangci.yml b/.golangci.yml index 979eafdcf5bd9..f14862b9e77a9 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -156,6 +156,10 @@ linters: google.golang.org/protobuf/protoadapt instead, or, better yet, see if you can move the code generation for that message to the modern protoc-gen-go. + synctest: + deny: + - pkg: testing/synctest + desc: 'use "github.com/gravitational/teleport/lib/utils/testutils/synctest" instead' test_packages: files: - '!$test' diff --git a/lib/inventory/controller_test.go b/lib/inventory/controller_test.go index 2c855ab670865..c6cd4b597bd23 100644 --- a/lib/inventory/controller_test.go +++ b/lib/inventory/controller_test.go @@ -40,6 +40,7 @@ import ( "github.com/gravitational/teleport/lib/inventory/metadata" usagereporter "github.com/gravitational/teleport/lib/usagereporter/teleport" "github.com/gravitational/teleport/lib/utils" + "github.com/gravitational/teleport/lib/utils/testutils/synctest" ) func TestMain(m *testing.M) { @@ -176,7 +177,7 @@ func (a *fakeAuth) UpsertInstance(ctx context.Context, instance types.Instance) // an ssh service. func TestSSHServerBasics(t *testing.T) { t.Parallel() - maybeSynctest(t, testSSHServerBasics) + synctest.Test(t, testSSHServerBasics) } func testSSHServerBasics(t *testing.T) { const serverID = "test-server" @@ -383,7 +384,7 @@ func testSSHServerBasics(t *testing.T) { // an app service. func TestAppServerBasics(t *testing.T) { t.Parallel() - maybeSynctest(t, testAppServerBasics) + synctest.Test(t, testAppServerBasics) } func testAppServerBasics(t *testing.T) { const serverID = "test-server" @@ -608,7 +609,7 @@ func testAppServerBasics(t *testing.T) { // a database server. func TestDatabaseServerBasics(t *testing.T) { t.Parallel() - maybeSynctest(t, testDatabaseServerBasics) + synctest.Test(t, testDatabaseServerBasics) } func testDatabaseServerBasics(t *testing.T) { const serverID = "test-server" @@ -833,7 +834,7 @@ func testDatabaseServerBasics(t *testing.T) { // TestInstanceHeartbeat verifies basic expected behaviors for instance heartbeat. func TestInstanceHeartbeat_Disabled(t *testing.T) { t.Parallel() - maybeSynctest(t, testInstanceHeartbeat_Disabled) + synctest.Test(t, testInstanceHeartbeat_Disabled) } func testInstanceHeartbeat_Disabled(t *testing.T) { const serverID = "test-instance" @@ -885,7 +886,7 @@ func TestInstanceHeartbeatDisabledEnv(t *testing.T) { // TestInstanceHeartbeat verifies basic expected behaviors for instance heartbeat. func TestInstanceHeartbeat(t *testing.T) { t.Parallel() - maybeSynctest(t, testInstanceHeartbeat) + synctest.Test(t, testInstanceHeartbeat) } func testInstanceHeartbeat(t *testing.T) { const serverID = "test-instance" @@ -990,7 +991,7 @@ func testInstanceHeartbeat(t *testing.T) { // inventory control stream. func TestUpdateLabels(t *testing.T) { t.Parallel() - maybeSynctest(t, testUpdateLabels) + synctest.Test(t, testUpdateLabels) } func testUpdateLabels(t *testing.T) { const serverID = "test-instance" @@ -1063,7 +1064,7 @@ func testUpdateLabels(t *testing.T) { // inventory control stream. func TestAgentMetadata(t *testing.T) { t.Parallel() - maybeSynctest(t, testAgentMetadata) + synctest.Test(t, testAgentMetadata) } func testAgentMetadata(t *testing.T) { const serverID = "test-instance" @@ -1340,14 +1341,14 @@ func TestGoodbye(t *testing.T) { } t.Run(test.name, func(t *testing.T) { t.Parallel() - maybeSynctest(t, inner) + synctest.Test(t, inner) }) } } func TestKubernetesServerBasics(t *testing.T) { t.Parallel() - maybeSynctest(t, testKubernetesServerBasics) + synctest.Test(t, testKubernetesServerBasics) } func testKubernetesServerBasics(t *testing.T) { const serverID = "test-server" @@ -1575,7 +1576,7 @@ func testKubernetesServerBasics(t *testing.T) { func TestGetSender(t *testing.T) { t.Parallel() - maybeSynctest(t, testGetSender) + synctest.Test(t, testGetSender) } func testGetSender(t *testing.T) { controller := NewController( diff --git a/lib/player/player_experimental_test.go b/lib/player/player_experimental_test.go deleted file mode 100644 index 484aa04065be5..0000000000000 --- a/lib/player/player_experimental_test.go +++ /dev/null @@ -1,108 +0,0 @@ -//go:build go1.24 && enablesynctest - -/* - * Teleport - * Copyright (C) 2025 Gravitational, Inc. - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . - */ - -package player_test - -import ( - "testing" - "testing/synctest" - "time" - - "github.com/gravitational/teleport/lib/player" - "github.com/stretchr/testify/require" -) - -/* -This file uses the experimental testing/synctest package introduced with Go 1.24: - - https://go.dev/blog/synctest - -When editing this file, you should set GOEXPERIMENT=synctest for your editor/LSP -to ensure that the language server doesn't fail to recognize the package. - -This file is also protected by a build tag to ensure that `go test` doesn't fail -for users who haven't set the environment variable. -*/ - -// TestInterruptsDelay tests that the player responds to playback -// controls even when it is waiting to emit an event. -func TestInterruptsDelay(t *testing.T) { - synctest.Run(func() { - p, err := player.New(&player.Config{ - SessionID: "test-session", - Streamer: &simpleStreamer{count: 3, delay: 5000}, - }) - require.NoError(t, err) - - synctest.Wait() - - require.NoError(t, p.Play()) - t.Cleanup(func() { p.Close() }) - - synctest.Wait() // player is now waiting to emit event 0 - - // emulate the user seeking forward while the player is waiting.. - start := time.Now() - p.SetPos(10_001 * time.Millisecond) - - // expect event 0 and event 1 to be emitted right away - evt0 := <-p.C() - evt1 := <-p.C() - require.Equal(t, int64(0), evt0.GetIndex()) - require.Equal(t, int64(1), evt1.GetIndex()) - - // Time will advance automatically in the synctest bubble. - // This assertion checks that the player was unblocked by - // the SetPos call and not because enough time elapsed. - require.Zero(t, time.Since(start)) - - <-p.C() - - // the user seeked to 10.001 seconds, it should take 4.999 - // seconds to get the third event that arrives at second 15. - require.Equal(t, time.Since(start), 4999*time.Millisecond) - }) -} - -func TestSeekForward(t *testing.T) { - synctest.Run(func() { - p, err := player.New(&player.Config{ - SessionID: "test-session", - Streamer: &simpleStreamer{count: 1, delay: 6000}, - }) - require.NoError(t, err) - t.Cleanup(func() { p.Close() }) - require.NoError(t, p.Play()) - - start := time.Now() - - time.Sleep(100 * time.Millisecond) - p.SetPos(500 * time.Millisecond) - time.Sleep(100 * time.Millisecond) - p.SetPos(5900 * time.Millisecond) - - <-p.C() - - // It should take 300ms for the event to be emitted. - // Two 100ms sleeps (200ms), then a seek to 5.900 seconds which - // requires another 100ms to wait for the event at 6s. - require.Equal(t, time.Since(start), 300*time.Millisecond) - }) -} diff --git a/lib/player/player_test.go b/lib/player/player_test.go index 0ffd8417c13b6..73455995e7480 100644 --- a/lib/player/player_test.go +++ b/lib/player/player_test.go @@ -33,6 +33,7 @@ import ( "github.com/gravitational/teleport/lib/events" "github.com/gravitational/teleport/lib/player" "github.com/gravitational/teleport/lib/session" + "github.com/gravitational/teleport/lib/utils/testutils/synctest" ) func TestBasicStream(t *testing.T) { @@ -392,3 +393,69 @@ func (d *databaseStreamer) sendEvent(ctx context.Context, evts chan apievents.Au d.idx++ } } + +// TestInterruptsDelay tests that the player responds to playback +// controls even when it is waiting to emit an event. +func TestInterruptsDelay(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + p, err := player.New(&player.Config{ + SessionID: "test-session", + Streamer: &simpleStreamer{count: 3, delay: 5000}, + }) + require.NoError(t, err) + + synctest.Wait() + + require.NoError(t, p.Play()) + t.Cleanup(func() { p.Close() }) + + synctest.Wait() // player is now waiting to emit event 0 + + // emulate the user seeking forward while the player is waiting.. + start := time.Now() + p.SetPos(10_001 * time.Millisecond) + + // expect event 0 and event 1 to be emitted right away + evt0 := <-p.C() + evt1 := <-p.C() + require.Equal(t, int64(0), evt0.GetIndex()) + require.Equal(t, int64(1), evt1.GetIndex()) + + // Time will advance automatically in the synctest bubble. + // This assertion checks that the player was unblocked by + // the SetPos call and not because enough time elapsed. + require.Zero(t, time.Since(start)) + + <-p.C() + + // the user seeked to 10.001 seconds, it should take 4.999 + // seconds to get the third event that arrives at second 15. + require.Equal(t, 4999*time.Millisecond, time.Since(start)) + }) +} + +func TestSeekForward(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + p, err := player.New(&player.Config{ + SessionID: "test-session", + Streamer: &simpleStreamer{count: 1, delay: 6000}, + }) + require.NoError(t, err) + t.Cleanup(func() { p.Close() }) + require.NoError(t, p.Play()) + + start := time.Now() + + time.Sleep(100 * time.Millisecond) + p.SetPos(500 * time.Millisecond) + time.Sleep(100 * time.Millisecond) + p.SetPos(5900 * time.Millisecond) + + <-p.C() + + // It should take 300ms for the event to be emitted. + // Two 100ms sleeps (200ms), then a seek to 5.900 seconds which + // requires another 100ms to wait for the event at 6s. + require.Equal(t, 300*time.Millisecond, time.Since(start)) + }) +} diff --git a/lib/srv/desktop/audit_compactor_test.go b/lib/srv/desktop/audit_compactor_test.go index 74eff13729296..1f3d4d6e8e3ce 100644 --- a/lib/srv/desktop/audit_compactor_test.go +++ b/lib/srv/desktop/audit_compactor_test.go @@ -1,5 +1,3 @@ -//go:build go1.24 && enablesynctest - /* * Teleport * Copyright (C) 2025 Gravitational, Inc. @@ -25,13 +23,13 @@ import ( "math" "sync" "testing" - "testing/synctest" "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/gravitational/teleport/api/types/events" + "github.com/gravitational/teleport/lib/utils/testutils/synctest" ) func newReadEvent(path string, directory directoryID, offset uint64, length uint32) *events.DesktopSharedDirectoryRead { @@ -70,8 +68,8 @@ func TestAuditCompactor(t *testing.T) { t.Run("basic", func(t *testing.T) { auditEvents = auditEvents[:0] - ctx := t.Context() - synctest.Run(func() { + synctest.Test(t, func(t *testing.T) { + ctx := t.Context() // Read sequence A compactor.handleRead(ctx, newReadEvent("foo", 1, 0, 100)) compactor.handleRead(ctx, newReadEvent("foo", 1, 100, 100)) @@ -97,8 +95,8 @@ func TestAuditCompactor(t *testing.T) { t.Run("overflow", func(t *testing.T) { auditEvents = auditEvents[:0] - ctx := t.Context() - synctest.Run(func() { + synctest.Test(t, func(t *testing.T) { + ctx := t.Context() // Walk up to and beyond MaxUint32 compactor.handleRead(ctx, newReadEvent("foo", 1, 0, math.MaxUint32-1)) compactor.handleRead(ctx, newReadEvent("foo", 1, math.MaxUint32-1, 1)) @@ -114,8 +112,8 @@ func TestAuditCompactor(t *testing.T) { t.Run("zero-length-event", func(t *testing.T) { auditEvents = auditEvents[:0] - ctx := t.Context() - synctest.Run(func() { + synctest.Test(t, func(t *testing.T) { + ctx := t.Context() // Create two read events with zero length, but with differing // error codes. Neither should get compacted, as zero length // events are not eligible for compaction. @@ -138,9 +136,8 @@ func TestAuditCompactor(t *testing.T) { t.Run("complex", func(t *testing.T) { auditEvents = auditEvents[:0] - ctx := t.Context() - synctest.Run(func() { - + synctest.Test(t, func(t *testing.T) { + ctx := t.Context() // Three separate reads (with different lengths) of the same file // Read sequence A compactor.handleRead(ctx, newReadEvent("foo", 1, 0, 100)) @@ -172,8 +169,8 @@ func TestAuditCompactor(t *testing.T) { t.Run("expirations", func(t *testing.T) { auditEvents = auditEvents[:0] - ctx := t.Context() - synctest.Run(func() { + synctest.Test(t, func(t *testing.T) { + ctx := t.Context() // 2 sequential reads compactor.handleRead(ctx, newReadEvent("foo", 1, 0, 100)) compactor.handleRead(ctx, newReadEvent("foo", 1, 100, 100)) @@ -212,7 +209,7 @@ func TestAuditCompactor(t *testing.T) { // a single consolidated event eventsLock.Lock() require.Len(t, auditEvents, 1) - assert.Contains(t, auditEvents, newReadEvent("foo", 1, 200, uint32(length*uint32(count)))) + assert.Contains(t, auditEvents, newReadEvent("foo", 1, 200, length*uint32(count))) eventsLock.Unlock() }) @@ -221,8 +218,8 @@ func TestAuditCompactor(t *testing.T) { t.Run("mix-reads-writes", func(t *testing.T) { auditEvents = auditEvents[:0] - ctx := t.Context() - synctest.Run(func() { + synctest.Test(t, func(t *testing.T) { + ctx := t.Context() // 3 sequential reads compactor.handleRead(ctx, newReadEvent("foo", 1, 0, 100)) compactor.handleRead(ctx, newReadEvent("foo", 1, 100, 100)) @@ -241,8 +238,8 @@ func TestAuditCompactor(t *testing.T) { t.Run("mix-files-and-directories", func(t *testing.T) { auditEvents = auditEvents[:0] - ctx := t.Context() - synctest.Run(func() { + synctest.Test(t, func(t *testing.T) { + ctx := t.Context() // Identical offsets and lengths, but different path and/or directoryID compactor.handleRead(ctx, newReadEvent("foo", 1, 0, 100)) compactor.handleRead(ctx, newReadEvent("foo", 2, 0, 100)) @@ -263,8 +260,8 @@ func TestAuditCompactor(t *testing.T) { }) t.Run("racy-flush", func(t *testing.T) { - ctx := t.Context() - synctest.Run(func() { + synctest.Test(t, func(t *testing.T) { + ctx := t.Context() auditEvents := make(chan events.AuditEvent) compactor.emitFn = func(_ context.Context, ae events.AuditEvent) { auditEvents <- ae @@ -292,7 +289,7 @@ func TestAuditCompactor(t *testing.T) { newReadEvent("baz", 1, 0, 100), newReadEvent("caz", 1, 0, 100), } - for _ = range len(expectedEvents) { + for range len(expectedEvents) { assert.False(t, flushDone) assert.Contains(t, expectedEvents, <-auditEvents) synctest.Wait() diff --git a/lib/tbot/internal/heartbeat/service_test.go b/lib/tbot/internal/heartbeat/service_test.go index 272116ec554fc..787b9888b4b99 100644 --- a/lib/tbot/internal/heartbeat/service_test.go +++ b/lib/tbot/internal/heartbeat/service_test.go @@ -24,7 +24,6 @@ import ( "runtime" "strings" "testing" - "testing/synctest" "time" "github.com/google/go-cmp/cmp" @@ -40,6 +39,7 @@ import ( "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/lib/tbot/readyz" "github.com/gravitational/teleport/lib/utils" + "github.com/gravitational/teleport/lib/utils/testutils/synctest" ) type fakeHeartbeatSubmitter struct { @@ -56,7 +56,7 @@ func (f *fakeHeartbeatSubmitter) SubmitHeartbeat( func TestHeartbeatService(t *testing.T) { t.Parallel() - synctest.Run(func() { + synctest.Test(t, func(t *testing.T) { log := utils.NewSlogLoggerForTests() ctx, cancel := context.WithCancel(t.Context()) t.Cleanup(cancel) diff --git a/lib/inventory/inventory_synctest_exp_test.go b/lib/utils/testutils/synctest/synctest_goexperiment.go similarity index 64% rename from lib/inventory/inventory_synctest_exp_test.go rename to lib/utils/testutils/synctest/synctest_goexperiment.go index 7e4fd28befae5..29fb08431b2b9 100644 --- a/lib/inventory/inventory_synctest_exp_test.go +++ b/lib/utils/testutils/synctest/synctest_goexperiment.go @@ -1,3 +1,5 @@ +//go:build !go1.25 && goexperiment.synctest + // Teleport // Copyright (C) 2025 Gravitational, Inc. // @@ -14,22 +16,26 @@ // You should have received a copy of the GNU Affero General Public License // along with this program. If not, see . -//go:build !go1.25 && goexperiment.synctest - -package inventory +package synctest import ( "testing" - "testing/synctest" + "testing/synctest" //nolint:depguard // this package wraps synctest ) -// TODO(espadolini): DELETE IN v21 or after the oldest supported Teleport -// version is on go 1.25 - -func maybeSynctest(t *testing.T, fn func(*testing.T)) { +// Test runs the provided function in a synctest bubble if synctest is +// supported, otherwise skips the test. To support older versions of Go, the +// function might be called in a subtest of the provided test. +func Test(t *testing.T, f func(*testing.T)) { // in go 1.24 with the synctest experiment there's no integrated support for // t.Cleanup callbacks to run before exiting the bubble, but if we run // things in a subtest we get the same behavior, albeit with everything in a // subtest, which is ugly but functional - synctest.Run(func() { t.Run("goexperiment.synctest", fn) }) + synctest.Run(func() { t.Run("goexperiment.synctest", f) }) +} + +// Wait blocks until every goroutine in the bubble is durably blocked. See +// [synctest.Wait] for details. Wait will panic if called from outside a bubble. +func Wait() { + synctest.Wait() } diff --git a/lib/inventory/inventory_nosynctest_test.go b/lib/utils/testutils/synctest/synctest_nosynctest.go similarity index 57% rename from lib/inventory/inventory_nosynctest_test.go rename to lib/utils/testutils/synctest/synctest_nosynctest.go index 5d1b7632ab2e7..396df418d6f1c 100644 --- a/lib/inventory/inventory_nosynctest_test.go +++ b/lib/utils/testutils/synctest/synctest_nosynctest.go @@ -1,3 +1,5 @@ +//go:build !go1.25 && !goexperiment.synctest + // Teleport // Copyright (C) 2025 Gravitational, Inc. // @@ -14,15 +16,22 @@ // You should have received a copy of the GNU Affero General Public License // along with this program. If not, see . -//go:build !go1.25 && !goexperiment.synctest +package synctest -package inventory +import ( + "testing" +) -import "testing" - -// TODO(espadolini): DELETE IN v21 or after the oldest supported Teleport -// version is on go 1.25 +// Test runs the provided function in a synctest bubble if synctest is +// supported, otherwise skips the test. To support older versions of Go, the +// function might be called in a subtest of the provided test. +func Test(t *testing.T, f func(*testing.T)) { + t.Skip("this test requires synctest") +} -func maybeSynctest(t *testing.T, fn func(*testing.T)) { - fn(t) +// Wait blocks until every goroutine in the bubble is durably blocked. See +// [synctest.Wait] for details. Wait will panic if called from outside a bubble. +func Wait() { + // technically true! + panic("goroutine is not in a bubble") } diff --git a/lib/inventory/inventory_synctest_test.go b/lib/utils/testutils/synctest/synctest_release.go similarity index 56% rename from lib/inventory/inventory_synctest_test.go rename to lib/utils/testutils/synctest/synctest_release.go index 334ef2e963620..f0fe20f668f43 100644 --- a/lib/inventory/inventory_synctest_test.go +++ b/lib/utils/testutils/synctest/synctest_release.go @@ -1,3 +1,5 @@ +//go:build go1.25 + // Teleport // Copyright (C) 2025 Gravitational, Inc. // @@ -14,15 +16,23 @@ // You should have received a copy of the GNU Affero General Public License // along with this program. If not, see . -//go:build go1.25 - -package inventory +package synctest import ( "testing" - "testing/synctest" + "testing/synctest" //nolint:depguard // this package wraps synctest ) -func maybeSynctest(t *testing.T, fn func(*testing.T)) { - synctest.Test(t, fn) +// Test runs the provided function in a synctest bubble if synctest is +// supported, otherwise skips the test. To support older versions of Go, the +// function might be called in a subtest of the provided test. +func Test(t *testing.T, f func(*testing.T)) { + // this version of Go (1.25+) is recent enough + synctest.Test(t, f) +} + +// Wait blocks until every goroutine in the bubble is durably blocked. See +// [synctest.Wait] for details. Wait will panic if called from outside a bubble. +func Wait() { + synctest.Wait() }