nt4f04uNd / sweyer

Music player built with Flutter
BSD 3-Clause "New" or "Revised" License
200 stars 47 forks source link

Investigate test flakiness #162

Open nt4f04uNd opened 1 month ago

nt4f04uNd commented 1 month ago

https://github.com/nt4f04uNd/sweyer/actions/runs/11412091655/job/31757501525?pr=161

Possible solution https://github.com/nt4f04uNd/sweyer/pull/161#issuecomment-2423351681

══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════
The following LateError was thrown running a test (but after the test had completed):
LateInitializationError: Field '_onQueueChangeSubject@38260717' has not been initialized.

When the exception was thrown, this was the stack:
#0      QueueControl._onQueueChangeSubject (package:sweyer/logic/player/queue.dart)
#1      QueueControl.onQueueChanged (package:sweyer/logic/player/queue.dart:378:38)
#2      AudioHandler._init (package:sweyer/logic/player/music_player.dart:220:45)
#3      MusicPlayer.init (package:sweyer/logic/player/music_player.dart:647:14)
<asynchronous suspension>
#4      ContentControl.init (package:sweyer/logic/player/content.dart:248:9)
<asynchronous suspension>
#5      _SongsEmptyScreenState._handleRefetch (package:sweyer/routes/home_route/home_route.dart:[186](https://github.com/nt4f04uNd/sweyer/actions/runs/11412091655/job/31757501525?pr=161#step:3:198):5)
<asynchronous suspension>
════════════════════════════════════════════════════════════════════════════════════════════════════
❌ /home/runner/work/sweyer/sweyer/test/routes/home_route_test.dart: no songs screen - shows when the library is empty and pressing the button performs refetching (failed after test completion)
❌ /home/runner/work/sweyer/sweyer/test/routes/home_route_test.dart: no songs screen - shows when the library is empty and pressing the button performs refetching (failed after test completion)
══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════
The following assertion was thrown running a test (but after the test had completed):
Current queue must not be empty
'package:sweyer/logic/player/queue.dart':
Failed assertion: line 257 pos 12: 'value.isNotEmpty'

When the exception was thrown, this was the stack:
#2      QueuesState.current (package:sweyer/logic/player/queue.dart:257:12)
#3      PlaybackControl.currentSongIndex (package:sweyer/logic/player/playback.dart:52:45)
#4      AudioHandler._setState (package:sweyer/logic/player/music_player.dart:616:44)
#5      AudioHandler._init.<anonymous closure> (package:sweyer/logic/player/music_player.dart:209:7)
#16     _StartWithStreamSink._safeAddFirstEvent (package:rxdart/src/transformers/start_with.dart:53:12)
#17     _StartWithStreamSink.onListen.<anonymous closure> (package:rxdart/src/transformers/start_with.dart:35:29)
(elided 20 frames from class _AssertionError, dart:async, and package:stack_trace)
════════════════════════════════════════════════════════════════════════════════════════════════════
❌ /home/runner/work/sweyer/sweyer/test/routes/home_route_test.dart: no songs screen - shows when the library is empty and pressing the button performs refetching (failed after test completion)
══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════
The following assertion was thrown running a test (but after the test had completed):
Current queue must not be empty
'package:sweyer/logic/player/queue.dart':
Failed assertion: line 257 pos 12: 'value.isNotEmpty'

When the exception was thrown, this was the stack:
#2      QueuesState.current (package:sweyer/logic/player/queue.dart:257:12)
#3      PlaybackControl.currentSongIndex (package:sweyer/logic/player/playback.dart:52:45)
#4      AudioHandler._setState (package:sweyer/logic/player/music_player.dart:616:44)
#5      AudioHandler._init.<anonymous closure> (package:sweyer/logic/player/music_player.dart:215:44)
#16     Subject._add (package:rxdart/src/subjects/subject.dart:141:17)
#17     Subject.add (package:rxdart/src/subjects/subject.dart:135:5)
#39     _StartWithStreamSink._safeAddFirstEvent (package:rxdart/src/transformers/start_with.dart:53:12)
#40     _StartWithStreamSink.onListen.<anonymous closure> (package:rxdart/src/transformers/start_with.dart:35:29)
(elided 41 frames from class _AssertionError, dart:async, and package:stack_trace)
Abestanis commented 1 month ago

Tracking and waiting for all unawaited futures seems non trivial, and I like the idea of creating the controllers at the start and passing them in when creating the widgets, because it could eliminate a bunch of late variables. The downside is that we would have to pass the controllers in and can't use the global singletons anymore. Would you be ok with that approach?

nt4f04uNd commented 1 month ago

Tracking and waiting for all unawaited futures seems non trivial, and I like the idea of creating the controllers at the start and passing them in when creating the widgets, because it could eliminate a bunch of late variables. The downside is that we would have to pass the controllers in and can't use the global singletons anymore. Would you be ok with that approach?

Yes, singletones are a tech debt https://github.com/nt4f04uNd/sweyer/issues/81

Although, please let's not do any major refactorings just yet. There is a new open source library coming out next week, which will be perfect for rewriting our application DI and business logic. I'll let you know.

Abestanis commented 4 weeks ago

I was looking into the root cause of this some more and I'm certain that the main cause is the Streams we use throughout the app, like the player.playingStream. Adding a value to the stream causes a new in awaited future to be spawned that calls the listeners, which can cause the listeners to run after a test has completed.

This is obviously not good even if we remove the singletons, because it means that tests potentially don't use the fake clock and test specific device configurations (e.g. if we ever want to add a test for landscape mode).

I found binding.pumpEventQueue which does work and runs all stream listeners, but it also causes some tests to run forever and never finish, so I'm still investigating that.