From d22b1a2b097180b47de5bef579551df736cc6ede Mon Sep 17 00:00:00 2001 From: DualFroz Date: Fri, 2 Oct 2026 16:34:40 +0200 Subject: [PATCH] audio: run the tests waiting on real time on the fake clock of synctest TestGC, TestSameSourcePlayers, TestDeferredDeviceCreation, TestPauseAndStopReading, TestPauseAndStopReadingWhilePaused, TestPositionNotGrowingAfterFinished and TestPlayerErrorDoesNotStopOtherPlayers now run in a synctest bubble, so their sleeps and deadlines take no real time. A bubble must not leave goroutines behind, so ResetContextForTesting now stops the goroutine updating the players and the dummy players reading their sources, and waits for them to exit. Before, every test leaked them. Updates #3716 --- audio/audio.go | 28 ++- audio/audio_test.go | 505 ++++++++++++++++++++++--------------------- audio/export_test.go | 46 +++- 3 files changed, 322 insertions(+), 257 deletions(-) diff --git a/audio/audio.go b/audio/audio.go index 8f5e65e..ded88b2 100644 --- a/audio/audio.go +++ b/audio/audio.go @@ -73,9 +73,17 @@ type Context struct { playingPlayers map[*playerImpl]struct{} m sync.Mutex - // cond is signalled under m when a player is added to playingPlayers. The goroutine - // updating the players waits on it while playingPlayers is empty. + // cond is signalled under m when a player is added to playingPlayers or when the context is + // disposed. The goroutine updating the players waits on it while playingPlayers is empty. cond *sync.Cond + + // disposed reports that the goroutine updating the players must exit. A context lives until the + // process exits, so this is set only when a test discards the context. + disposed bool + + // updater tracks the goroutine updating the players, so that a test discarding the context can wait + // for the goroutine to exit. + updater sync.WaitGroup } var ( @@ -162,12 +170,14 @@ func NewContext(sampleRate int) *Context { // In the current Ebitengine implementation, update might not be called when the window is in the background (#3154). // In this case, an audio player's position is not updated correctly with AppendHookOnBeforeUpdateWithVMGuestInfo. // Use a distinct goroutine to update the player states. - go func() { + c.updater.Go(func() { for { // Block while there is no player to update, so that an idle context does not wake // up periodically. The sweep itself can remove the last player, so the check is // repeated after every sweep. - c.waitForPlayingPlayers() + if !c.waitForPlayingPlayers() { + return + } // A failure is local to the player which reported it. Record it and keep updating // the other players (#3647). if err := c.updatePlayers(); err != nil { @@ -175,7 +185,7 @@ func NewContext(sampleRate int) *Context { } time.Sleep(time.Second / 100) } - }() + }) return c } @@ -212,13 +222,15 @@ func (c *Context) setReady() { c.m.Unlock() } -// waitForPlayingPlayers blocks until at least one player is registered as playing. -func (c *Context) waitForPlayingPlayers() { +// waitForPlayingPlayers blocks until at least one player is registered as playing, and reports +// whether the players should be updated. It returns false once the context is disposed. +func (c *Context) waitForPlayingPlayers() bool { c.m.Lock() defer c.m.Unlock() - for len(c.playingPlayers) == 0 { + for len(c.playingPlayers) == 0 && !c.disposed { c.cond.Wait() } + return !c.disposed } func (c *Context) addPlayingPlayer(p *playerImpl) { diff --git a/audio/audio_test.go b/audio/audio_test.go index f0ab5c8..8526f91 100644 --- a/audio/audio_test.go +++ b/audio/audio_test.go @@ -24,6 +24,7 @@ import ( "sync" "sync/atomic" "testing" + "testing/synctest" "time" "github.com/hajimehoshi/ebiten/v2/audio" @@ -42,38 +43,40 @@ func teardown() { // Issue #746 func TestGC(t *testing.T) { - setup() - defer teardown() + synctest.Test(t, func(t *testing.T) { + setup() + defer teardown() - p, _ := context.NewPlayer(bytes.NewReader(make([]byte, 4))) - got := audio.PlayersCountForTesting() - if want := 0; got != want { - t.Errorf("PlayersCountForTesting(): got: %d, want: %d", got, want) - } + p, _ := context.NewPlayer(bytes.NewReader(make([]byte, 4))) + got := audio.PlayersCountForTesting() + if want := 0; got != want { + t.Errorf("PlayersCountForTesting(): got: %d, want: %d", got, want) + } - p.Play() - got = audio.PlayersCountForTesting() - if want := 1; got != want { - t.Errorf("PlayersCountForTesting() after Play: got: %d, want: %d", got, want) - } - - runtime.KeepAlive(p) - p = nil - runtime.GC() - - for range 10 { + p.Play() got = audio.PlayersCountForTesting() - if want := 0; got == want { - return + if want := 1; got != want { + t.Errorf("PlayersCountForTesting() after Play: got: %d, want: %d", got, want) } - if err := audio.UpdateForTesting(); err != nil { - t.Error(err) + + runtime.KeepAlive(p) + p = nil + runtime.GC() + + for range 10 { + got = audio.PlayersCountForTesting() + if want := 0; got == want { + return + } + if err := audio.UpdateForTesting(); err != nil { + t.Error(err) + } + // 200[ms] should be enough for all the bytes to be consumed. The test runs on the fake + // clock of synctest, so this does not take real time. + time.Sleep(200 * time.Millisecond) } - // 200[ms] should be enough for all the bytes to be consumed. - // TODO: This is a dirty hack. Would it be possible to use virtual time? - time.Sleep(200 * time.Millisecond) - } - t.Errorf("time out") + t.Errorf("time out") + }) } type infiniteReader struct{} @@ -87,35 +90,37 @@ func (i *infiniteReader) Read(p []byte) (int, error) { // Issue #853 func TestSameSourcePlayers(t *testing.T) { - setup() - defer teardown() + synctest.Test(t, func(t *testing.T) { + setup() + defer teardown() - src := &infiniteReader{} - p0, err := context.NewPlayer(src) - if err != nil { - t.Fatal(err) - } - p1, err := context.NewPlayer(src) - if err != nil { - t.Fatal(err) - } - - // As the player does not play yet, error doesn't happen. - if err := audio.UpdateForTesting(); err != nil { - t.Error(err) - } - - p0.Play() - p1.Play() - - for range 10 { - if err := audio.UpdateForTesting(); err != nil { - // An error is expected. - return + src := &infiniteReader{} + p0, err := context.NewPlayer(src) + if err != nil { + t.Fatal(err) } - time.Sleep(200 * time.Millisecond) - } - t.Errorf("time out") + p1, err := context.NewPlayer(src) + if err != nil { + t.Fatal(err) + } + + // As the player does not play yet, error doesn't happen. + if err := audio.UpdateForTesting(); err != nil { + t.Error(err) + } + + p0.Play() + p1.Play() + + for range 10 { + if err := audio.UpdateForTesting(); err != nil { + // An error is expected. + return + } + time.Sleep(200 * time.Millisecond) + } + t.Errorf("time out") + }) } func TestPauseBeforeInit(t *testing.T) { @@ -166,53 +171,55 @@ func TestNonSeekableSource(t *testing.T) { // Issue #3438 func TestDeferredDeviceCreation(t *testing.T) { - setup() - defer teardown() + synctest.Test(t, func(t *testing.T) { + setup() + defer teardown() - p, err := context.NewPlayer(bytes.NewReader(make([]byte, 4))) - if err != nil { - t.Fatal(err) - } - - // Touching a player before the first update must not create the audio device. - p.SetVolume(0.5) - p.Play() - - if audio.ContextCreatedForTesting() { - t.Errorf("the audio device must not be created before the first update") - } - - // The state set before the device is created is recorded. - if got, want := p.Volume(), 0.5; got != want { - t.Errorf("Volume(): got: %v, want: %v", got, want) - } - if got, want := p.IsPlaying(), true; got != want { - t.Errorf("IsPlaying(): got: %t, want: %t", got, want) - } - if got, want := audio.PlayersCountForTesting(), 1; got != want { - t.Errorf("PlayersCountForTesting(): got: %d, want: %d", got, want) - } - - // The first update creates the audio device. - if err := audio.UpdateForTesting(); err != nil { - t.Error(err) - } - if !audio.ContextCreatedForTesting() { - t.Errorf("the audio device must be created after the first update") - } - - // The pending player eventually starts playing and finishes, which is only possible - // once the device is created and the player is materialized. - for range 10 { - if audio.PlayersCountForTesting() == 0 { - return + p, err := context.NewPlayer(bytes.NewReader(make([]byte, 4))) + if err != nil { + t.Fatal(err) } + + // Touching a player before the first update must not create the audio device. + p.SetVolume(0.5) + p.Play() + + if audio.ContextCreatedForTesting() { + t.Errorf("the audio device must not be created before the first update") + } + + // The state set before the device is created is recorded. + if got, want := p.Volume(), 0.5; got != want { + t.Errorf("Volume(): got: %v, want: %v", got, want) + } + if got, want := p.IsPlaying(), true; got != want { + t.Errorf("IsPlaying(): got: %t, want: %t", got, want) + } + if got, want := audio.PlayersCountForTesting(), 1; got != want { + t.Errorf("PlayersCountForTesting(): got: %d, want: %d", got, want) + } + + // The first update creates the audio device. if err := audio.UpdateForTesting(); err != nil { t.Error(err) } - time.Sleep(200 * time.Millisecond) - } - t.Errorf("time out") + if !audio.ContextCreatedForTesting() { + t.Errorf("the audio device must be created after the first update") + } + + // The pending player eventually starts playing and finishes, which is only possible + // once the device is created and the player is materialized. + for range 10 { + if audio.PlayersCountForTesting() == 0 { + return + } + if err := audio.UpdateForTesting(); err != nil { + t.Error(err) + } + time.Sleep(200 * time.Millisecond) + } + t.Errorf("time out") + }) } // Issue #3438 @@ -450,43 +457,45 @@ func TestPauseAndStopReading(t *testing.T) { t.Skip("infinite streams in tests cannot be treated well on browsers") } - setup() - defer teardown() + synctest.Test(t, func(t *testing.T) { + setup() + defer teardown() - src := &countingSource{} - p, err := context.NewPlayerF32(src) - if err != nil { - t.Fatal(err) - } + src := &countingSource{} + p, err := context.NewPlayerF32(src) + if err != nil { + t.Fatal(err) + } - p.Play() + p.Play() - // The first update creates the audio device and starts the pending player. - if err := audio.UpdateForTesting(); err != nil { - t.Error(err) - } + // The first update creates the audio device and starts the pending player. + if err := audio.UpdateForTesting(); err != nil { + t.Error(err) + } - waitForRead(t, src, 0) + waitForRead(t, src, 0) - p.PauseAndStopReading() + p.PauseAndStopReading() - if p.IsPlaying() { - t.Error("IsPlaying() after PauseAndStopReading: got: true, want: false") - } - if got, want := audio.PlayersCountForTesting(), 0; got != want { - t.Errorf("PlayersCountForTesting() after PauseAndStopReading: got: %d, want: %d", got, want) - } + if p.IsPlaying() { + t.Error("IsPlaying() after PauseAndStopReading: got: true, want: false") + } + if got, want := audio.PlayersCountForTesting(), 0; got != want { + t.Errorf("PlayersCountForTesting() after PauseAndStopReading: got: %d, want: %d", got, want) + } - // After PauseAndStopReading, the source must not be read. - reads := src.reads.Load() - time.Sleep(100 * time.Millisecond) - if got := src.reads.Load(); got != reads { - t.Errorf("the source was read after PauseAndStopReading: got: %d reads, want: %d", got, reads) - } + // After PauseAndStopReading, the source must not be read. + reads := src.reads.Load() + time.Sleep(100 * time.Millisecond) + if got := src.reads.Load(); got != reads { + t.Errorf("the source was read after PauseAndStopReading: got: %d reads, want: %d", got, reads) + } - // The player is reusable after PauseAndStopReading. - p.Play() - waitForRead(t, src, reads) + // The player is reusable after PauseAndStopReading. + p.Play() + waitForRead(t, src, reads) + }) } // Issue #3510 @@ -495,35 +504,37 @@ func TestPauseAndStopReadingWhilePaused(t *testing.T) { t.Skip("infinite streams in tests cannot be treated well on browsers") } - setup() - defer teardown() + synctest.Test(t, func(t *testing.T) { + setup() + defer teardown() - src := &countingSource{} - p, err := context.NewPlayerF32(src) - if err != nil { - t.Fatal(err) - } + src := &countingSource{} + p, err := context.NewPlayerF32(src) + if err != nil { + t.Fatal(err) + } - p.Play() + p.Play() - // The first update creates the audio device and starts the pending player. - if err := audio.UpdateForTesting(); err != nil { - t.Error(err) - } + // The first update creates the audio device and starts the pending player. + if err := audio.UpdateForTesting(); err != nil { + t.Error(err) + } - reads := waitForRead(t, src, 0) + reads := waitForRead(t, src, 0) - // A paused player keeps reading the source to fill its buffer. - p.Pause() - waitForRead(t, src, reads) + // A paused player keeps reading the source to fill its buffer. + p.Pause() + waitForRead(t, src, reads) - // PauseAndStopReading must stop the reads even though the player is already paused. - p.PauseAndStopReading() - reads = src.reads.Load() - time.Sleep(100 * time.Millisecond) - if got := src.reads.Load(); got != reads { - t.Errorf("the source was read after PauseAndStopReading: got: %d reads, want: %d", got, reads) - } + // PauseAndStopReading must stop the reads even though the player is already paused. + p.PauseAndStopReading() + reads = src.reads.Load() + time.Sleep(100 * time.Millisecond) + if got := src.reads.Load(); got != reads { + t.Errorf("the source was read after PauseAndStopReading: got: %d reads, want: %d", got, reads) + } + }) } // Issue #3510 @@ -635,51 +646,53 @@ func TestSetBufferSize(t *testing.T) { } func TestPositionNotGrowingAfterFinished(t *testing.T) { - setup() - defer teardown() + synctest.Test(t, func(t *testing.T) { + setup() + defer teardown() - // 44100 [Hz] * 8 [bytes/sample] is one second of 32bit float stereo audio. - src := bytes.NewReader(make([]byte, 44100*8*4)) - p, err := context.NewPlayerF32(src) - if err != nil { - t.Fatal(err) - } - - p.Play() - // Wait until the player finishes its source. The deadline is generous because time.Sleep has - // a several-millisecond floor on browsers, which slows draining the source down. - var finished bool - deadline := time.Now().Add(10 * time.Second) - for time.Now().Before(deadline) { - if err := audio.UpdateForTesting(); err != nil { + // 44100 [Hz] * 8 [bytes/sample] is one second of 32bit float stereo audio. + src := bytes.NewReader(make([]byte, 44100*8*4)) + p, err := context.NewPlayerF32(src) + if err != nil { t.Fatal(err) } - if !p.IsPlaying() { - finished = true - break - } - time.Sleep(5 * time.Millisecond) - } - if !finished { - t.Fatal("time out: the player did not finish") - } - end := p.Position() - if got, want := end, 4*time.Second; got < want { - t.Errorf("Position() after the player finished: got: %v, want: at least %v", got, want) - } - - // Play on a finished player does nothing, so the position must not grow anymore. - for range 20 { p.Play() - if err := audio.UpdateForTesting(); err != nil { - t.Fatal(err) + // Wait until the player finishes its source. The test runs on the fake clock of synctest, so + // the deadline does not take real time. + var finished bool + deadline := time.Now().Add(10 * time.Second) + for time.Now().Before(deadline) { + if err := audio.UpdateForTesting(); err != nil { + t.Fatal(err) + } + if !p.IsPlaying() { + finished = true + break + } + time.Sleep(5 * time.Millisecond) } - time.Sleep(5 * time.Millisecond) - } - if got := p.Position(); got != end { - t.Errorf("position grew after the player finished: %v -> %v", end, got) - } + if !finished { + t.Fatal("time out: the player did not finish") + } + + end := p.Position() + if got, want := end, 4*time.Second; got < want { + t.Errorf("Position() after the player finished: got: %v, want: at least %v", got, want) + } + + // Play on a finished player does nothing, so the position must not grow anymore. + for range 20 { + p.Play() + if err := audio.UpdateForTesting(); err != nil { + t.Fatal(err) + } + time.Sleep(5 * time.Millisecond) + } + if got := p.Position(); got != end { + t.Errorf("position grew after the player finished: %v -> %v", end, got) + } + }) } func TestNewPlayerWithSourceOfClosedPlayer(t *testing.T) { @@ -797,70 +810,72 @@ func TestPlayerErrorDoesNotStopOtherPlayers(t *testing.T) { t.Skip("infinite streams in tests cannot be treated well on browsers") } - setup() - defer teardown() + synctest.Test(t, func(t *testing.T) { + setup() + defer teardown() - // The first update creates the audio device, so that the players below play for real. - if err := audio.UpdateForTesting(); err != nil { - t.Fatal(err) - } - - failing, err := context.NewPlayerF32(&failingSource{remaining: 4096 * 4}) - if err != nil { - t.Fatal(err) - } - // 44100 [Hz] * 8 [bytes/sample] is one second of 32bit float stereo audio. - healthy, err := context.NewPlayerF32(audio.NewInfiniteLoopF32(bytes.NewReader(make([]byte, 44100*8)), 44100*8)) - if err != nil { - t.Fatal(err) - } - - failing.Play() - healthy.Play() - - // Wait until the failing player's source fails and the failure is reported as the context - // error. - var failed bool - deadline := time.Now().Add(5 * time.Second) - for time.Now().Before(deadline) { + // The first update creates the audio device, so that the players below play for real. if err := audio.UpdateForTesting(); err != nil { - if !errors.Is(err, errSourceFailed) { - t.Fatal(err) - } - failed = true - break + t.Fatal(err) } - time.Sleep(5 * time.Millisecond) - } - if !failed { - t.Fatal("time out: the failing player's error was not reported") - } - // The failed player is no longer tracked as playing, so that its error is reported once and - // the healthy player is the only playing player left. - deadline = time.Now().Add(time.Second) - for audio.PlayersCountForTesting() != 1 && time.Now().Before(deadline) { - time.Sleep(5 * time.Millisecond) - } - if got, want := audio.PlayersCountForTesting(), 1; got != want { - t.Errorf("PlayersCountForTesting() after a player failed: got: %d, want: %d", got, want) - } + failing, err := context.NewPlayerF32(&failingSource{remaining: 4096 * 4}) + if err != nil { + t.Fatal(err) + } + // 44100 [Hz] * 8 [bytes/sample] is one second of 32bit float stereo audio. + healthy, err := context.NewPlayerF32(audio.NewInfiniteLoopF32(bytes.NewReader(make([]byte, 44100*8)), 44100*8)) + if err != nil { + t.Fatal(err) + } - // The healthy player must keep playing and its position must keep advancing: one player's - // failure must not stop the context from updating the other players. - if !healthy.IsPlaying() { - t.Error("IsPlaying() of the healthy player after another player failed: got: false, want: true") - } - pos0 := healthy.Position() - time.Sleep(100 * time.Millisecond) - if pos1 := healthy.Position(); pos1 <= pos0 { - t.Errorf("Position() of the healthy player did not advance after another player failed: %v -> %v", pos0, pos1) - } + failing.Play() + healthy.Play() - // The context error is sticky. - if err := audio.UpdateForTesting(); !errors.Is(err, errSourceFailed) { - t.Errorf("UpdateForTesting() after a player failed: got: %v, want: %v", err, errSourceFailed) - } + // Wait until the failing player's source fails and the failure is reported as the context + // error. + var failed bool + deadline := time.Now().Add(5 * time.Second) + for time.Now().Before(deadline) { + if err := audio.UpdateForTesting(); err != nil { + if !errors.Is(err, errSourceFailed) { + t.Fatal(err) + } + failed = true + break + } + time.Sleep(5 * time.Millisecond) + } + if !failed { + t.Fatal("time out: the failing player's error was not reported") + } + + // The failed player is no longer tracked as playing, so that its error is reported once and + // the healthy player is the only playing player left. + deadline = time.Now().Add(time.Second) + for audio.PlayersCountForTesting() != 1 && time.Now().Before(deadline) { + time.Sleep(5 * time.Millisecond) + } + if got, want := audio.PlayersCountForTesting(), 1; got != want { + t.Errorf("PlayersCountForTesting() after a player failed: got: %d, want: %d", got, want) + } + + // The healthy player must keep playing and its position must keep advancing: one player's + // failure must not stop the context from updating the other players. + if !healthy.IsPlaying() { + t.Error("IsPlaying() of the healthy player after another player failed: got: false, want: true") + } + pos0 := healthy.Position() + time.Sleep(100 * time.Millisecond) + if pos1 := healthy.Position(); pos1 <= pos0 { + t.Errorf("Position() of the healthy player did not advance after another player failed: %v -> %v", pos0, pos1) + } + + // The context error is sticky. + if err := audio.UpdateForTesting(); !errors.Is(err, errSourceFailed) { + t.Errorf("UpdateForTesting() after a player failed: got: %v, want: %v", err, errSourceFailed) + } + }) } func TestPlayOnIdleContext(t *testing.T) { diff --git a/audio/export_test.go b/audio/export_test.go index 654df73..e2638ee 100644 --- a/audio/export_test.go +++ b/audio/export_test.go @@ -27,6 +27,10 @@ type ( suspendErr error resumeErr error + // players are the players created since the context was reset last. ResetContextForTesting + // stops them, so that a player reading an infinite source does not keep reading after its test. + players []*dummyPlayer + mu sync.Mutex } dummyPlayer struct { @@ -45,6 +49,9 @@ type ( // readGen is incremented by PauseAndStopReading to stop the goroutine reading r. readGen int + // readers tracks the goroutines reading r, so that a test can wait for them to stop. + readers sync.WaitGroup + // err is the first non-EOF error the source returned. Like the real players, a player // whose source failed stops for good and reports the error from Err. err error @@ -54,10 +61,27 @@ type ( ) func (c *dummyContext) NewPlayer(r io.Reader) player { - return &dummyPlayer{ + p := &dummyPlayer{ r: r, volume: 1, } + c.mu.Lock() + defer c.mu.Unlock() + c.players = append(c.players, p) + return p +} + +// stopPlayers stops the players' goroutines reading their sources, and waits for them to exit. +func (c *dummyContext) stopPlayers() { + c.mu.Lock() + players := c.players + c.players = nil + c.mu.Unlock() + + for _, p := range players { + p.PauseAndStopReading() + p.readers.Wait() + } } func (c *dummyContext) MaxBufferSize() int { @@ -95,7 +119,7 @@ func (p *dummyPlayer) Play() { } p.playing = true gen := p.readGen - go func() { + p.readers.Go(func() { var buf [4096]byte for { stopped, err := p.readOnce(gen, buf[:]) @@ -114,7 +138,7 @@ func (p *dummyPlayer) Play() { if p.playing { p.eof = true } - }() + }) } // readOnce performs one read from the source with the mutex held, so that PauseAndStopReading waits @@ -324,10 +348,24 @@ func ContextCreatedForTesting() bool { return c.playerFactory.currentContext() != nil } +// ResetContextForTesting discards the current context, so that a test can create a new one. +// ResetContextForTesting waits for the goroutines the context and its players started to exit, so +// that they do not outlive the test. func ResetContextForTesting() { theContextLock.Lock() - defer theContextLock.Unlock() + c := theContext theContext = nil + theContextLock.Unlock() + + if c != nil { + c.m.Lock() + c.disposed = true + c.cond.Broadcast() + c.m.Unlock() + c.updater.Wait() + } + + dummyContextForTesting.stopPlayers() } func (i *InfiniteLoop) SetNoBlendForTesting(value bool) {