From 306b8bb150377d4d041786c7337e80d7b218d705 Mon Sep 17 00:00:00 2001 From: Kgothatso Ngako Date: Fri, 2 Oct 2026 20:20:12 +0200 Subject: [PATCH] test(chapters): the five gaps a coverage pass found, and two vacuous tests A pass counting tests per file rather than per feature found five pieces of this work with none. One of them mattered. MAX_PROPOSAL_BYTES was enforced and never tested, on either side. The inbound check is the half that counts -- the proposal is the one place a remote party decides how much work everybody else does -- and it had nothing at all. Four tests now, propose and accept, bytes and count, in SignedGroupKeyStateTest where the adversarial inbound tests belong. The count cap's accept side was untested too, although the propose-side test's own comment already claimed otherwise. Two of those tests passed vacuously as first written, which is the part worth keeping. acceptProposal rebuilds every event from its own fields under the room's path and drops the whole proposal if an id does not match, so a hand-built event with an invented id is refused whatever its size -- and an assertNull then stays green with the guard deleted. authoredProposal fixes it by taking its events from real proposals, so their ids are the ones the manager computes, and a control asserts that a well-formed proposal of the same shape does open a session. That control failed on its first run, which is how the vacuity was found; commenting the guard out now fails exactly the test named after it. UploadChaptersScreen had no UI test, against a project with fifteen runDesktopComposeUiTest files that tests other screens this way. Ten now: the cost of an upload stated before the picker opens, the index and name per row, every per-row reason rendering, and the FAB counting only what it would actually propose rather than how many files were picked. canPropose, kibibytes and the chapter summary had none either, each being read only by a composable. Eleven, five and five. Still open, and not closable here: the three FilePicker actuals need instrumentation this project does not have, and on iOS a host that can compile them. 1030 tests, no failures; m3Audit's budgets all met. Co-Authored-By: Claude Opus 5 --- .../mantra/compose/text/KibibytesTest.kt | 50 ++++ .../mantra/compose/text/ProposedEventTest.kt | 106 ++++++++ .../ui/view/state/UploadChaptersStateTest.kt | 167 ++++++++++++ .../managers/SignedGroupKeyStateTest.kt | 163 ++++++++++++ .../composable/UploadChaptersScreenJvmTest.kt | 247 ++++++++++++++++++ docs/chapter-uploads.md | 33 ++- 6 files changed, 765 insertions(+), 1 deletion(-) create mode 100644 composeApp/src/commonTest/kotlin/press/mantra/compose/text/KibibytesTest.kt create mode 100644 composeApp/src/commonTest/kotlin/press/mantra/compose/ui/view/state/UploadChaptersStateTest.kt create mode 100644 composeApp/src/jvmTest/kotlin/press/mantra/compose/ui/composable/UploadChaptersScreenJvmTest.kt diff --git a/composeApp/src/commonTest/kotlin/press/mantra/compose/text/KibibytesTest.kt b/composeApp/src/commonTest/kotlin/press/mantra/compose/text/KibibytesTest.kt new file mode 100644 index 00000000..195b9c1f --- /dev/null +++ b/composeApp/src/commonTest/kotlin/press/mantra/compose/text/KibibytesTest.kt @@ -0,0 +1,50 @@ +package press.mantra.compose.text + +import kotlin.test.Test +import kotlin.test.assertEquals + +/** + * The figure in "19.9 KiB", which two screens show next to each other. + * + * Whole kibibytes would print a 20,473-byte chapter and its 21,226-byte limit + * as the same "19 KiB", which reads as a form refusing something it says is + * within the limit. And truncating rather than rounding is the whole point: a + * figure shown as under the limit has to be under it. + */ +class KibibytesTest { + + @Test + fun `whole kibibytes`() { + assertEquals("0.0", kibibytes(0)) + assertEquals("1.0", kibibytes(1024)) + assertEquals("64.0", kibibytes(64 * 1024)) + } + + @Test + fun `one decimal place`() { + assertEquals("1.5", kibibytes(1024 + 512)) + assertEquals("19.9", kibibytes(20473)) + assertEquals("20.7", kibibytes(21226)) + } + + @Test + fun `two figures a form shows together do not collapse into one`() { + // The reason this is not whole kibibytes. Both of these round to 19. + assertEquals(true, kibibytes(20473) != kibibytes(21226)) + } + + @Test + fun `it truncates rather than rounds`() { + // 1023 bytes is 0.999 KiB. Rounding would print "1.0", which is a figure + // over the value it describes -- and when the value is being compared to + // a cap, that is the direction that lies. + assertEquals("0.9", kibibytes(1023)) + assertEquals("63.9", kibibytes(64 * 1024 - 1)) + } + + @Test + fun `under a tenth of a kibibyte reads as zero point zero`() { + // Honest rather than pretty: a 50-byte chapter is not 0.1 KiB. + assertEquals("0.0", kibibytes(50)) + } +} diff --git a/composeApp/src/commonTest/kotlin/press/mantra/compose/text/ProposedEventTest.kt b/composeApp/src/commonTest/kotlin/press/mantra/compose/text/ProposedEventTest.kt index 9ecddf7e..c8215240 100644 --- a/composeApp/src/commonTest/kotlin/press/mantra/compose/text/ProposedEventTest.kt +++ b/composeApp/src/commonTest/kotlin/press/mantra/compose/text/ProposedEventTest.kt @@ -3,6 +3,9 @@ package press.mantra.compose.text import com.vitorpamplona.quartz.nip01Core.core.Event import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertTrue +import press.mantra.compose.nostr.nip30303.ChapterEvent +import press.mantra.compose.nostr.nip30303.ChunkEvent /** * How an event a group signs is said to the member signing it. @@ -23,6 +26,109 @@ class ProposedEventTest { assertEquals("An article", summary.detail) } + /** + * A chapter is the longest thing a group is asked to sign, and the one whose + * summary cannot be the thing itself. + * + * So the summary stands in for it: the name, how many words, and how many + * chunks came out of the text -- which is the rest of what the batch + * contains and so the rest of what is being agreed to. + */ + @Test + fun `a chapter is named, measured, and counted`() { + val summary = ProposedEvent.summarize(chapter(name = "The wreck"), chunkCount = 4) + + assertEquals("New chapter", summary.label) + assertEquals("The wreck · 12 words · 4 chunks", summary.detail) + } + + @Test + fun `one chunk is said in the singular`() { + val summary = ProposedEvent.summarize(chapter(), chunkCount = 1) + + assertTrue(summary.detail.endsWith("1 chunk"), summary.detail) + } + + /** + * The count comes from the batch, not from the text. + * + * It used to be derived by re-splitting the chapter's own markdown, which is + * a count of the batch dressed as a fact about the text: across a splitter + * change the figure a member reads would disagree with the events they are + * approving, because the batch was built by the proposer's splitter and + * counted by the reader's. This asserts the summary reports what it was + * handed -- the text here splits into two, and the batch says five. + */ + @Test + fun `the chunk count is the batch's, not a re-reading of the text`() { + val summary = ProposedEvent.summarize( + chapter(originalText = "One.\n\nTwo."), + chunkCount = 5, + ) + + assertTrue(summary.detail.endsWith("5 chunks"), summary.detail) + } + + @Test + fun `with no batch to count, a chapter says nothing about chunks`() { + // A transcript line, or a signed event on its own: there is no batch to + // ask, and guessing is what this parameter exists to stop. + val summary = ProposedEvent.summarize(chapter(name = "The wreck")) + + assertEquals("The wreck · 12 words", summary.detail) + } + + @Test + fun `a chunk is said as its own words, in its own place`() { + // The chunks travel in the same batch as their chapter and are read + // under it, so each has to be checkable on its own. + val summary = ProposedEvent.summarize( + ChunkEvent.build( + chapterId = "d".repeat(64), + text = "It began badly.", + index = 2, + wordCount = 3, + characterCount = 15, + ).let { template -> + Event( + id = "a".repeat(64), + pubKey = "b".repeat(64), + createdAt = template.createdAt, + kind = template.kind, + tags = template.tags, + content = template.content, + sig = "", + ) + } + ) + + assertTrue(summary.detail.contains("It began badly."), summary.detail) + } + + /** A chapter as the group would be asked to sign it. */ + private fun chapter( + name: String = "The wreck", + originalText: String = "One two three.\n\nFour five six.\n\nSeven eight nine.\n\nTen eleven twelve.", + ): Event { + val template = ChapterEvent.build( + artifactVersionId = "e".repeat(64), + name = name, + originalText = originalText, + index = 0, + wordCount = Markdown.wordCount(originalText), + characterCount = Markdown.characterCount(originalText), + ) + return Event( + id = "a".repeat(64), + pubKey = "b".repeat(64), + createdAt = template.createdAt, + kind = template.kind, + tags = template.tags, + content = template.content, + sig = "", + ) + } + private fun event( kind: Int, tags: Array> = emptyArray(), diff --git a/composeApp/src/commonTest/kotlin/press/mantra/compose/ui/view/state/UploadChaptersStateTest.kt b/composeApp/src/commonTest/kotlin/press/mantra/compose/ui/view/state/UploadChaptersStateTest.kt new file mode 100644 index 00000000..bafb8d18 --- /dev/null +++ b/composeApp/src/commonTest/kotlin/press/mantra/compose/ui/view/state/UploadChaptersStateTest.kt @@ -0,0 +1,167 @@ +package press.mantra.compose.ui.view.state + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue +import press.mantra.compose.database.model.ChatRoom +import press.mantra.compose.database.model.MantraArtifact +import press.mantra.compose.database.model.MantraArtifactVersion +import press.mantra.compose.database.model.intermdiate.LocalChatRoom + +/** + * What the upload screen's button is allowed to offer. + * + * `canPropose` is read only by the FAB, which is exactly why it is worth a test + * of its own: there is no other caller to notice if one of its four conditions + * goes missing, and each of them exists because offering the action without it + * would ask the group for something that cannot happen. + */ +class UploadChaptersStateTest { + + private val room = LocalChatRoom( + chatRoom = ChatRoom( + id = "room", + userPublicKey = "", + subject = "#Ekklesia", + description = "", + mlsGroupState = null, + ) + ) + + private val artifact = MantraArtifact( + id = "artifactId", + publicKey = "author", + name = "To Kill a Mockingbird", + url = "https://example.test/bird", + visibility = "private", + dialectId = "dialectId", + license = "cc", + chatRoomId = "room", + signature = "", + ) + + private val version = MantraArtifactVersion( + id = "artifactVersionId", + artifactId = "artifactId", + publicKey = "author", + versionLabel = "1.0", + chatRoomId = "room", + signature = "", + ) + + private fun row( + id: String, + problem: UploadProblem? = null, + sessionId: String? = null, + ) = PickedChapter( + id = id, + fileName = "$id.md", + name = id, + markdown = "A paragraph.", + chunks = listOf("A paragraph."), + wordCount = 2, + characterCount = 12, + proposalBytes = 1000, + problem = problem, + sessionId = sessionId, + ) + + private fun state( + chapters: List = listOf(row("one")), + canSign: Boolean = true, + artifactVersion: MantraArtifactVersion? = version, + isProposing: Boolean = false, + ) = UploadChaptersUIState.Loaded( + localChatRoom = room, + artifact = artifact, + artifactVersion = artifactVersion, + canSign = canSign, + chapters = chapters, + isProposing = isProposing, + ) + + @Test + fun `a clean row with a version and a key can be proposed`() { + assertTrue(state().canPropose) + } + + @Test + fun `nothing picked is nothing to propose`() { + assertFalse(state(chapters = emptyList()).canPropose) + } + + @Test + fun `a group with no shared key cannot sign a chapter into existence`() { + assertFalse(state(canSign = false).canPropose) + } + + @Test + fun `an artifact with no version has nothing for a chapter to hang off`() { + assertFalse(state(artifactVersion = null).canPropose) + } + + @Test + fun `a pass already running is not offered a second time`() { + // The guard against a double tap proposing every chapter twice, which + // nothing could retract. + assertFalse(state(isProposing = true).canPropose) + } + + @Test + fun `a blocking problem takes its own row out and leaves the others`() { + val chapters = listOf( + row("good"), + row("binary", problem = UploadProblem.NotText), + row("also-good"), + ) + + assertEquals(listOf("good", "also-good"), state(chapters).proposable.map { it.id }) + assertTrue(state(chapters).canPropose) + } + + @Test + fun `every row blocked is nothing to propose`() { + val chapters = listOf( + row("binary", problem = UploadProblem.NotText), + row("empty", problem = UploadProblem.Empty), + ) + + assertEquals(emptyList(), state(chapters).proposable) + assertFalse(state(chapters).canPropose) + } + + @Test + fun `a warning is not a block`() { + // A book can repeat a page, so a duplicate is said and not refused. + val chapters = listOf(row("copy", problem = UploadProblem.Duplicate("The Wreck"))) + + assertEquals(listOf("copy"), state(chapters).proposable.map { it.id }) + assertTrue(state(chapters).canPropose) + } + + @Test + fun `a failed proposal is still proposable, because a retry is a new proposal`() { + val chapters = listOf(row("tried", problem = UploadProblem.ProposalFailed)) + + assertEquals(listOf("tried"), state(chapters).proposable.map { it.id }) + assertTrue(state(chapters).canPropose) + } + + @Test + fun `a row already proposed is neither proposable nor forgotten`() { + val chapters = listOf(row("sent", sessionId = "session"), row("waiting")) + val loaded = state(chapters) + + assertEquals(listOf("waiting"), loaded.proposable.map { it.id }) + assertEquals(listOf("sent"), loaded.proposed.map { it.id }) + } + + @Test + fun `everything proposed leaves nothing to offer`() { + val chapters = listOf(row("sent", sessionId = "session")) + + assertEquals(emptyList(), state(chapters).proposable) + assertFalse(state(chapters).canPropose) + } +} diff --git a/composeApp/src/jvmTest/kotlin/press/mantra/compose/managers/SignedGroupKeyStateTest.kt b/composeApp/src/jvmTest/kotlin/press/mantra/compose/managers/SignedGroupKeyStateTest.kt index 05c24f91..8c09e3f0 100644 --- a/composeApp/src/jvmTest/kotlin/press/mantra/compose/managers/SignedGroupKeyStateTest.kt +++ b/composeApp/src/jvmTest/kotlin/press/mantra/compose/managers/SignedGroupKeyStateTest.kt @@ -1207,6 +1207,169 @@ class SignedGroupKeyStateTest { assertEquals(before.map { it.nonceRandom }, other.items(session.id).map { it.nonceRandom }) } + /** + * The byte cap, on both sides, for the same reason the count cap has both. + * + * `MAX_BATCH_SIZE` bounds how many events a batch holds; `MAX_PROPOSAL_BYTES` + * bounds how large they are, and a batch can pass the first and fail the + * second -- sixty-four dialects are a few kilobytes and one chapter is + * eighty. Before the cap existed the failure was silent: the proposal went + * out, the group event was over size, and the session stalled with nothing + * to blame. + */ + @Test + fun `a proposal over the byte cap is refused when proposed`() = runBlocking { + val creator = device(members[0], signerIndex = 0) + + // One event, so the count cap cannot be what refuses this. + assertFailsWith { + FrostSigningManager.proposeSigning( + database = creator.db, + localChatRoom = creator.room, + userPublicKey = creator.publicKey, + kind = 1, + tags = emptyArray(), + content = "w".repeat(FrostSigningManager.MAX_PROPOSAL_BYTES + 1) + ) + } + + assertEquals( + emptyList(), + creator.db.frostSigningSessionDao().getSessionsForChatRoom(adminRoomId), + "a refused proposal must leave no session behind" + ) + } + + /** + * And dropped on arrival, which is the half that matters. + * + * The proposal is the one place a remote party decides how much work + * everybody else does. A proposer who could hand every member an arbitrarily + * large batch would have them do unbounded native work and publish an + * unbounded group event from a single message. + * + * **The payload has to be valid in every other way**, or this passes for the + * wrong reason. `acceptProposal` rebuilds each event from its own fields + * under the room's path and drops the whole proposal if any id does not + * match what it claims -- so a hand-built event with an invented id is + * refused whatever its size, and a test using one would stay green with the + * cap deleted. That is how this was first written and what + * [authoredProposal] exists to fix: the events come from real proposals, so + * their ids are the ones the manager computes. + */ + @Test + fun `a proposal over the byte cap is dropped on arrival`() = runBlocking { + val creator = device(members[0], signerIndex = 0) + val other = device(members[1], signerIndex = 1) + val sessionId = "f1".repeat(32) + + // Two events, each legitimately proposed at a third of the cap, so + // together they are over it and apart they are unremarkable. + val third = FrostSigningManager.MAX_PROPOSAL_BYTES / 3 + val oversize = authoredProposal(creator, "a".repeat(third), "b".repeat(third), "c".repeat(third)) + assertTrue( + FrostSigningEvents.encodeProposal(oversize).encodeToByteArray().size > + FrostSigningManager.MAX_PROPOSAL_BYTES, + "the fixture must actually exceed the cap" + ) + + deliverProposal(other, sessionId, oversize) + + assertNull(other.session(sessionId), "an over-size proposal must not open a session") + } + + /** + * The count cap on arrival too, which this file claimed and did not check. + * + * The propose-side test above says in its own comment that the cap "is + * checked on arrival and not only when proposing" -- which was true of the + * code and untrue of the tests. + */ + @Test + fun `a batch over the count cap is dropped on arrival`() = runBlocking { + val creator = device(members[0], signerIndex = 0) + val other = device(members[1], signerIndex = 1) + val sessionId = "f2".repeat(32) + + // Small events, so the byte cap cannot be what refuses this. + val tooMany = authoredProposal( + creator, + *(0..FrostSigningManager.MAX_BATCH_SIZE).map { "event $it" }.toTypedArray() + ) + assertEquals(FrostSigningManager.MAX_BATCH_SIZE + 1, tooMany.size) + + deliverProposal(other, sessionId, tooMany) + + assertNull( + other.session(sessionId), + "a proposal of more events than the cap must not open a session" + ) + } + + /** + * The control for the two tests above. + * + * Both assert that no session is opened, and `acceptProposal` has four other + * reasons to open none -- an unknown key, a ceremony this device was not in, + * an unparseable payload, an id that does not match its fields. Without + * this, either could pass for a reason that has nothing to do with the cap + * it is named after, and the guard could be deleted with both still green. + * + * Which is not hypothetical: written with hand-built ids, both passed and + * this failed. + */ + @Test + fun `a well-formed proposal of the same shape does open a session`() = runBlocking { + val creator = device(members[0], signerIndex = 0) + val other = device(members[1], signerIndex = 1) + val sessionId = "f3".repeat(32) + + deliverProposal(other, sessionId, authoredProposal(creator, "a dialect")) + + assertNotNull( + other.session(sessionId), + "a proposal that breaks no cap must open a session, or the cap tests prove nothing" + ) + } + + /** + * The unsigned events a real proposal produced, ids and all. + * + * Taken from the manager rather than built here, because the id is a hash + * over the group's key at the room's derivation path and the receiving + * device recomputes it. Anything else is refused before a cap is consulted. + */ + private suspend fun authoredProposal(device: Device, vararg contents: String): List = + contents.flatMap { content -> + val session = FrostSigningManager.proposeSigning( + database = device.db, + localChatRoom = device.room, + userPublicKey = device.publicKey, + kind = 1, + tags = emptyArray(), + content = content + ) + device.items(session.id).map { Event.fromJson(it.unsignedEventJson) } + } + + /** Hands [device] a proposal under [sessionId], as a dishonest coordinator would. */ + private suspend fun deliverProposal(device: Device, sessionId: String, events: List) { + FrostSigningManager.processSigningPayload( + database = device.db, + localChatRoom = device.room, + innerEvent = Event( + id = "4".repeat(64), + pubKey = members[0], + createdAt = 1, + kind = FrostSigningEvents.PROPOSAL, + tags = FrostSigningEvents.assembleTags(sessionId, dkgSessionId = ceremonyId), + content = FrostSigningEvents.encodeProposal(events), + sig = "" + ), + userPublicKey = device.publicKey + ) + } + /** Three dialects, distinct enough that an order bug shows up as a wrong name. */ private fun dialects() = listOf( Triple("Sepedi", "ZA", "nso"), diff --git a/composeApp/src/jvmTest/kotlin/press/mantra/compose/ui/composable/UploadChaptersScreenJvmTest.kt b/composeApp/src/jvmTest/kotlin/press/mantra/compose/ui/composable/UploadChaptersScreenJvmTest.kt new file mode 100644 index 00000000..ac2a8411 --- /dev/null +++ b/composeApp/src/jvmTest/kotlin/press/mantra/compose/ui/composable/UploadChaptersScreenJvmTest.kt @@ -0,0 +1,247 @@ +package press.mantra.compose.ui.composable + +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.ui.Modifier +import androidx.compose.ui.test.ComposeUiTest +import androidx.compose.ui.test.ExperimentalTestApi +import androidx.compose.ui.test.assertIsDisplayed +import androidx.compose.ui.test.assertIsEnabled +import androidx.compose.ui.test.assertIsNotEnabled +import androidx.compose.ui.test.onNodeWithContentDescription +import androidx.compose.ui.test.onNodeWithText +import androidx.compose.ui.test.runDesktopComposeUiTest +import kotlin.test.Test +import press.mantra.compose.database.model.ChatRoom +import press.mantra.compose.database.model.MantraArtifact +import press.mantra.compose.database.model.MantraArtifactVersion +import press.mantra.compose.database.model.intermdiate.LocalChatRoom +import press.mantra.compose.repository.ChatRepository +import press.mantra.compose.repository.FrostSigningRepository +import press.mantra.compose.repository.MantraRepository +import press.mantra.compose.ui.composable.widgets.ProvideSnackbarHost +import press.mantra.compose.ui.theme.MantraTheme +import press.mantra.compose.ui.view.state.PickedChapter +import press.mantra.compose.ui.view.state.UploadChaptersUIState +import press.mantra.compose.ui.view.state.UploadProblem + +/** + * What the upload screen says before anything has been proposed. + * + * The screen is a review step, so its whole job is to be read: the cost of the + * upload in front of the picker, a row per file saying where it lands and what + * is wrong with it, and a button that offers only what can actually be + * proposed. None of that is checkable from the view model -- `canPropose` is + * read by the FAB and the per-row reasons are rendered from a `when` -- so it is + * checked here, against a real composition. + */ +@OptIn(ExperimentalTestApi::class) +class UploadChaptersScreenJvmTest { + + private val room = LocalChatRoom( + chatRoom = ChatRoom( + id = "room", + userPublicKey = "", + subject = "#Ekklesia", + description = "", + mlsGroupState = null, + ) + ) + + private val artifact = MantraArtifact( + id = "artifactId", + publicKey = "author", + name = "To Kill a Mockingbird", + url = "https://example.test/bird", + visibility = "private", + dialectId = "dialectId", + license = "cc", + chatRoomId = "room", + signature = "", + ) + + private val version = MantraArtifactVersion( + id = "artifactVersionId", + artifactId = "artifactId", + publicKey = "author", + versionLabel = "1.0", + chatRoomId = "room", + signature = "", + ) + + private fun row( + id: String, + name: String = id, + problem: UploadProblem? = null, + sessionId: String? = null, + ) = PickedChapter( + id = id, + fileName = "$id.md", + name = name, + markdown = "A paragraph.", + chunks = listOf("A paragraph."), + wordCount = 2, + characterCount = 12, + proposalBytes = 1000, + problem = problem, + sessionId = sessionId, + ) + + @Test + fun `before any file is picked, the cost of an upload is already on screen`() = render( + state(chapters = emptyList()) + ) { + // Said before the picker opens, not after: an upload spends the room's + // attention and the person picking files is the only one who can weigh + // it. + onNodeWithText("Every file becomes its own proposal", substring = true).assertIsDisplayed() + onNodeWithText("At most 20 files in one upload.").assertIsDisplayed() + onNodeWithText("No files picked yet.").assertIsDisplayed() + onNodeWithText("Choose files").assertIsDisplayed() + } + + @Test + fun `a picked file shows where it lands and what it will be called`() = render( + state(chapters = listOf(row("the-wreck", name = "The wreck")), baseIndex = 3) + ) { + onNodeWithText("the-wreck.md").assertIsDisplayed() + // Numbered from the version's own next index, not from one. + onNodeWithText("Chapter 3 of the version").assertIsDisplayed() + onNodeWithText("The wreck").assertIsDisplayed() + } + + @Test + fun `the button counts what it would actually propose`() = render( + state( + chapters = listOf( + row("good"), + row("binary", problem = UploadProblem.NotText), + row("also-good"), + ) + ) + ) { + // Two, not three: a file that is not text was never offered. + onNodeWithText("Propose 2 chapters").assertIsEnabled() + onNodeWithText("Not text. A chapter is markdown or plain text.").assertIsDisplayed() + } + + @Test + fun `a group with no shared key is told, and offered nothing`() = render( + state(canSign = false) + ) { + onNodeWithText("This group has no shared key", substring = true).assertIsDisplayed() + onNodeWithText("Propose 1 chapters").assertIsNotEnabled() + } + + @Test + fun `an artifact with no version is told, and offered nothing`() = render( + state(artifactVersion = null) + ) { + onNodeWithText("This artifact has no version", substring = true).assertIsDisplayed() + onNodeWithText("Propose 1 chapters").assertIsNotEnabled() + } + + @Test + fun `every file blocked leaves the button offering nothing`() = render( + state( + chapters = listOf( + row("binary", problem = UploadProblem.NotText), + row("blank", problem = UploadProblem.Empty), + ) + ) + ) { + onNodeWithText("Propose 0 chapters").assertIsNotEnabled() + onNodeWithText("Nothing in it to translate.").assertIsDisplayed() + } + + @Test + fun `a duplicate is said without being refused`() = render( + state(chapters = listOf(row("copy", problem = UploadProblem.Duplicate("The wreck")))) + ) { + onNodeWithText("The same text as The wreck.", substring = true).assertIsDisplayed() + // A book can repeat a page, so it is a warning and the button still works. + onNodeWithText("Propose 1 chapters").assertIsEnabled() + } + + @Test + fun `a chapter too long for one session says how to fix it`() = render( + state(chapters = listOf(row("long", problem = UploadProblem.TooManyChunks(80)))) + ) { + onNodeWithText("80 chunks", substring = true).assertIsDisplayed() + onNodeWithText("Split this file.", substring = true).assertIsDisplayed() + } + + @Test + fun `a proposed row waits, keeps its name, and cannot be removed`() = render( + state(chapters = listOf(row("sent", sessionId = "session"), row("waiting"))) + ) { + onNodeWithText("Waiting for the group to sign").assertIsDisplayed() + // Its chapter is in front of the group: nothing about it is still a + // decision, so there is no way to take it out of the list. + onNodeWithContentDescription("Waiting for the group to sign").assertIsDisplayed() + + // And the button becomes a retry for what is left. + onNodeWithText("Propose the remaining 1").assertIsEnabled() + } + + @Test + fun `a failed row is listed with its reason and can be tried again`() = render( + state( + chapters = listOf( + row("sent", sessionId = "session"), + row("failed", problem = UploadProblem.ProposalFailed), + ) + ) + ) { + onNodeWithText("Proposing this one failed. Try again.").assertIsDisplayed() + // The only place a retry can be aimed from. + onNodeWithText("Propose the remaining 1").assertIsEnabled() + } + + private fun state( + chapters: List = listOf(row("one")), + canSign: Boolean = true, + artifactVersion: MantraArtifactVersion? = version, + baseIndex: Int = 0, + ) = UploadChaptersUIState.Loaded( + localChatRoom = room, + artifact = artifact, + artifactVersion = artifactVersion, + canSign = canSign, + chapters = chapters, + baseIndex = baseIndex, + ) + + /** + * Tall on purpose: the rows are cards in a `LazyColumn` and an assertion + * about one that has not been scrolled to is an assertion about nothing. + */ + private fun render( + uiState: UploadChaptersUIState, + assertions: ComposeUiTest.() -> Unit, + ) = runDesktopComposeUiTest(width = 420, height = 2400) { + setContent { + MantraTheme { + ProvideSnackbarHost { + Box(modifier = Modifier.fillMaxSize()) { + UploadChaptersScreen( + activeUserPublicKey = "a".repeat(64), + artifactId = "artifactId", + chatRoomId = "room", + relayHint = null, + initialUIState = uiState, + mantraRepository = MantraRepository.NO_OP_MANTRA_REPOSITORY, + chatRepository = ChatRepository.NO_OP_CHAT_REPOSITORY, + frostSigningRepository = + FrostSigningRepository.NO_OP_FROST_SIGNING_REPOSITORY, + onNavigateToRouteAndPopUpInclusive = {}, + onNavigateBack = {}, + ) + } + } + } + } + + assertions() + } +} diff --git a/docs/chapter-uploads.md b/docs/chapter-uploads.md index 8b6471f2..f69bd0b0 100644 --- a/docs/chapter-uploads.md +++ b/docs/chapter-uploads.md @@ -987,7 +987,38 @@ and stays retryable, and a second pass after the key arrives proposes all three numbered from zero. The failed pass spent no indices, because it created nothing. -Total: 995 tests, and the counts per phase are in each commit. +Total: 1030 tests, and the counts per phase are in each commit. + +### The five gaps a coverage pass found afterwards + +Counting tests per file rather than per feature turned up five things with none, +and one of them mattered: + +- **`MAX_PROPOSAL_BYTES` was enforced and never tested, on either side.** The + inbound check is the half that counts — the proposal is where a remote party + decides how much work everybody else does — and it had nothing. Now four + tests, propose and accept, bytes and count, in `SignedGroupKeyStateTest` where + the adversarial inbound tests live. The count cap's accept side was missing + too, although the propose-side test's own comment already claimed it. +- **Two of those tests passed vacuously when first written**, which is the part + worth remembering. `acceptProposal` rebuilds every event under the room's path + and drops the proposal if an id does not match its fields, so a hand-built + event with an invented id is refused whatever its size — and an `assertNull` + then passes with the cap deleted. The fix is `authoredProposal`, which takes + its events from real proposals so the ids are the ones the manager computes, + plus a control asserting that a well-formed proposal of the same shape *does* + open a session. The control failed on the first run, which is how this was + found; commenting the guard out now fails exactly the test named after it. +- **`UploadChaptersScreen` had no UI test**, against a project that has fifteen + `runDesktopComposeUiTest` files and tests other screens this way. Ten now: the + cost stated before the picker, the index and name per row, every per-row + reason rendering, and the FAB counting only what it would actually propose. +- **`canPropose`, `kibibytes` and the chapter summary** had none either, being + read only by composables. Eleven, five and five. + +The gap that stays open is the three `FilePicker` actuals, which need +instrumentation this project does not have — and on iOS a host that can compile +them. Rollout needs no flag. The chapter and chunk kinds, their tags and their apply branches are unchanged, so an older build receives an uploaded chapter as the