Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
dec7089b59 | ||
|
|
b677a9ad02 | ||
|
|
34e4ab52a4 | ||
|
|
1437157a8f | ||
|
|
1041faf920 | ||
|
|
fe68f839c1 | ||
|
|
f65578b1f7 | ||
|
|
b38ad6a683 | ||
|
|
9a0f494e26 | ||
|
|
f3478706b3 | ||
|
|
61c400d2c6 | ||
|
|
d83775d5c6 | ||
|
|
e90f5a801c | ||
|
|
68015b3374 | ||
|
|
ec2cae256f | ||
|
|
4e88de3045 | ||
|
|
2db0dc65d3 | ||
|
|
6334dcba34 | ||
|
|
7e09f010c7 |
@@ -673,13 +673,23 @@ class ConversionViewModel @JvmOverloads constructor(
|
||||
else -> null
|
||||
}
|
||||
|
||||
private fun currentInput(): InputFile? = when (val s = _state.value) {
|
||||
is ConversionState.Ready -> s.input
|
||||
is ConversionState.Converting -> s.input
|
||||
is ConversionState.Waiting -> s.input
|
||||
is ConversionState.Converted -> s.input
|
||||
else -> null
|
||||
}
|
||||
/**
|
||||
* The input `convert()` may act on, which is only ever the one on a `Ready` screen.
|
||||
*
|
||||
* This used to answer for `Converting`, `Waiting` and `Converted` as well. Those arms were not
|
||||
* reachable by tapping Convert -- the button renders only in the `Ready` branch -- but they
|
||||
* were reachable through the POST_NOTIFICATIONS **result**, which `ConverterScreen.kt:91` wires
|
||||
* to `convert()` rather than to the button. Reaching one of them enqueued a *second* job over a
|
||||
* live one: `activeWorkId` was overwritten, and the first job kept running with its foreground
|
||||
* notification orphaned and nothing left holding its id to cancel it.
|
||||
*
|
||||
* Narrowed under #202 rather than tested as it stood, because a test written against the old
|
||||
* shape would have frozen the double-enqueue as intended behaviour -- the F1/F5 failure mode.
|
||||
*
|
||||
* `JoinViewModel.join()` has been `(_state.value as? JoinState.Ready)?.inputs ?: return` all
|
||||
* along. The two screens are the same shape and only one of them was over-general.
|
||||
*/
|
||||
private fun currentInput(): InputFile? = (_state.value as? ConversionState.Ready)?.input
|
||||
|
||||
private companion object {
|
||||
/**
|
||||
|
||||
@@ -5,7 +5,6 @@ import android.net.Uri
|
||||
import android.util.Log
|
||||
import com.arthenica.ffmpegkit.FFmpegKit
|
||||
import com.arthenica.ffmpegkit.FFmpegKitConfig
|
||||
import com.arthenica.ffmpegkit.ReturnCode
|
||||
import kotlinx.coroutines.suspendCancellableCoroutine
|
||||
import org.libremediaconverter.convert.ConcatJoiner
|
||||
import org.libremediaconverter.convert.MediaProbe
|
||||
@@ -66,16 +65,16 @@ class ConcatEngine(private val context: Context) : ConcatJoiner {
|
||||
private suspend fun execute(args: List<String>) = suspendCancellableCoroutine { cont ->
|
||||
Log.i(TAG, "ffmpeg ${args.joinToString(" ")}")
|
||||
val session = FFmpegKit.executeWithArgumentsAsync(args.toTypedArray()) { completed ->
|
||||
val rc = completed.getReturnCode()
|
||||
when {
|
||||
ReturnCode.isSuccess(rc) -> cont.resume(Unit)
|
||||
ReturnCode.isCancel(rc) -> cont.cancel()
|
||||
else -> cont.resumeWithException(
|
||||
FFmpegEngine.FFmpegException(
|
||||
"Joining failed (${rc?.value}): " +
|
||||
completed.getAllLogsAsString(LOG_TAIL_LIMIT).orEmpty(),
|
||||
),
|
||||
)
|
||||
val outcome = sessionOutcome(
|
||||
rc = completed.getReturnCode(),
|
||||
prefix = "Joining",
|
||||
failStackTrace = { completed.getFailStackTrace() },
|
||||
logTail = { completed.getAllLogsAsString(LOG_TAIL_LIMIT) },
|
||||
)
|
||||
when (outcome) {
|
||||
SessionOutcome.Success -> cont.resume(Unit)
|
||||
SessionOutcome.Cancelled -> cont.cancel()
|
||||
is SessionOutcome.Failed -> cont.resumeWithException(FFmpegEngine.FFmpegException(outcome.message))
|
||||
}
|
||||
}
|
||||
cont.invokeOnCancellation { FFmpegKit.cancel(session.getSessionId()) }
|
||||
|
||||
@@ -4,7 +4,6 @@ import android.util.Log
|
||||
import com.arthenica.ffmpegkit.FFmpegKit
|
||||
import com.arthenica.ffmpegkit.FFmpegKitConfig
|
||||
import com.arthenica.ffmpegkit.Level
|
||||
import com.arthenica.ffmpegkit.ReturnCode
|
||||
import kotlinx.coroutines.suspendCancellableCoroutine
|
||||
import org.libremediaconverter.convert.SoftwareTranscoder
|
||||
import org.libremediaconverter.model.ConversionRequest
|
||||
@@ -51,19 +50,16 @@ class FFmpegEngine : SoftwareTranscoder {
|
||||
val session = FFmpegKit.executeWithArgumentsAsync(
|
||||
args.toTypedArray(),
|
||||
{ completed ->
|
||||
val rc = completed.getReturnCode()
|
||||
when {
|
||||
ReturnCode.isSuccess(rc) -> cont.resume(Unit)
|
||||
ReturnCode.isCancel(rc) ->
|
||||
cont.cancel()
|
||||
else -> cont.resumeWithException(
|
||||
FFmpegException(
|
||||
"FFmpeg failed (${rc?.value}): " +
|
||||
completed.getFailStackTrace().orEmpty().ifBlank {
|
||||
completed.getAllLogsAsString(LOG_TAIL_LIMIT).orEmpty()
|
||||
},
|
||||
),
|
||||
)
|
||||
val outcome = sessionOutcome(
|
||||
rc = completed.getReturnCode(),
|
||||
prefix = "FFmpeg",
|
||||
failStackTrace = { completed.getFailStackTrace() },
|
||||
logTail = { completed.getAllLogsAsString(LOG_TAIL_LIMIT) },
|
||||
)
|
||||
when (outcome) {
|
||||
SessionOutcome.Success -> cont.resume(Unit)
|
||||
SessionOutcome.Cancelled -> cont.cancel()
|
||||
is SessionOutcome.Failed -> cont.resumeWithException(FFmpegException(outcome.message))
|
||||
}
|
||||
},
|
||||
{ log -> Log.d(TAG, log.message.trimEnd()) },
|
||||
|
||||
@@ -0,0 +1,54 @@
|
||||
package org.libremediaconverter.ffmpeg
|
||||
|
||||
import com.arthenica.ffmpegkit.ReturnCode
|
||||
|
||||
/**
|
||||
* What a finished FFmpegKit session means, as a function of its return code.
|
||||
*
|
||||
* Both engines had their own copy of this `when`, twelve lines apart in two files, and the copies
|
||||
* had drifted: [FFmpegEngine] preferred the fail stack trace and fell back to the log tail, while
|
||||
* [ConcatEngine] only ever read the log tail. Neither was tested — both live inside a callback
|
||||
* handed to `FFmpegKit`, which does not run on the JVM — so the divergence was invisible.
|
||||
*
|
||||
* #203 decided to unify on the stack trace, so a join failure now carries the diagnostics a
|
||||
* conversion failure always did. The *prefix* stays per-engine: unifying the strategy must not
|
||||
* unify the sentence, since "FFmpeg failed" and "Joining failed" describe different jobs.
|
||||
*/
|
||||
internal sealed interface SessionOutcome {
|
||||
|
||||
/** rc 0. The suspension resumes normally. */
|
||||
data object Success : SessionOutcome
|
||||
|
||||
/** rc 255. The suspension is cancelled rather than failed — the user asked for this. */
|
||||
data object Cancelled : SessionOutcome
|
||||
|
||||
/** Anything else, with the sentence the user is shown. */
|
||||
data class Failed(val message: String) : SessionOutcome
|
||||
}
|
||||
|
||||
/**
|
||||
* Maps a return code onto the outcome, and builds the failure sentence when there is one.
|
||||
*
|
||||
* **The two message parts arrive as lambdas, deliberately.** `getAllLogsAsString` and
|
||||
* `getFailStackTrace` are calls onto a native session, and only the failure arm needs either. Taking
|
||||
* them by value would put both on the happy path of every successful conversion, which is a cost the
|
||||
* shape this replaced did not have — the old code read them inside the `else` branch. That is the
|
||||
* same reason [org.libremediaconverter.codec.AndroidDeviceCodecs.capabilitiesFrom] takes a
|
||||
* `Sequence`: a seam should not change what runs when.
|
||||
*
|
||||
* A null [rc] is a real input rather than a defensive one — `getReturnCode()` is nullable, and a
|
||||
* session killed before it reported anything has none. It is neither success nor cancellation, so
|
||||
* it fails, and the sentence says `null` where the number would be.
|
||||
*/
|
||||
internal fun sessionOutcome(
|
||||
rc: ReturnCode?,
|
||||
prefix: String,
|
||||
failStackTrace: () -> String?,
|
||||
logTail: () -> String?,
|
||||
): SessionOutcome = when {
|
||||
ReturnCode.isSuccess(rc) -> SessionOutcome.Success
|
||||
ReturnCode.isCancel(rc) -> SessionOutcome.Cancelled
|
||||
else -> SessionOutcome.Failed(
|
||||
"$prefix failed (${rc?.value}): " + failStackTrace().orEmpty().ifBlank { logTail().orEmpty() },
|
||||
)
|
||||
}
|
||||
@@ -51,10 +51,13 @@ import java.io.File
|
||||
* here needs. `OutputPublisherPublishTest` owns what a real publish writes.
|
||||
* - **The screen's two buttons.** `ConverterStateAffordancesTest` and `JoinStateAffordancesTest`
|
||||
* own what each state renders; this file owns what each state carries.
|
||||
* - **`ConverterScreen`'s `destinationMime` line itself.** It lives in the entry point, above the
|
||||
* `ScreenContent` seam, and reaching it needs a real ViewModel inside a composition. What it
|
||||
* reads -- `pendingSave()?.mimeType` -- is asserted directly instead, which is why that
|
||||
* derivation was moved out of the entry point in the first place.
|
||||
* - ~~**`ConverterScreen`'s `destinationMime` line itself.**~~ **Withdrawn 2026-09-02 (#201).** The
|
||||
* exemption read: "it lives in the entry point, above the `ScreenContent` seam, and reaching it
|
||||
* needs a real ViewModel inside a composition". That was true when written and is no longer:
|
||||
* `AdaptiveShellTest` (#173) established composing the real screens with real ViewModels, and
|
||||
* #200 added the `ShadowActivity` mechanics for reading what a launcher launched. `RetrySaveMimeTest`
|
||||
* now asserts the line directly. What this file still owns is the half below the seam -- what each
|
||||
* state *carries* -- which is why `pendingSave()?.mimeType` is also asserted here.
|
||||
* - **Picking a new input while a `Failed` carries a file.** `onInputPicked` overwrites the state
|
||||
* without discarding, from `Converted` exactly as much as from a carrying `Failed`, and neither
|
||||
* branch renders a picker. It is a pre-existing path this change neither opens nor widens: the
|
||||
|
||||
@@ -0,0 +1,141 @@
|
||||
package org.libremediaconverter.convert
|
||||
|
||||
import android.app.Activity
|
||||
import android.content.Intent
|
||||
import android.net.Uri
|
||||
import androidx.activity.ComponentActivity
|
||||
import androidx.compose.ui.test.assertIsDisplayed
|
||||
import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule
|
||||
import androidx.compose.ui.test.onNodeWithTag
|
||||
import androidx.compose.ui.test.performClick
|
||||
import androidx.media3.common.util.UnstableApi
|
||||
import androidx.work.Data
|
||||
import org.junit.After
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Before
|
||||
import org.junit.Rule
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.libremediaconverter.join.JoinScreen
|
||||
import org.libremediaconverter.model.InputProbe
|
||||
import org.libremediaconverter.ui.TestTags
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
import org.robolectric.RuntimeEnvironment
|
||||
import org.robolectric.Shadows.shadowOf
|
||||
import org.robolectric.shadows.ShadowActivity
|
||||
|
||||
/**
|
||||
* The launcher layer above the `ScreenContent` seam — registered, and until now never resulted.
|
||||
*
|
||||
* ## The hazard this exists for
|
||||
*
|
||||
* `ConversionViewModel.onInputPicked(uri: Uri)` and `.save(destination: Uri)` are **both
|
||||
* `(Uri) -> Unit`**, so swapping the two launcher callbacks at `ConverterScreen.kt:70` and `:83`
|
||||
* compiles, renders, and passes the entire suite. Picking a file would attempt a save to it, and
|
||||
* choosing a destination would load it as input.
|
||||
*
|
||||
* That is precisely the defect class `ScreenWiringTest` exists for, on the one pair it declines to
|
||||
* cover: it drives `converterActions` directly and says the launcher-backed actions stay
|
||||
* parameters. Correct for the `actions` seam, and it leaves the edge above that seam unpinned.
|
||||
*
|
||||
* Join's equivalents (`JoinScreen.kt:45`, `:55`) are `List<Uri>` and `Uri`, so they are **not**
|
||||
* transposable and need no such test. The picker filter is a different matter and is covered below
|
||||
* for both screens.
|
||||
*
|
||||
* ## The two mechanics, verified before the assertions were written
|
||||
*
|
||||
* Neither is used anywhere else in the suite, so both were spiked first:
|
||||
*
|
||||
* - **Reading what was launched** — `shadowOf(activity).nextStartedActivityForResult`, which returns
|
||||
* the `Intent` with its `EXTRA_MIME_TYPES` intact.
|
||||
* - **Delivering a result** — `shadowOf(activity).receiveResult(...)`, which reaches
|
||||
* `ComponentActivity`'s `ActivityResultRegistry` and fires the `rememberLauncherForActivityResult`
|
||||
* callback.
|
||||
*
|
||||
* `createAndroidComposeRule`, as `AdaptiveShellTest` uses and for the reason it gives: the screens
|
||||
* compose real ViewModels through `viewModel()`, and the plain rule supplies no `ViewModelStoreOwner`.
|
||||
*/
|
||||
@UnstableApi
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
class LauncherWiringTest {
|
||||
|
||||
@get:Rule
|
||||
val composeRule = createAndroidComposeRule<ComponentActivity>()
|
||||
|
||||
@Before
|
||||
fun setUp() {
|
||||
val app = RuntimeEnvironment.getApplication()
|
||||
installTestWorkManager(app, Data.EMPTY)
|
||||
// The real screen composes a real ViewModel; neither test here is about probing.
|
||||
ConversionDependencies.probe = { _, _ -> InputProbe() }
|
||||
}
|
||||
|
||||
@After
|
||||
fun tearDown() = ConversionDependencies.reset()
|
||||
|
||||
/**
|
||||
* The transposition guard. A picked file has to reach `onInputPicked`, which is observable as
|
||||
* the screen arriving at `Ready` with the file card showing — `save()` from `Idle` returns at
|
||||
* its own guard and leaves nothing behind.
|
||||
*/
|
||||
@Test
|
||||
fun `a picked document is loaded as input rather than saved to`() {
|
||||
composeRule.setContent { ConverterScreen() }
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.Converter.CHOOSE_FILE).performClick()
|
||||
deliver(Uri.parse("content://test/holiday.mkv"))
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.Converter.FILE_CARD_NAME).assertIsDisplayed()
|
||||
}
|
||||
|
||||
/**
|
||||
* `ConverterScreen.kt:65-67` records why the all-types wildcard is load-bearing rather than lazy:
|
||||
*
|
||||
* > the picker is images and video only, offers no audio at all, and will not reliably surface
|
||||
* > .mkv/.flac/.webm
|
||||
*
|
||||
* Narrowing it would make every audio conversion unreachable from the file picker, and nothing
|
||||
* would have gone red. (The literal is spelled only in the assertion below: a KDoc cannot
|
||||
* contain it, because the wildcard's second half closes the comment.)
|
||||
*/
|
||||
@Test
|
||||
fun `the converter picker asks for every type, not just the ones a photo picker offers`() {
|
||||
composeRule.setContent { ConverterScreen() }
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.Converter.CHOOSE_FILE).performClick()
|
||||
|
||||
val intent = launched().intent
|
||||
assertEquals(Intent.ACTION_OPEN_DOCUMENT, intent.action)
|
||||
assertEquals(listOf("*/*"), intent.getStringArrayExtra(Intent.EXTRA_MIME_TYPES)?.toList())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `the join picker asks for video and accepts more than one file`() {
|
||||
composeRule.setContent { JoinScreen() }
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.Join.CHOOSE_FILES).performClick()
|
||||
|
||||
val intent = launched().intent
|
||||
assertEquals(Intent.ACTION_OPEN_DOCUMENT, intent.action)
|
||||
assertEquals(listOf("video/*"), intent.getStringArrayExtra(Intent.EXTRA_MIME_TYPES)?.toList())
|
||||
// A join of one file is not a join; the contract is what asks for several.
|
||||
assertEquals(true, intent.getBooleanExtra(Intent.EXTRA_ALLOW_MULTIPLE, false))
|
||||
}
|
||||
|
||||
private fun launched(): ShadowActivity.IntentForResult {
|
||||
composeRule.waitForIdle()
|
||||
return requireNotNull(shadowOf(composeRule.activity).nextStartedActivityForResult) {
|
||||
"nothing was launched for a result"
|
||||
}
|
||||
}
|
||||
|
||||
private fun deliver(uri: Uri) {
|
||||
val started = launched()
|
||||
shadowOf(composeRule.activity).receiveResult(
|
||||
started.intent,
|
||||
Activity.RESULT_OK,
|
||||
Intent().setData(uri),
|
||||
)
|
||||
composeRule.waitForIdle()
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,136 @@
|
||||
package org.libremediaconverter.convert
|
||||
|
||||
import android.app.Application
|
||||
import android.content.Intent
|
||||
import android.net.Uri
|
||||
import androidx.activity.ComponentActivity
|
||||
import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule
|
||||
import androidx.compose.ui.test.onNodeWithTag
|
||||
import androidx.compose.ui.test.performClick
|
||||
import androidx.compose.ui.test.performScrollTo
|
||||
import androidx.media3.common.util.UnstableApi
|
||||
import androidx.work.WorkManager
|
||||
import androidx.work.workDataOf
|
||||
import org.junit.After
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertNotEquals
|
||||
import org.junit.Before
|
||||
import org.junit.Rule
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.libremediaconverter.model.InputProbe
|
||||
import org.libremediaconverter.model.OutputFormat
|
||||
import org.libremediaconverter.ui.TestTags
|
||||
import org.libremediaconverter.work.ConversionWorker
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
import org.robolectric.RuntimeEnvironment
|
||||
import org.robolectric.Shadows.shadowOf
|
||||
import java.io.File
|
||||
|
||||
/**
|
||||
* The save dialog opens with the type the *job* produced, not the type the picker is showing now.
|
||||
*
|
||||
* `ConverterScreen.kt:80` — `state.pendingSave()?.mimeType ?: settings.spec.mimeType` — had never
|
||||
* taken its left-hand side. Its comment records what the line is for:
|
||||
*
|
||||
* > a retry offered after a failed save opens the dialog with the type its first attempt used —
|
||||
* > the cast answered null for a `Failed`, and the fallback below is the current picker, which a
|
||||
* > reattached job never set.
|
||||
*
|
||||
* So the untested half is the fix, and the tested half is the fallback it was added to stop being
|
||||
* used.
|
||||
*
|
||||
* ## This revises a named exemption, deliberately
|
||||
*
|
||||
* `FailedSaveRetryTest`'s KDoc lists this line under "Not asserted here, so each is a decision
|
||||
* rather than an omission":
|
||||
*
|
||||
* > It lives in the entry point, above the `ScreenContent` seam, and reaching it needs a real
|
||||
* > ViewModel inside a composition.
|
||||
*
|
||||
* That was true when written. `AdaptiveShellTest` (#173) then established exactly that capability,
|
||||
* and #200 added the two `ShadowActivity` mechanics that let a test read what a launcher launched.
|
||||
* The reason the exemption gave no longer holds, so the exemption is withdrawn rather than left to
|
||||
* be taken at face value — the same shape as #141 revising #84's boundary. That KDoc is corrected
|
||||
* in this change.
|
||||
*
|
||||
* ## Why the job is reattached rather than run
|
||||
*
|
||||
* The screen composes its own ViewModel through `viewModel()`, so nothing can be injected into it.
|
||||
* A job finished before the composition is the one route to a `Converted` state carrying output
|
||||
* `Data` this test chose — and it is also the case the line exists for, since a reattached job's
|
||||
* spec "was never in these settings at all".
|
||||
*/
|
||||
@UnstableApi
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
class RetrySaveMimeTest {
|
||||
|
||||
@get:Rule
|
||||
val composeRule = createAndroidComposeRule<ComponentActivity>()
|
||||
|
||||
private lateinit var app: Application
|
||||
private lateinit var staged: File
|
||||
|
||||
@Before
|
||||
fun setUp() {
|
||||
app = RuntimeEnvironment.getApplication()
|
||||
ConversionDependencies.probe = { _, _ -> InputProbe() }
|
||||
staged = OutputPublisher(app).createStagingFile("holiday.mkv").apply { writeBytes(ByteArray(4096)) }
|
||||
}
|
||||
|
||||
@After
|
||||
fun tearDown() = ConversionDependencies.reset()
|
||||
|
||||
@Test
|
||||
fun `the save dialog offers the type the job produced, not the one the picker is showing`() {
|
||||
finishAJobProducing(JOB_MIME_TYPE)
|
||||
composeRule.setContent { ConverterScreen() }
|
||||
composeRule.waitForIdle()
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.SAVE_FILE).performScrollTo().performClick()
|
||||
composeRule.waitForIdle()
|
||||
|
||||
val intent = requireNotNull(shadowOf(composeRule.activity).nextStartedActivityForResult) {
|
||||
"the save dialog was never launched"
|
||||
}.intent
|
||||
assertEquals(Intent.ACTION_CREATE_DOCUMENT, intent.action)
|
||||
assertEquals(JOB_MIME_TYPE, intent.type)
|
||||
// The fixture is only meaningful while the two differ; without this the assertion above
|
||||
// would pass just as well against the fallback.
|
||||
assertNotEquals(
|
||||
"the picker's own type must differ, or this test proves nothing",
|
||||
JOB_MIME_TYPE,
|
||||
OutputFormat.MP4_H265.spec.mimeType,
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* A conversion that finished while nothing was watching, which is what `reattach()` picks up.
|
||||
*
|
||||
* `SucceedingWorkerFactory` reports this output `Data` for whatever is enqueued, so the job
|
||||
* lands `SUCCEEDED` carrying a staged path that exists — the two things `Reattachment.choose`
|
||||
* requires of a finished job.
|
||||
*/
|
||||
private fun finishAJobProducing(mimeType: String) {
|
||||
installTestWorkManager(
|
||||
app,
|
||||
workDataOf(
|
||||
ConversionWorker.KEY_OUTPUT_PATH to staged.absolutePath,
|
||||
ConversionWorker.KEY_SUGGESTED_NAME to "holiday.mkv",
|
||||
ConversionWorker.KEY_MIME_TYPE to mimeType,
|
||||
),
|
||||
)
|
||||
WorkManager.getInstance(app).enqueue(
|
||||
ConversionWorker.request(
|
||||
inputUri = Uri.parse("content://test/holiday.mkv"),
|
||||
displayName = "holiday.mkv",
|
||||
sizeBytes = 4_096L,
|
||||
),
|
||||
).result.get()
|
||||
}
|
||||
|
||||
private companion object {
|
||||
/** Matroska, against the MP4 the picker defaults to. */
|
||||
const val JOB_MIME_TYPE = "video/x-matroska"
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,147 @@
|
||||
package org.libremediaconverter.convert
|
||||
|
||||
import android.app.Application
|
||||
import android.net.Uri
|
||||
import androidx.media3.common.util.UnstableApi
|
||||
import androidx.work.WorkManager
|
||||
import androidx.work.workDataOf
|
||||
import kotlinx.coroutines.Dispatchers
|
||||
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.join.JoinState
|
||||
import org.libremediaconverter.join.JoinViewModel
|
||||
import org.libremediaconverter.model.InputProbe
|
||||
import org.libremediaconverter.work.ConcatWorker
|
||||
import org.libremediaconverter.work.ConversionWorker
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
import org.robolectric.RuntimeEnvironment
|
||||
|
||||
/**
|
||||
* An answer that arrives after the screen has moved on does nothing.
|
||||
*
|
||||
* Four refusal arms, cold before this file:
|
||||
*
|
||||
* ```
|
||||
* convert/ConversionViewModel.kt:513 currentInput() ?: return
|
||||
* convert/ConversionViewModel.kt:600 pendingSave() ?: return
|
||||
* join/JoinViewModel.kt:316 (as? Ready)?.inputs ?: return
|
||||
* join/JoinViewModel.kt:390 pendingSave() ?: return
|
||||
* ```
|
||||
*
|
||||
* They are not merely defensive. `ConverterScreen.kt:91` wires `convert()` to the
|
||||
* **POST_NOTIFICATIONS result**, and `:83` wires `save()` to the CreateDocument result — so both
|
||||
* are entered by a system callback rather than by a tap, and a result redelivered after process
|
||||
* death arrives at a brand-new ViewModel sitting on `Idle`.
|
||||
*
|
||||
* ## The production change that came with this
|
||||
*
|
||||
* `currentInput()` used to answer for `Converting`, `Waiting` and `Converted` as well as `Ready`.
|
||||
* Those arms were unreachable by tapping Convert but reachable through that permission callback,
|
||||
* and reaching one enqueued a **second** job over a live one — `activeWorkId` overwritten, the
|
||||
* first job still running with an orphaned notification and nothing holding its id.
|
||||
*
|
||||
* #202 decided to narrow rather than to test it as it stood, because a test written against the old
|
||||
* shape would have frozen the double-enqueue as intended behaviour. `JoinViewModel.join()` has been
|
||||
* `(_state.value as? JoinState.Ready)?.inputs ?: return` all along; the two screens are the same
|
||||
* shape and only one was over-general.
|
||||
*/
|
||||
@UnstableApi
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
class StaleLauncherResultTest {
|
||||
|
||||
private lateinit var app: Application
|
||||
private lateinit var workManager: WorkManager
|
||||
private lateinit var staged: java.io.File
|
||||
|
||||
@Before
|
||||
fun setUp() {
|
||||
app = RuntimeEnvironment.getApplication()
|
||||
val publisher = RecordingPublisher(app)
|
||||
ConversionDependencies.publisher = { publisher }
|
||||
ConversionDependencies.probe = { _, _ -> InputProbe() }
|
||||
// A real staged file, because a SUCCEEDED job with no output path maps to Failed rather
|
||||
// than Converted -- and Converted is the state this file's second case has to reach.
|
||||
staged = publisher.createStagingFile("holiday.mp4").apply { writeBytes(ByteArray(4096)) }
|
||||
installTestWorkManager(
|
||||
app,
|
||||
workDataOf(
|
||||
ConversionWorker.KEY_OUTPUT_PATH to staged.absolutePath,
|
||||
ConversionWorker.KEY_SUGGESTED_NAME to "holiday.mp4",
|
||||
ConversionWorker.KEY_MIME_TYPE to "video/mp4",
|
||||
),
|
||||
)
|
||||
workManager = WorkManager.getInstance(app)
|
||||
}
|
||||
|
||||
@After
|
||||
fun tearDown() = ConversionDependencies.reset()
|
||||
|
||||
@Test
|
||||
fun `a permission answer arriving on an empty screen enqueues nothing`() {
|
||||
val viewModel = ConversionViewModel(app, Dispatchers.Unconfined)
|
||||
awaitState(viewModel.state, "Idle") { it is ConversionState.Idle }
|
||||
|
||||
viewModel.convert()
|
||||
|
||||
assertEquals(ConversionState.Idle, viewModel.state.value)
|
||||
assertEquals("nothing may be enqueued for a file that is not there", 0, conversionJobs())
|
||||
}
|
||||
|
||||
/**
|
||||
* The narrowing itself: a permission answer that arrives while a conversion is already running
|
||||
* must not start a second one.
|
||||
*
|
||||
* Reached by converting once — the synchronous test WorkManager finishes it inline, so the
|
||||
* screen is `Converted`, which is one of the three arms `currentInput()` used to answer for.
|
||||
* Calling `convert()` again from there is precisely what the permission callback can do.
|
||||
*/
|
||||
@Test
|
||||
fun `a permission answer arriving after the job finished does not start a second one`() {
|
||||
val viewModel = ConversionViewModel(app, Dispatchers.Unconfined)
|
||||
viewModel.onInputPicked(Uri.parse("content://test/holiday.mkv"))
|
||||
awaitState(viewModel.state, "Ready") { it is ConversionState.Ready }
|
||||
viewModel.convert()
|
||||
val converted = awaitState(viewModel.state, "Converted") { it is ConversionState.Converted }
|
||||
assertEquals("the fixture needs exactly one job to start with", 1, conversionJobs())
|
||||
|
||||
viewModel.convert()
|
||||
|
||||
assertEquals("a second job must not be enqueued over the first", 1, conversionJobs())
|
||||
assertEquals("and the screen must not move", converted, viewModel.state.value)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a save answer arriving on an empty screen does nothing`() {
|
||||
val viewModel = ConversionViewModel(app, Dispatchers.Unconfined)
|
||||
awaitState(viewModel.state, "Idle") { it is ConversionState.Idle }
|
||||
|
||||
viewModel.save(DESTINATION)
|
||||
|
||||
assertEquals(ConversionState.Idle, viewModel.state.value)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a join answer arriving on an empty screen enqueues nothing`() {
|
||||
val viewModel = JoinViewModel(app, Dispatchers.Unconfined)
|
||||
awaitState(viewModel.state, "Idle") { it is JoinState.Idle }
|
||||
|
||||
viewModel.join()
|
||||
viewModel.save(DESTINATION)
|
||||
|
||||
assertEquals(JoinState.Idle, viewModel.state.value)
|
||||
assertEquals(0, joinJobs())
|
||||
}
|
||||
|
||||
private fun conversionJobs() = jobsTagged(ConversionWorker::class.java.name)
|
||||
|
||||
private fun joinJobs() = jobsTagged(ConcatWorker::class.java.name)
|
||||
|
||||
private fun jobsTagged(tag: String) = workManager.getWorkInfosByTag(tag).get().size
|
||||
|
||||
private companion object {
|
||||
val DESTINATION: Uri = Uri.parse("content://test/destination.mp4")
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,127 @@
|
||||
package org.libremediaconverter.ffmpeg
|
||||
|
||||
import com.arthenica.ffmpegkit.ReturnCode
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Test
|
||||
|
||||
/**
|
||||
* What a finished FFmpegKit session means, for both engines at once.
|
||||
*
|
||||
* `FFmpegEngine` and `ConcatEngine` each carried their own copy of this `when`, and the copies had
|
||||
* drifted: one preferred the fail stack trace and fell back to the log tail, the other only ever
|
||||
* read the log tail. Neither was tested, because both live inside a callback handed to `FFmpegKit`,
|
||||
* which does not run on the JVM — so nothing could see that the two disagreed.
|
||||
*
|
||||
* **JVM-safe, verified rather than assumed.** `javap` over the committed AAR's runtime jar shows
|
||||
* `ReturnCode(int)` as a plain public constructor with `SUCCESS`/`CANCEL` int constants and pure
|
||||
* static `isSuccess`/`isCancel`; its `<clinit>` is constant initialisation and loads no native
|
||||
* library.
|
||||
*
|
||||
* The unification is #203's decision, so the tests pin it as one: a join failure now carries the
|
||||
* stack trace a conversion failure always did, while the two prefixes stay distinct.
|
||||
*/
|
||||
class SessionOutcomeTest {
|
||||
|
||||
@Test
|
||||
fun `a return code of zero is success`() {
|
||||
assertEquals(SessionOutcome.Success, outcome(ReturnCode(ReturnCode.SUCCESS)))
|
||||
}
|
||||
|
||||
/**
|
||||
* Cancellation is a separate outcome from failure, and the distinction is the point: the engines
|
||||
* resume the continuation *cancelled* rather than exceptionally, so a user who pressed Cancel
|
||||
* does not get an error card.
|
||||
*/
|
||||
@Test
|
||||
fun `a return code of 255 is a cancellation, not a failure`() {
|
||||
assertEquals(SessionOutcome.Cancelled, outcome(ReturnCode(ReturnCode.CANCEL)))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `any other return code fails, and the sentence carries the number`() {
|
||||
val failed = outcome(ReturnCode(1), stackTrace = "boom") as SessionOutcome.Failed
|
||||
|
||||
assertTrue("the code belongs in the message, got: ${failed.message}", failed.message.contains("(1)"))
|
||||
}
|
||||
|
||||
/**
|
||||
* The half that was different between the two engines before #203, now the same in both.
|
||||
*/
|
||||
@Test
|
||||
fun `the stack trace is preferred over the log tail`() {
|
||||
val failed = outcome(ReturnCode(1), stackTrace = "the real cause", logTail = "…noise…")
|
||||
as SessionOutcome.Failed
|
||||
|
||||
assertTrue(failed.message.contains("the real cause"))
|
||||
assertTrue("the log tail must not be appended as well", !failed.message.contains("noise"))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a blank stack trace falls back to the log tail`() {
|
||||
val blank = outcome(ReturnCode(1), stackTrace = " ", logTail = "the last few lines") as SessionOutcome.Failed
|
||||
val absent = outcome(ReturnCode(1), stackTrace = null, logTail = "the last few lines") as SessionOutcome.Failed
|
||||
|
||||
assertTrue(blank.message.contains("the last few lines"))
|
||||
assertTrue("a null stack trace is a blank one", absent.message.contains("the last few lines"))
|
||||
}
|
||||
|
||||
/**
|
||||
* Both sources empty still has to produce a sentence. A message ending in a dangling colon is
|
||||
* thin, but it is what the user gets when FFmpeg said nothing at all, and it must not be an
|
||||
* exception on the way to the screen.
|
||||
*/
|
||||
@Test
|
||||
fun `a failure with nothing to say still names the code`() {
|
||||
val failed = outcome(ReturnCode(1), stackTrace = null, logTail = null) as SessionOutcome.Failed
|
||||
|
||||
assertEquals("FFmpeg failed (1): ", failed.message)
|
||||
}
|
||||
|
||||
/**
|
||||
* `getReturnCode()` is nullable and a session killed before it reported anything has none.
|
||||
* Neither success nor cancellation, so it fails — and the sentence says so rather than throwing.
|
||||
*/
|
||||
@Test
|
||||
fun `a session with no return code at all fails`() {
|
||||
val failed = outcome(null, logTail = "whatever was logged") as SessionOutcome.Failed
|
||||
|
||||
assertTrue("got: ${failed.message}", failed.message.startsWith("FFmpeg failed (null): "))
|
||||
}
|
||||
|
||||
/**
|
||||
* Unifying the *strategy* must not unify the *sentence*: the two engines describe different
|
||||
* jobs, and a join that reports "FFmpeg failed" is a worse message than the one it replaced.
|
||||
*/
|
||||
@Test
|
||||
fun `each engine keeps its own prefix`() {
|
||||
val join = sessionOutcome(ReturnCode(1), "Joining", { "cause" }, { null }) as SessionOutcome.Failed
|
||||
|
||||
assertTrue(join.message.startsWith("Joining failed (1): "))
|
||||
}
|
||||
|
||||
/**
|
||||
* Neither message source is read unless the outcome is a failure.
|
||||
*
|
||||
* They are calls onto a native session, and reading them on the happy path is work every
|
||||
* successful conversion would do for nothing — which the shape this replaced did not, since it
|
||||
* read them inside the `else` branch. That is why the parameters are lambdas, and this is what
|
||||
* would notice if they stopped being.
|
||||
*/
|
||||
@Test
|
||||
fun `a session that succeeded reads neither the stack trace nor the log`() {
|
||||
var reads = 0
|
||||
fun counted(): String? {
|
||||
reads++
|
||||
return null
|
||||
}
|
||||
|
||||
sessionOutcome(ReturnCode(ReturnCode.SUCCESS), "FFmpeg", ::counted, ::counted)
|
||||
sessionOutcome(ReturnCode(ReturnCode.CANCEL), "FFmpeg", ::counted, ::counted)
|
||||
|
||||
assertEquals("neither source may be touched unless the session failed", 0, reads)
|
||||
}
|
||||
|
||||
private fun outcome(rc: ReturnCode?, stackTrace: String? = null, logTail: String? = null) =
|
||||
sessionOutcome(rc, "FFmpeg", { stackTrace }, { logTail })
|
||||
}
|
||||
@@ -0,0 +1,89 @@
|
||||
package org.libremediaconverter.ui.theme
|
||||
|
||||
import androidx.compose.material3.ColorScheme
|
||||
import androidx.compose.material3.MaterialTheme
|
||||
import androidx.compose.ui.test.junit4.v2.createComposeRule
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertNotEquals
|
||||
import org.junit.Rule
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
import org.robolectric.annotation.Config
|
||||
|
||||
/**
|
||||
* The theme called the way the app calls it: with no arguments at all.
|
||||
*
|
||||
* [ThemeColorSchemeTest] resolves every branch of the `when` and always passes `darkTheme`
|
||||
* explicitly, so the `$default` bridge is never entered and **`isSystemInDarkTheme()` is never
|
||||
* called**. `MainActivity.kt:79` is its only default-argument caller and does not execute on the
|
||||
* JVM, which left the app's actual call shape the one nothing exercised —
|
||||
* `LibreMediaConverterTheme` reported `mi=21, mb=6, cb=12` at method level.
|
||||
*
|
||||
* ## Not #68
|
||||
*
|
||||
* #68 is about the two **unreachable** arms, `DarkColorScheme` and `LightColorScheme`, which cannot
|
||||
* run because `dynamicColor` is always `true` and nothing can flip it. That is an open product
|
||||
* decision. This is the reachable half — whether the default follows the system — and closing it
|
||||
* does not close that.
|
||||
*
|
||||
* ## Why the assertion compares schemes rather than reading a number
|
||||
*
|
||||
* A luminance threshold would be a guess about the device palette. What is asserted instead is that
|
||||
* the no-argument call resolves to **the same scheme** an explicit `darkTheme` of the matching
|
||||
* value does, and a different one from its opposite. That holds whatever palette the platform
|
||||
* hands back, and it is exactly the claim: the default reads the system rather than picking a side.
|
||||
*
|
||||
* Both schemes are resolved in one composition because `setContent` may be called once per test.
|
||||
*/
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
class ThemeFollowsSystemTest {
|
||||
|
||||
@get:Rule
|
||||
val composeRule = createComposeRule()
|
||||
|
||||
@Test
|
||||
@Config(qualifiers = "+night")
|
||||
fun `with no arguments the theme follows a system in dark mode`() {
|
||||
val resolved = resolve()
|
||||
|
||||
assertEquals("the default must resolve what darkTheme = true does", resolved.dark, resolved.bare)
|
||||
assertNotEquals(resolved.light, resolved.bare)
|
||||
}
|
||||
|
||||
@Test
|
||||
@Config(qualifiers = "+notnight")
|
||||
fun `with no arguments the theme follows a system in light mode`() {
|
||||
val resolved = resolve()
|
||||
|
||||
assertEquals("the default must resolve what darkTheme = false does", resolved.light, resolved.bare)
|
||||
assertNotEquals(resolved.dark, resolved.bare)
|
||||
}
|
||||
|
||||
/**
|
||||
* The three colours are read together as one value, because any single one could coincide
|
||||
* between the two schemes on some palette while the schemes themselves differ. Background is
|
||||
* what dark mode is chiefly about; primary and surface are along to make a coincidence
|
||||
* implausible rather than merely unlikely.
|
||||
*/
|
||||
private data class Fingerprint(val background: Long, val primary: Long, val surface: Long)
|
||||
|
||||
private fun ColorScheme.fingerprint() =
|
||||
Fingerprint(background.value.toLong(), primary.value.toLong(), surface.value.toLong())
|
||||
|
||||
private class Resolved(val bare: Fingerprint, val dark: Fingerprint, val light: Fingerprint)
|
||||
|
||||
private fun resolve(): Resolved {
|
||||
lateinit var bare: Fingerprint
|
||||
lateinit var dark: Fingerprint
|
||||
lateinit var light: Fingerprint
|
||||
composeRule.setContent {
|
||||
// No arguments — the call MainActivity makes, and the one nothing exercised.
|
||||
LibreMediaConverterTheme { bare = MaterialTheme.colorScheme.fingerprint() }
|
||||
LibreMediaConverterTheme(darkTheme = true) { dark = MaterialTheme.colorScheme.fingerprint() }
|
||||
LibreMediaConverterTheme(darkTheme = false) { light = MaterialTheme.colorScheme.fingerprint() }
|
||||
}
|
||||
composeRule.waitForIdle()
|
||||
return Resolved(bare, dark, light)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user