mirror of
https://github.com/austinried/subtracks.git
synced 2026-11-11 07:29:14 +01:00
cover the queue/shuffle invariants the QA review flagged
The shuffled-removal tests mostly removed the *current* track (a path that was already correct) and asserted `state.item`, which `rebuildWindow` keeps regardless of the computed cursor, so a wrong position went unnoticed. Add: - deterministic shuffle setup (`seedShuffledQueue`, or replace + setShuffle + setCursor + connect) so shuffle tests assert exact behaviour instead of looping over random seeds; - controller tests that assert the queue position still resolves to the playing track after removing a non-current track, with and without a block, and that undo restores the shuffled order; - a repository property test removing every position with a block present, checking the play/flat bijection, the combined/contextPlay inverse, the block span and that no track repeats; - a `QueueRow.isPlaying` predicate (position AND song id) extracted and unit-tested, and QueueViewModel tests for starting a shuffled queue at the top and reconciling after a shuffled removal; - Shuffle degenerate/out-of-range/MAX_SIZE coverage.
This commit is contained in:
@@ -80,6 +80,13 @@ data class QueueRow(
|
|||||||
val upNext: Boolean = false,
|
val upNext: Boolean = false,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
// Both must match: the position alone can transiently point at the row above while the list
|
||||||
|
// reconciles, and the song id alone lights up a duplicate elsewhere in the queue.
|
||||||
|
internal fun QueueRow.isPlaying(
|
||||||
|
currentPosition: Long?,
|
||||||
|
currentSongId: String?,
|
||||||
|
): Boolean = position == currentPosition && song.song.id == currentSongId
|
||||||
|
|
||||||
class QueueViewModel(
|
class QueueViewModel(
|
||||||
private val queueRepository: QueueRepository,
|
private val queueRepository: QueueRepository,
|
||||||
private val playbackController: PlaybackController,
|
private val playbackController: PlaybackController,
|
||||||
@@ -436,7 +443,7 @@ fun QueueScreen(
|
|||||||
) { isDragging ->
|
) { isDragging ->
|
||||||
QueueRowItem(
|
QueueRowItem(
|
||||||
row = row,
|
row = row,
|
||||||
isPlaying = row.position == currentPosition && row.song.song.id == currentSongId,
|
isPlaying = row.isPlaying(currentPosition, currentSongId),
|
||||||
floating = isDragging,
|
floating = isDragging,
|
||||||
coverArt = coverArt,
|
coverArt = coverArt,
|
||||||
dragHandle =
|
dragHandle =
|
||||||
|
|||||||
@@ -692,6 +692,48 @@ class QueueRepositoryTest {
|
|||||||
assertEquals((0L until after.contextSize).toList(), (0 until after.contextSize).map { after.flatContext(it)!! }.sorted())
|
assertEquals((0L until after.contextSize).toList(), (0 until after.contextSize).map { after.flatContext(it)!! }.sorted())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun removingAnyPositionWhileShuffledKeepsTheCoordinatesConsistent() =
|
||||||
|
runTest {
|
||||||
|
seedLibrary()
|
||||||
|
for (seed in listOf(1L, 42L, 999L)) {
|
||||||
|
for (position in 0L..4L) {
|
||||||
|
repository.replace(listOf(repository.albumEntry(1, "al1")))
|
||||||
|
repository.setShuffle(seed)
|
||||||
|
repository.addUpNext(
|
||||||
|
repository.snapshot(),
|
||||||
|
currentPosition = 0,
|
||||||
|
entry = repository.songEntry(1, "s4"),
|
||||||
|
playNext = false,
|
||||||
|
)
|
||||||
|
val before = repository.snapshot()
|
||||||
|
if (position >= before.size) continue
|
||||||
|
|
||||||
|
repository.removeAt(before, position)
|
||||||
|
|
||||||
|
val after = repository.snapshot()
|
||||||
|
assertTrue("seed=$seed position=$position stopped being shuffled", after.shuffled)
|
||||||
|
assertEquals(before.size - 1, after.size)
|
||||||
|
val flats = (0 until after.contextSize).map { after.flatContext(it)!! }
|
||||||
|
assertEquals((0L until after.contextSize).toList(), flats.sorted())
|
||||||
|
for (play in 0 until after.contextSize) {
|
||||||
|
assertEquals(play, after.playContext(after.flatContext(play)!!))
|
||||||
|
}
|
||||||
|
for (p in 0 until after.size) {
|
||||||
|
if (after.isUpNext(p)) continue
|
||||||
|
assertEquals(p, after.combined(after.contextPlay(p)))
|
||||||
|
}
|
||||||
|
val block = (0 until after.size).filter { after.isUpNext(it) }
|
||||||
|
assertEquals(
|
||||||
|
(after.anchorPlay + 1..after.anchorPlay + after.upNextSize).toList(),
|
||||||
|
block,
|
||||||
|
)
|
||||||
|
val ids = (0 until after.size).mapNotNull { repository.itemAt(after, it)?.song?.id }
|
||||||
|
assertEquals(ids.distinct().size, ids.size)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
private suspend fun seedLibrary() {
|
private suspend fun seedLibrary() {
|
||||||
db.sourcesDao().upsertSource(
|
db.sourcesDao().upsertSource(
|
||||||
Source(id = 1, name = "test", address = "http://localhost", isActive = true, createdAt = 0),
|
Source(id = 1, name = "test", address = "http://localhost", isActive = true, createdAt = 0),
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
package com.subtracks.data.repo
|
package com.subtracks.data.repo
|
||||||
|
|
||||||
import org.junit.Assert.assertEquals
|
import org.junit.Assert.assertEquals
|
||||||
|
import org.junit.Assert.assertThrows
|
||||||
import org.junit.Assert.assertTrue
|
import org.junit.Assert.assertTrue
|
||||||
import org.junit.Test
|
import org.junit.Test
|
||||||
|
|
||||||
@@ -103,6 +104,22 @@ class ShuffleTest {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun handlesDegenerateAndOutOfRangeInputs() {
|
||||||
|
assertEquals(0L, Shuffle.toFlat(1L, 0L, 5L))
|
||||||
|
assertEquals(0L, Shuffle.toSequence(1L, 0L, 5L))
|
||||||
|
assertEquals(0L, Shuffle.toFlat(1L, 1L, 0L))
|
||||||
|
assertEquals(10L, Shuffle.toFlat(1L, 10L, 10L))
|
||||||
|
assertEquals(10L, Shuffle.toSequence(1L, 10L, 10L))
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun rejectsSizesBeyondThePermutationDomain() {
|
||||||
|
assertThrows(IllegalArgumentException::class.java) {
|
||||||
|
Shuffle.toFlat(1L, (1L shl 40) + 1L, 0L)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun consecutivePositionsAreNotASingleStride() {
|
fun consecutivePositionsAreNotASingleStride() {
|
||||||
val size = 1000L
|
val size = 1000L
|
||||||
|
|||||||
@@ -28,6 +28,7 @@ import okhttp3.OkHttpClient
|
|||||||
import org.junit.After
|
import org.junit.After
|
||||||
import org.junit.Assert.assertEquals
|
import org.junit.Assert.assertEquals
|
||||||
import org.junit.Assert.assertFalse
|
import org.junit.Assert.assertFalse
|
||||||
|
import org.junit.Assert.assertNotEquals
|
||||||
import org.junit.Assert.assertNotNull
|
import org.junit.Assert.assertNotNull
|
||||||
import org.junit.Assert.assertTrue
|
import org.junit.Assert.assertTrue
|
||||||
import org.junit.Before
|
import org.junit.Before
|
||||||
@@ -1731,6 +1732,73 @@ class PlaybackControllerTest {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun undoingARemovalRestoresTheShuffledOrder() {
|
||||||
|
seedShuffledQueue(seed = 42L)
|
||||||
|
val orderBefore = resolvedIds()
|
||||||
|
val currentPosition = controller.state.value.position!!
|
||||||
|
val before = runBlocking { queues.snapshot() }
|
||||||
|
val removePosition = (0 until before.size).first { it != currentPosition }
|
||||||
|
|
||||||
|
runBlocking { controller.removeAt(removePosition) }
|
||||||
|
assertNotEquals(orderBefore, resolvedIds())
|
||||||
|
|
||||||
|
runBlocking { controller.undo() }
|
||||||
|
|
||||||
|
await { controller.state.value.shuffle && resolvedIds() == orderBefore }
|
||||||
|
assertEquals(orderBefore, resolvedIds())
|
||||||
|
assertEquals(currentPosition, controller.state.value.position)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun removingANonCurrentTrackWhileShuffledKeepsTheCursorOnThePlayingTrack() {
|
||||||
|
seedShuffledQueue(seed = 12345L)
|
||||||
|
val currentId =
|
||||||
|
controller.state.value.item!!
|
||||||
|
.id
|
||||||
|
val before = runBlocking { queues.snapshot() }
|
||||||
|
val currentPosition = controller.state.value.position!!
|
||||||
|
val removePosition = (0 until before.size).first { it != currentPosition }
|
||||||
|
|
||||||
|
runBlocking { controller.removeAt(removePosition) }
|
||||||
|
|
||||||
|
val after = runBlocking { queues.snapshot() }
|
||||||
|
assertEquals(
|
||||||
|
currentId,
|
||||||
|
controller.state.value.item
|
||||||
|
?.id,
|
||||||
|
)
|
||||||
|
assertEquals(currentId, runBlocking { queues.itemAt(after, controller.state.value.position!!)?.song?.id })
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun removingANonCurrentTrackWhileShuffledKeepsTheBlockNext() {
|
||||||
|
seedAlbum(6, sourceId = 1)
|
||||||
|
seedSong("x1", "al2")
|
||||||
|
runBlocking {
|
||||||
|
queues.replace(listOf(queues.albumEntry(1, "al1")))
|
||||||
|
queues.setShuffle(999L)
|
||||||
|
queues.setCursor(0)
|
||||||
|
}
|
||||||
|
controller.connect()
|
||||||
|
await { controller.state.value.item != null && controller.state.value.shuffle }
|
||||||
|
controller.addToQueue(1, QueueKind.Song, "x1")
|
||||||
|
await { runBlocking { controller.upcomingItem()?.id } == "x1" }
|
||||||
|
|
||||||
|
val currentId =
|
||||||
|
controller.state.value.item!!
|
||||||
|
.id
|
||||||
|
val before = runBlocking { queues.snapshot() }
|
||||||
|
val currentPosition = controller.state.value.position!!
|
||||||
|
val removePosition = (0 until before.size).first { !before.isUpNext(it) && it != currentPosition }
|
||||||
|
|
||||||
|
runBlocking { controller.removeAt(removePosition) }
|
||||||
|
|
||||||
|
val after = runBlocking { queues.snapshot() }
|
||||||
|
assertEquals(currentId, runBlocking { queues.itemAt(after, controller.state.value.position!!)?.song?.id })
|
||||||
|
await { runBlocking { controller.upcomingItem()?.id } == "x1" }
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun movingContextTracksWhileShuffledIsIgnored() {
|
fun movingContextTracksWhileShuffledIsIgnored() {
|
||||||
seedAlbum(6, sourceId = 1)
|
seedAlbum(6, sourceId = 1)
|
||||||
@@ -2376,6 +2444,28 @@ class PlaybackControllerTest {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Loads a shuffled album under a fixed seed, so the permutation is deterministic and a test can
|
||||||
|
// assert exact positions rather than looping until a lucky seed exposes a bug.
|
||||||
|
private fun seedShuffledQueue(
|
||||||
|
seed: Long,
|
||||||
|
count: Int = 6,
|
||||||
|
) {
|
||||||
|
seedAlbum(count, sourceId = 1)
|
||||||
|
runBlocking {
|
||||||
|
queues.replace(listOf(queues.albumEntry(1, "al1")))
|
||||||
|
queues.setShuffle(seed)
|
||||||
|
queues.setCursor(0)
|
||||||
|
}
|
||||||
|
controller.connect()
|
||||||
|
await { controller.state.value.item != null && controller.state.value.shuffle }
|
||||||
|
}
|
||||||
|
|
||||||
|
private fun resolvedIds(): List<String> =
|
||||||
|
runBlocking {
|
||||||
|
val snapshot = queues.snapshot()
|
||||||
|
(0 until snapshot.size).mapNotNull { queues.itemAt(snapshot, it)?.song?.id }
|
||||||
|
}
|
||||||
|
|
||||||
private fun await(predicate: () -> Boolean) {
|
private fun await(predicate: () -> Boolean) {
|
||||||
val deadline = System.nanoTime() + 5_000_000_000L
|
val deadline = System.nanoTime() + 5_000_000_000L
|
||||||
while (!predicate() && System.nanoTime() < deadline) Thread.sleep(10)
|
while (!predicate() && System.nanoTime() < deadline) Thread.sleep(10)
|
||||||
|
|||||||
@@ -25,6 +25,7 @@ import kotlinx.coroutines.test.setMain
|
|||||||
import okhttp3.OkHttpClient
|
import okhttp3.OkHttpClient
|
||||||
import org.junit.After
|
import org.junit.After
|
||||||
import org.junit.Assert.assertEquals
|
import org.junit.Assert.assertEquals
|
||||||
|
import org.junit.Assert.assertFalse
|
||||||
import org.junit.Assert.assertTrue
|
import org.junit.Assert.assertTrue
|
||||||
import org.junit.Before
|
import org.junit.Before
|
||||||
import org.junit.Test
|
import org.junit.Test
|
||||||
@@ -85,6 +86,67 @@ class QueueViewModelTest {
|
|||||||
assertEquals((10L..189L).toList(), viewModel.rows.map { it.position })
|
assertEquals((10L..189L).toList(), viewModel.rows.map { it.position })
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun thePlayingHighlightNeedsBothPositionAndSong() {
|
||||||
|
val playing = row(5)
|
||||||
|
|
||||||
|
assertTrue(playing.isPlaying(5L, "s5"))
|
||||||
|
assertFalse("position lagged while the list reconciled", playing.isPlaying(6L, "s5"))
|
||||||
|
assertFalse("duplicate song elsewhere in the queue", playing.isPlaying(5L, "s6"))
|
||||||
|
assertFalse(playing.isPlaying(null, "s5"))
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun openingAShuffledQueueStartsAtTheTop() {
|
||||||
|
runBlocking { seedSongs(20) }
|
||||||
|
runBlocking {
|
||||||
|
queues.replace(listOf(queues.albumEntry(1, "al1")))
|
||||||
|
queues.setShuffle(12345L)
|
||||||
|
queues.setCursor(0)
|
||||||
|
}
|
||||||
|
controller.connect()
|
||||||
|
await { controller.state.value.item != null && controller.state.value.shuffle }
|
||||||
|
|
||||||
|
viewModel.open()
|
||||||
|
await { viewModel.ready }
|
||||||
|
|
||||||
|
assertEquals(0, viewModel.initialIndex)
|
||||||
|
assertEquals(0L, viewModel.rows.first().position)
|
||||||
|
assertTrue(
|
||||||
|
viewModel.rows.first().isPlaying(
|
||||||
|
0L,
|
||||||
|
controller.state.value.item
|
||||||
|
?.id,
|
||||||
|
),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun removingWhileShuffledReconcilesTheRows() {
|
||||||
|
runBlocking { seedSongs(20) }
|
||||||
|
runBlocking {
|
||||||
|
queues.replace(listOf(queues.albumEntry(1, "al1")))
|
||||||
|
queues.setShuffle(12345L)
|
||||||
|
queues.setCursor(0)
|
||||||
|
}
|
||||||
|
controller.connect()
|
||||||
|
await { controller.state.value.item != null && controller.state.value.shuffle }
|
||||||
|
viewModel.open()
|
||||||
|
await { viewModel.ready }
|
||||||
|
|
||||||
|
val before = runBlocking { queues.snapshot() }
|
||||||
|
val currentPosition = controller.state.value.position!!
|
||||||
|
val removePosition = (0 until before.size).first { it != currentPosition }
|
||||||
|
|
||||||
|
viewModel.remove(removePosition)
|
||||||
|
|
||||||
|
await {
|
||||||
|
val snapshot = runBlocking { queues.snapshot() }
|
||||||
|
viewModel.rows.isNotEmpty() &&
|
||||||
|
viewModel.rows.all { runBlocking { queues.itemAt(snapshot, it.position)?.song?.id } == it.song.song.id }
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun openingAtTheEndLoadsAWindowAndExtendsOlder() {
|
fun openingAtTheEndLoadsAWindowAndExtendsOlder() {
|
||||||
runBlocking { seedSongs(200) }
|
runBlocking { seedSongs(200) }
|
||||||
|
|||||||
Reference in New Issue
Block a user