finish the queue view test row race fix

#71 converted only two reads; the rest of `QueueViewModelTest` still read
`viewModel.rows` — a snapshot list mutated on the main dispatcher — from the
test thread, so `removeAndUndoKeepTheBoundedWindowConsistent` could map a torn
list (the removed position missing) under load. Route every test-thread read
through `rowsSnapshot()` and run the synchronous `reorder` mutations on the
dispatcher too.
This commit is contained in:
austinried
2026-10-04 00:30:01 +00:00
parent 65ed658433
commit 6a311638c2
@@ -89,16 +89,16 @@ class QueueViewModelTest {
viewModel.open() viewModel.open()
await { viewModel.ready } await { viewModel.ready }
assertEquals((70L..129L).toList(), viewModel.rows.map { it.position }) assertEquals((70L..129L).toList(), rowsSnapshot().map { it.position })
assertEquals(30, viewModel.initialIndex) assertEquals(30, viewModel.initialIndex)
viewModel.loadNewer() viewModel.loadNewer()
await { viewModel.rows.lastOrNull()?.position == 189L } await { rowsSnapshot().lastOrNull()?.position == 189L }
assertEquals((70L..189L).toList(), viewModel.rows.map { it.position }) assertEquals((70L..189L).toList(), rowsSnapshot().map { it.position })
viewModel.loadOlder() viewModel.loadOlder()
await { viewModel.rows.firstOrNull()?.position == 10L } await { rowsSnapshot().firstOrNull()?.position == 10L }
assertEquals((10L..189L).toList(), viewModel.rows.map { it.position }) assertEquals((10L..189L).toList(), rowsSnapshot().map { it.position })
} }
@Test @Test
@@ -126,9 +126,9 @@ class QueueViewModelTest {
await { viewModel.ready } await { viewModel.ready }
assertEquals(0, viewModel.initialIndex) assertEquals(0, viewModel.initialIndex)
assertEquals(0L, viewModel.rows.first().position) assertEquals(0L, rowsSnapshot().first().position)
assertTrue( assertTrue(
viewModel.rows.first().isPlaying( rowsSnapshot().first().isPlaying(
0L, 0L,
controller.state.value.item controller.state.value.item
?.id, ?.id,
@@ -172,12 +172,12 @@ class QueueViewModelTest {
viewModel.open() viewModel.open()
await { viewModel.ready } await { viewModel.ready }
assertEquals((169L..199L).toList(), viewModel.rows.map { it.position }) assertEquals((169L..199L).toList(), rowsSnapshot().map { it.position })
assertEquals(30, viewModel.initialIndex) assertEquals(30, viewModel.initialIndex)
viewModel.loadOlder() viewModel.loadOlder()
await { viewModel.rows.firstOrNull()?.position == 109L } await { rowsSnapshot().firstOrNull()?.position == 109L }
assertEquals((109L..199L).toList(), viewModel.rows.map { it.position }) assertEquals((109L..199L).toList(), rowsSnapshot().map { it.position })
} }
@Test @Test
@@ -195,13 +195,13 @@ class QueueViewModelTest {
viewModel.open() viewModel.open()
await { viewModel.ready } await { viewModel.ready }
val firstIds = viewModel.rows.map { it.id } val firstIds = rowsSnapshot().map { it.id }
val firstGeneration = viewModel.generation val firstGeneration = viewModel.generation
viewModel.open() viewModel.open()
await { viewModel.generation > firstGeneration } await { viewModel.generation > firstGeneration }
assertEquals(firstIds, viewModel.rows.map { it.id }) assertEquals(firstIds, rowsSnapshot().map { it.id })
} }
@Test @Test
@@ -213,9 +213,9 @@ class QueueViewModelTest {
viewModel.open() viewModel.open()
await { viewModel.ready } await { viewModel.ready }
viewModel.reorder(fromIndex = 0, toIndex = 2) runOnMain { viewModel.reorder(fromIndex = 0, toIndex = 2) }
val positions = viewModel.rows.map { it.position } val positions = rowsSnapshot().map { it.position }
assertEquals(listOf(71L, 72L, 70L), positions.take(3)) assertEquals(listOf(71L, 72L, 70L), positions.take(3))
} }
@@ -227,12 +227,12 @@ class QueueViewModelTest {
viewModel.open() viewModel.open()
await { viewModel.ready } await { viewModel.ready }
assertEquals((70L..129L).toList(), viewModel.rows.map { it.position }) assertEquals((70L..129L).toList(), rowsSnapshot().map { it.position })
extendToEnd() extendToEnd()
assertEquals(QUEUE_WINDOW_ROWS, viewModel.rows.size) assertEquals(QUEUE_WINDOW_ROWS, rowsSnapshot().size)
assertEquals((220L..399L).toList(), viewModel.rows.map { it.position }) assertEquals((220L..399L).toList(), rowsSnapshot().map { it.position })
} }
@Test @Test
@@ -248,7 +248,7 @@ class QueueViewModelTest {
viewModel.loadOlder() viewModel.loadOlder()
awaitWindow(first = 160, last = 339) awaitWindow(first = 160, last = 339)
assertEquals(QUEUE_WINDOW_ROWS, viewModel.rows.size) assertEquals(QUEUE_WINDOW_ROWS, rowsSnapshot().size)
} }
@Test @Test
@@ -262,23 +262,23 @@ class QueueViewModelTest {
extendToEnd() extendToEnd()
val movedSongId = val movedSongId =
viewModel.rows[10] rowsSnapshot()[10]
.song.song.id .song.song.id
val from = viewModel.rows[10].position val from = rowsSnapshot()[10].position
val to = viewModel.rows[13].position val to = rowsSnapshot()[13].position
viewModel.reorder(10, 13) runOnMain { viewModel.reorder(10, 13) }
viewModel.move(from, to) viewModel.move(from, to)
await { await {
viewModel.rows.size == QUEUE_WINDOW_ROWS && rowsSnapshot().size == QUEUE_WINDOW_ROWS &&
viewModel.rows.getOrNull(13)?.position == 233L rowsSnapshot().getOrNull(13)?.position == 233L
} }
assertEquals( assertEquals(
movedSongId, movedSongId,
viewModel.rows[13] rowsSnapshot()[13]
.song.song.id, .song.song.id,
) )
assertEquals((220L..399L).toList(), viewModel.rows.map { it.position }) assertEquals((220L..399L).toList(), rowsSnapshot().map { it.position })
} }
@Test @Test
@@ -290,18 +290,18 @@ class QueueViewModelTest {
viewModel.open() viewModel.open()
await { viewModel.ready } await { viewModel.ready }
extendToEnd() extendToEnd()
assertEquals((220L..399L).toList(), viewModel.rows.map { it.position }) assertEquals((220L..399L).toList(), rowsSnapshot().map { it.position })
val from = viewModel.rows.first().position val from = rowsSnapshot().first().position
viewModel.reorder(0, 5) runOnMain { viewModel.reorder(0, 5) }
val to = dropTarget(viewModel.rows, index = 5, from = from) val to = dropTarget(rowsSnapshot(), index = 5, from = from)
viewModel.move(from, to) viewModel.move(from, to)
await { await {
viewModel.rows.firstOrNull()?.position == 220L && rowsSnapshot().firstOrNull()?.position == 220L &&
viewModel.rows.lastOrNull()?.position == 399L rowsSnapshot().lastOrNull()?.position == 399L
} }
assertEquals((220L..399L).toList(), viewModel.rows.map { it.position }) assertEquals((220L..399L).toList(), rowsSnapshot().map { it.position })
} }
@Test @Test
@@ -344,26 +344,26 @@ class QueueViewModelTest {
viewModel.open() viewModel.open()
await { viewModel.ready } await { viewModel.ready }
val removedSongId = val removedSongId =
viewModel.rows[30] rowsSnapshot()[30]
.song.song.id .song.song.id
viewModel.remove(viewModel.rows[30].position) viewModel.remove(rowsSnapshot()[30].position)
await { await {
viewModel.rows.size == 60 && rowsSnapshot().size == 60 &&
viewModel.rows.firstOrNull()?.position == 70L && rowsSnapshot().firstOrNull()?.position == 70L &&
viewModel.rows rowsSnapshot()
.getOrNull(30) .getOrNull(30)
?.song ?.song
?.song ?.song
?.id != removedSongId ?.id != removedSongId
} }
assertEquals((70L..129L).toList(), viewModel.rows.map { it.position }) assertEquals((70L..129L).toList(), rowsSnapshot().map { it.position })
viewModel.undo() viewModel.undo()
await { await {
viewModel.rows.size == 60 && rowsSnapshot().size == 60 &&
viewModel.rows.firstOrNull()?.position == 70L && rowsSnapshot().firstOrNull()?.position == 70L &&
viewModel.rows rowsSnapshot()
.getOrNull(30) .getOrNull(30)
?.song ?.song
?.song ?.song
@@ -375,7 +375,7 @@ class QueueViewModelTest {
private fun assertContiguous() = assertTrue(isContiguous()) private fun assertContiguous() = assertTrue(isContiguous())
private fun isContiguous(): Boolean { private fun isContiguous(): Boolean {
val positions = viewModel.rows.map { it.position } val positions = rowsSnapshot().map { it.position }
return positions.isNotEmpty() && positions == (positions.first()..positions.last()).toList() return positions.isNotEmpty() && positions == (positions.first()..positions.last()).toList()
} }
@@ -383,9 +383,9 @@ class QueueViewModelTest {
first: Long, first: Long,
last: Long, last: Long,
) = await { ) = await {
viewModel.rows.size == (last - first + 1).toInt() && rowsSnapshot().size == (last - first + 1).toInt() &&
viewModel.rows.firstOrNull()?.position == first && rowsSnapshot().firstOrNull()?.position == first &&
viewModel.rows.lastOrNull()?.position == last rowsSnapshot().lastOrNull()?.position == last
} }
private fun extendTo( private fun extendTo(
@@ -456,6 +456,8 @@ class QueueViewModelTest {
// the test thread (otherwise iterating it races the view model and throws or reads torn state). // the test thread (otherwise iterating it races the view model and throws or reads torn state).
private fun rowsSnapshot(): List<QueueRow> = runBlocking { withContext(dispatcher) { viewModel.rows.toList() } } private fun rowsSnapshot(): List<QueueRow> = runBlocking { withContext(dispatcher) { viewModel.rows.toList() } }
private fun runOnMain(block: () -> Unit) = runBlocking { withContext(dispatcher) { block() } }
private fun await(predicate: () -> Boolean) { private fun await(predicate: () -> Boolean) {
repeat(500) { repeat(500) {
if (predicate()) return if (predicate()) return