JMR-devandClaude Opus 5
cc424dd08f
`reattach()` read `_state.value`, found it `Idle`, and then handed the job to `observe()` -- which launches a *separate* coroutine that cannot write until its `collect` has resumed with a `WorkInfo`. So the check happened at one moment and the write landed at another, with a whole pick able to fit in between: the user tapped, their metadata query suspended, the guard saw an empty screen, and the finished job from an earlier session wrote over `Ready(picked)` a moment later. The comment above that guard said "no suspension point between this check and the assignment below, so nothing can interleave". There is no assignment below, and the two lines are in different coroutines. That sentence is why this sat as flaky CI for two days rather than being read as the product race it is. `ScreenOwnership` makes the answer the test already encodes -- the user's pick wins -- true rather than probable. A claim is taken synchronously when the user acts; every write that lands after a suspension point checks the claim it was made under and drops itself if that claim has been superseded. Dropped, not reordered: a write that is dropped cannot come back later. Cancelling the superseded observer was never enough on its own. `Job.cancel` is honoured at the next suspension point, and a collector that has already resumed and is on its way to `_state.value = ...` has none left; the write lands anyway. It also cannot help at all in the case reported, where nothing supersedes the observation until after it has been launched. `JoinViewModel` had the identical shape and nothing watching it, so it gets the same fix and the counterpart test that was missing. Its pick dispatcher becomes injectable for the same reason `ConversionViewModel`'s already was: without that seam there is no way to ask what happens while a pick is still in flight. Closes #49 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
88 lines
3.0 KiB
Kotlin
88 lines
3.0 KiB
Kotlin
package org.libremediaconverter.join
|
|
|
|
import android.app.Application
|
|
import android.net.Uri
|
|
import androidx.media3.common.util.UnstableApi
|
|
import androidx.work.workDataOf
|
|
import org.junit.After
|
|
import org.junit.Assert.assertEquals
|
|
import org.junit.Before
|
|
import org.junit.Test
|
|
import org.junit.runner.RunWith
|
|
import org.libremediaconverter.convert.ConversionDependencies
|
|
import org.libremediaconverter.convert.ParkedPickDispatcher
|
|
import org.libremediaconverter.convert.RecordingPublisher
|
|
import org.libremediaconverter.convert.installTestWorkManager
|
|
import org.libremediaconverter.work.ConcatWorker
|
|
import org.robolectric.RobolectricTestRunner
|
|
import org.robolectric.RuntimeEnvironment
|
|
|
|
/**
|
|
* `PickOwnershipTest`'s case on the join side.
|
|
*
|
|
* `onInputsPicked` makes one write and it lands after a hop off the main thread, so it belongs to
|
|
* whichever pick was in flight rather than to whichever set of files the user last chose. Two
|
|
* selections in quick succession — likelier here than on the convert side, since a join picks
|
|
* several files at a time and the metadata query is per file — put the loser's files on screen if
|
|
* its query came back second.
|
|
*/
|
|
@UnstableApi
|
|
@RunWith(RobolectricTestRunner::class)
|
|
class JoinPickOwnershipTest {
|
|
|
|
private lateinit var app: Application
|
|
private lateinit var parkedPick: ParkedPickDispatcher
|
|
private lateinit var viewModel: JoinViewModel
|
|
|
|
@Before
|
|
fun setUp() {
|
|
app = RuntimeEnvironment.getApplication()
|
|
ConversionDependencies.publisher = { RecordingPublisher(app) }
|
|
installTestWorkManager(app, workDataOf(ConcatWorker.KEY_OUTPUT_PATH to "/dev/null"))
|
|
|
|
parkedPick = ParkedPickDispatcher()
|
|
viewModel = JoinViewModel(app, pickDispatcher = parkedPick)
|
|
}
|
|
|
|
@After
|
|
fun tearDown() {
|
|
ConversionDependencies.reset()
|
|
}
|
|
|
|
/**
|
|
* Two selections, with the first one's metadata query the slow one.
|
|
*
|
|
* The order is chosen rather than raced: both queries are parked, and this runs the second
|
|
* before the first.
|
|
*/
|
|
@Test
|
|
fun `the slower of two selections does not land on top of the faster one`() {
|
|
viewModel.onInputsPicked(FIRST)
|
|
viewModel.onInputsPicked(SECOND)
|
|
|
|
val queries = parkedPick.takeParked()
|
|
assertEquals("both selections should be in flight", 2, queries.size)
|
|
// The second selection's query comes back first; the first one's is the straggler.
|
|
queries[1].run()
|
|
queries[0].run()
|
|
|
|
val current = viewModel.state.value
|
|
assertEquals(
|
|
"a selection the user has already replaced took the screen: $current",
|
|
SECOND,
|
|
(current as JoinState.Ready).inputs.map { it.uri },
|
|
)
|
|
}
|
|
|
|
private companion object {
|
|
val FIRST = listOf(
|
|
Uri.parse("content://test/first-a.mp4"),
|
|
Uri.parse("content://test/first-b.mp4"),
|
|
)
|
|
val SECOND = listOf(
|
|
Uri.parse("content://test/second-a.mp4"),
|
|
Uri.parse("content://test/second-b.mp4"),
|
|
)
|
|
}
|
|
}
|