diff --git a/.github/workflows/android-ci.yaml b/.github/workflows/android-ci.yaml index 01353d7..c1def30 100644 --- a/.github/workflows/android-ci.yaml +++ b/.github/workflows/android-ci.yaml @@ -41,6 +41,12 @@ concurrency: jobs: build: runs-on: ubuntu-latest + # A healthy run is ~6 minutes. Two runs (2026-07-09, 2026-07-28) hung inside + # `./gradlew test` and, with no ceiling of our own, were only killed by + # GitHub's 6-hour maximum-execution limit — burning six runner-hours and, on + # the second one, stranding a pushed tag with no Release (issue #93). 30 + # minutes is generous for a cold Gradle cache and still fails fast on a hang. + timeout-minutes: 30 permissions: contents: write # to push the version-bump commit and tag outputs: @@ -61,14 +67,17 @@ jobs: - id: version name: Compute version env: - # Use the GitHub-provided token to push the bump commit and tag. + # Bakes the GitHub-provided token into the origin URL so the later + # "Push version bump + tag" step can publish the commit and tag. GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | if [ "$GITHUB_REF" = "refs/heads/main" ]; then - # On main: compute the next CalVer from date + run number, - # write it into gradle.properties, commit + tag, then push - # both before Gradle runs. The build then reads the bumped - # values directly from gradle.properties. + # On main: compute the next CalVer from date + run number, write it + # into gradle.properties and commit + tag it LOCALLY. Nothing is + # pushed here — "Push version bump + tag" below does that, but only + # once the tests and lint have passed. Everything Gradle builds in + # between therefore already carries the bumped version, while a run + # that fails or hangs leaves origin untouched. BUILD_DATE=$(date -u +%Y-%m-%d) NEW_NAME="${BUILD_DATE//-/.}.${GITHUB_RUN_NUMBER}" YY=$(date -u +%y) @@ -79,39 +88,21 @@ jobs: sed -i "s/^VERSION_NAME=.*/VERSION_NAME=$NEW_NAME/" gradle.properties sed -i "s/^VERSION_CODE=.*/VERSION_CODE=$NEW_CODE/" gradle.properties + # Committer identity and the tokenised remote live in .git/config, + # which — unlike shell state — does survive into later steps, so the + # push step inherits both without repeating them. git config user.name "github-actions[bot]" git config user.email "41898282+github-actions[bot]@users.noreply.github.com" git remote set-url origin "https://x-access-token:${GH_TOKEN}@github.com/${GITHUB_REPOSITORY}.git" - # If another main push lands between our checkout and our push - # (or between two consecutive bump runs), HEAD is non-fast-forward. - # The naive `git rebase` strategy conflicts when both bump commits - # mutate the same `VERSION_NAME=` line — that's unrecoverable - # without manual resolution. Instead, on rejection we reset hard - # to the latest main, re-apply the bump on top, and retry. The - # bump value is derived from GITHUB_RUN_NUMBER which is stable - # across retries, so we always end up with a unique tag. - apply_bump() { - sed -i "s/^VERSION_NAME=.*/VERSION_NAME=$NEW_NAME/" gradle.properties - sed -i "s/^VERSION_CODE=.*/VERSION_CODE=$NEW_CODE/" gradle.properties - git add gradle.properties - git commit -m ":bookmark: chore: release v$NEW_NAME" - git tag -f "v$NEW_NAME" - } - apply_bump - attempts=0 - until git push origin "HEAD:$GITHUB_REF" "v$NEW_NAME"; do - attempts=$((attempts + 1)) - if [ "$attempts" -ge 5 ]; then - echo "release bump push failed after $attempts attempts" >&2 - exit 1 - fi - echo "release bump push rejected; resetting + re-applying bump ($attempts)…" - git fetch origin "$GITHUB_REF" - git tag -d "v$NEW_NAME" 2>/dev/null || true - git reset --hard "origin/${GITHUB_REF#refs/heads/}" - apply_bump - done + git add gradle.properties + git commit -m ":bookmark: chore: release v$NEW_NAME" + git tag -f "v$NEW_NAME" + + # Hand the tag name to the push step rather than letting it derive + # the name a second time — a run straddling UTC midnight would + # otherwise push a tag that disagrees with the commit it points at. + echo "RELEASE_VERSION_NAME=$NEW_NAME" >> "$GITHUB_ENV" VERSION="$NEW_NAME" else @@ -182,6 +173,40 @@ jobs: run: ./gradlew test --stacktrace - name: Lint run: ./gradlew lint --stacktrace + # The bump commit and tag were created locally by "Compute version"; this + # is where they become public, and deliberately not one step earlier. When + # the push happened up front, a build that hung (2026-07-28, killed at + # GitHub's 6-hour ceiling) left v2026.07.28.223 on origin with no Release + # and no APK behind it, which is a hard stop for F-Droid's reproducible + # build check (issue #93). Now the tag only exists publicly if the tree it + # points at assembled, tested and linted clean. + - name: Push version bump + tag + if: github.ref == 'refs/heads/main' + run: | + # Belt and braces: "Compute version" only exports this on main, and + # this step only runs on main, so an empty value means the two guards + # have drifted apart and we must not push a "v" tag. + if [ -z "$RELEASE_VERSION_NAME" ]; then + echo "::error::Version bump value missing — Compute version did not produce a bump." + exit 1 + fi + + # --atomic so the branch and the tag land together or not at all. + # Plain git applies each refspec independently: a branch update + # rejected as non-fast-forward still lets the brand-new tag through, + # leaving a tag that points at a commit which never reached main — + # #93 again, by a shorter route. + # + # There is deliberately no retry here. main can only move under us + # via a push that misses this workflow's path filters (a server- or + # docs-only change), because android-ci's own main runs are + # serialised by the concurrency group above. Rebasing onto that tip + # conflicts on the VERSION_NAME line, and resetting onto it would + # publish a tag whose tree this run never assembled or tested — + # trading a visible failure for a silently unverified release. A + # failure here strands nothing, since --atomic means nothing was + # pushed: re-run the job and it bumps cleanly from the new tip. + git push --atomic origin "HEAD:$GITHUB_REF" "v$RELEASE_VERSION_NAME" - name: Upload debug APK if: github.ref == 'refs/heads/main' uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5 @@ -190,14 +215,20 @@ jobs: path: app/build/outputs/apk/debug/*.apk if-no-files-found: error - # On main only: build the signed release APK and publish a GitHub - # Release. The release action creates the tag itself, so we don't push - # tags from the workflow (workflow-pushed tags via GITHUB_TOKEN don't - # trigger downstream runs anyway, which is why this used to be split). + # On main only: check out the tag the build job pushed after its checks went + # green, build the signed release APK from it and publish a GitHub Release. + # The tag itself is already on origin by now, so the release action only ever + # attaches a Release to an existing tag rather than creating one. If a runner + # dies in the gap between the push and this job, the tag is public with no + # APK — run .github/workflows/release-apk.yaml against that tag to republish + # without cutting a new version. release: needs: build if: github.ref == 'refs/heads/main' runs-on: ubuntu-latest + # Same reasoning as the build job: cap the run well under GitHub's 6-hour + # maximum-execution ceiling so a hung Gradle task fails instead of idling. + timeout-minutes: 30 permissions: contents: write steps: @@ -232,3 +263,7 @@ jobs: tag_name: v${{ needs.build.outputs.version }} files: app/build/outputs/apk/release/*.apk generate_release_notes: true + # A Release with no APK is the state that blocks F-Droid, so if the + # glob ever stops matching (an AGP output-path change, a variant + # rename) fail loudly instead of publishing an empty Release green. + fail_on_unmatched_files: true diff --git a/.github/workflows/release-apk.yaml b/.github/workflows/release-apk.yaml new file mode 100644 index 0000000..2fa3441 --- /dev/null +++ b/.github/workflows/release-apk.yaml @@ -0,0 +1,136 @@ +name: release-apk + +# Manual recovery lever for a tag that is already on origin but has no GitHub +# Release, or has one with no APK attached to it. +# +# android-ci pushes the version-bump tag only after its checks pass, but the +# gap between that push and its `release` job is still a window: if the runner +# dies inside it the tag is public with nothing to download. That is exactly +# what stranded v2026.07.28.223 (issue #93) back when the push happened before +# the build — the unit-test step hung, GitHub killed the job at its 6-hour +# ceiling, and the release job never started. F-Droid verifies its reproducible +# builds against the upstream signed APK, so a tag with no APK stops them dead. +# +# This workflow rebuilds and publishes for an EXISTING tag. It never computes a +# version, never touches gradle.properties and never creates a tag, so it can +# repair a release without burning a new version number. It mirrors android-ci's +# `release` job step for step — same pinned action SHAs, same JDK, same SDK +# packages, same signing env, same Gradle task — so what it ships is what +# android-ci would have shipped. +# +# Note it has no push/pull_request trigger, and android-ci's path filters name +# `.github/workflows/android-ci.yaml` explicitly rather than `.github/**`, so +# merging or editing this file never fires a version bump. + +on: + workflow_dispatch: + inputs: + tag: + description: "Existing tag to build and publish, e.g. v2026.07.28.223" + required: true + type: string + +# Never let two republishes of the same tag race each other over the release +# assets. Distinct tags may run in parallel. +concurrency: + group: release-apk-${{ github.event.inputs.tag }} + cancel-in-progress: false + +jobs: + release: + runs-on: ubuntu-latest + # A release build is a few minutes. Cap it well under GitHub's 6-hour + # maximum-execution ceiling so a hung Gradle task fails instead of idling. + timeout-minutes: 30 + permissions: + contents: write + steps: + # Checkout only proves the ref resolves — it accepts a branch name or a + # bare SHA just as happily as a tag. + - uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5 + with: + ref: ${{ inputs.tag }} + fetch-depth: 0 + fetch-tags: true + # …so check it really is a tag. tag_name is handed straight to the release + # action, and GitHub's create-release API creates the ref when it is + # missing: dispatching with "main" by mistake would otherwise mint a + # refs/tags/main and mark it Latest. This workflow republishes tags, it + # never invents them. + - name: Verify the input is a tag + # Via env rather than inline ${{ }}: the input is free text, and + # interpolating it straight into the shell would let it run as script. + env: + TAG: ${{ inputs.tag }} + run: | + git rev-parse --verify "refs/tags/$TAG" >/dev/null 2>&1 || { + echo "::error::$TAG is not an existing tag — refusing to publish." + exit 1 + } + - uses: actions/setup-java@be666c2fcd27ec809703dec50e508c2fdc7f6654 # v5 + with: + distribution: temurin + java-version: "21" + - uses: android-actions/setup-android@40fd30fb8d7440372e1316f5d1809ec01dcd3699 # v4 + with: + packages: "platform-tools platforms;android-34 build-tools;34.0.0" + - uses: gradle/actions/setup-gradle@4c125117fe7c5aed11272ec4213f602f012f89f2 # v5 + - name: Decode keystore + if: env.KEYSTORE_B64 != '' + run: echo "$KEYSTORE_B64" | base64 -d > "$RUNNER_TEMP/release.keystore" + env: + KEYSTORE_B64: ${{ secrets.QUIRE_RELEASE_KEYSTORE_B64 }} + # A blank keystore makes app/build.gradle.kts fall back to the debug + # signing config, and :app:assembleRelease then succeeds and produces a + # debug-signed app-release.apk. Publishing that is worse than publishing + # nothing: F-Droid rejects the signature outright, and anyone who installs + # it can never upgrade to a properly signed build, because Android refuses + # to install over a changed signature. + - name: Require signing secrets + run: | + test -s "$RUNNER_TEMP/release.keystore" || { + echo "::error::Release keystore missing — refusing to publish a debug-signed build." + exit 1 + } + # GitHub marks a newly created Release as "Latest" unless told otherwise, + # so repairing an older tag would quietly demote the current release and + # point everyone at a stale version. Decide it from the tag graph instead + # of asking the operator to remember: mark it Latest only when nothing + # newer has been tagged. Repairing the newest tag — the ordinary case, + # right after a run stranded it — still gets the badge. + - name: Decide whether this tag is the newest + id: latest + env: + TAG: ${{ inputs.tag }} + run: | + newest=$(git tag --sort=-creatordate | head -n 1) + if [ "$newest" = "$TAG" ]; then + echo "make_latest=true" >> "$GITHUB_OUTPUT" + else + echo "make_latest=false" >> "$GITHUB_OUTPUT" + echo "$newest is newer than $TAG — leaving the Latest badge where it is." + fi + - name: Assemble release APK + env: + QUIRE_RELEASE_KEYSTORE: ${{ secrets.QUIRE_RELEASE_KEYSTORE_B64 != '' && format('{0}/release.keystore', runner.temp) || '' }} + QUIRE_RELEASE_KEYSTORE_PASSWORD: ${{ secrets.QUIRE_RELEASE_KEYSTORE_PASSWORD }} + QUIRE_RELEASE_KEY_ALIAS: ${{ secrets.QUIRE_RELEASE_KEY_ALIAS }} + QUIRE_RELEASE_KEY_PASSWORD: ${{ secrets.QUIRE_RELEASE_KEY_PASSWORD }} + run: ./gradlew :app:assembleRelease --stacktrace + - name: Create GitHub Release + uses: softprops/action-gh-release@b4309332981a82ec1c5618f44dd2e27cc8bfbfda # v3 + with: + tag_name: ${{ inputs.tag }} + files: app/build/outputs/apk/release/*.apk + generate_release_notes: true + # The action looks the Release up by tag first: no Release yet means + # it creates one against the tag that already exists, an existing one + # means it updates that in place. overwrite_files is the action's own + # default but is spelled out because this workflow exists for the + # re-run case — without it a second attempt would trip over the + # app-release.apk asset the first attempt already uploaded. + overwrite_files: true + # An empty Release is the very failure we are here to repair, so + # refuse to publish one if the glob matched nothing. + fail_on_unmatched_files: true + make_latest: ${{ steps.latest.outputs.make_latest }} diff --git a/app/src/test/java/io/theficos/ereader/data/ai/AiRepositoryLocalFirstTest.kt b/app/src/test/java/io/theficos/ereader/data/ai/AiRepositoryLocalFirstTest.kt index f71dbfc..60d438a 100644 --- a/app/src/test/java/io/theficos/ereader/data/ai/AiRepositoryLocalFirstTest.kt +++ b/app/src/test/java/io/theficos/ereader/data/ai/AiRepositoryLocalFirstTest.kt @@ -62,8 +62,8 @@ class AiRepositoryLocalFirstTest { ) repo.refresh() // Drain the two requests so subsequent assertions count from zero. - server.takeRequest() - server.takeRequest() + server.awaitRequest() + server.awaitRequest() } private fun seedRow( diff --git a/app/src/test/java/io/theficos/ereader/data/ai/AiRepositoryRefreshTest.kt b/app/src/test/java/io/theficos/ereader/data/ai/AiRepositoryRefreshTest.kt index 586d303..a8188fc 100644 --- a/app/src/test/java/io/theficos/ereader/data/ai/AiRepositoryRefreshTest.kt +++ b/app/src/test/java/io/theficos/ereader/data/ai/AiRepositoryRefreshTest.kt @@ -79,7 +79,7 @@ class AiRepositoryRefreshTest { val repo = AiRepository(client = client, insightDao = insightDao) server.enqueue(MockResponse().setResponseCode(204)) repo.deleteProfile() - val req = server.takeRequest() + val req = server.awaitRequest() assertThat(req.method).isEqualTo("DELETE") assertThat(req.path).isEqualTo("/ai/v1/profile") } diff --git a/app/src/test/java/io/theficos/ereader/data/ai/AiRepositoryStyleTest.kt b/app/src/test/java/io/theficos/ereader/data/ai/AiRepositoryStyleTest.kt index b23868f..9c0509f 100644 --- a/app/src/test/java/io/theficos/ereader/data/ai/AiRepositoryStyleTest.kt +++ b/app/src/test/java/io/theficos/ereader/data/ai/AiRepositoryStyleTest.kt @@ -50,10 +50,10 @@ class AiRepositoryStyleTest { repo.setStyleTone("scholarly") // Drain config + prefs GETs. - server.takeRequest() - server.takeRequest() + server.awaitRequest() + server.awaitRequest() // The PUT body must include the preserved language. - val put = server.takeRequest() + val put = server.awaitRequest() assertThat(put.method).isEqualTo("PUT") val body = put.body.readUtf8() assertThat(body).contains("\"tone\":\"scholarly\"") @@ -69,9 +69,9 @@ class AiRepositoryStyleTest { server.enqueue(MockResponse().setResponseCode(200).setBody("""{"ai_enabled":true,"style":{"tone":"scholarly","language":"es"}}""")) repo.setStyleLanguage("es") - server.takeRequest() - server.takeRequest() - val put = server.takeRequest() + server.awaitRequest() + server.awaitRequest() + val put = server.awaitRequest() val body = put.body.readUtf8() assertThat(body).contains("\"tone\":\"scholarly\"") assertThat(body).contains("\"language\":\"es\"") @@ -83,7 +83,7 @@ class AiRepositoryStyleTest { server.enqueue(MockResponse().setResponseCode(200).setBody("""{"ai_enabled":true,"style":{"tone":"neutral","language":"fr"}}""")) repo.setStyleLanguage("fr") - val put = server.takeRequest() + val put = server.awaitRequest() val body = put.body.readUtf8() assertThat(body).contains("\"tone\":\"neutral\"") assertThat(body).contains("\"language\":\"fr\"") diff --git a/app/src/test/java/io/theficos/ereader/data/ai/InsightSyncRepositoryTest.kt b/app/src/test/java/io/theficos/ereader/data/ai/InsightSyncRepositoryTest.kt index dc603a1..5c840ff 100644 --- a/app/src/test/java/io/theficos/ereader/data/ai/InsightSyncRepositoryTest.kt +++ b/app/src/test/java/io/theficos/ereader/data/ai/InsightSyncRepositoryTest.kt @@ -56,7 +56,7 @@ class InsightSyncRepositoryTest { ) ) aiRepo.refresh() - server.takeRequest(); server.takeRequest() + server.awaitRequest(); server.awaitRequest() } private fun syncItem(id: Long, generatedAt: String, identityKey: String = "m$id"): String = @@ -106,8 +106,8 @@ class InsightSyncRepositoryTest { assertThat(synced.items).isEqualTo(3) assertThat(dao.count()).isEqualTo(3) // The second call carried the cursor from page1. - val req1 = server.takeRequest() - val req2 = server.takeRequest() + val req1 = server.awaitRequest() + val req2 = server.awaitRequest() assertThat(req1.requestUrl?.queryParameter("since_ts")).isNull() assertThat(req2.requestUrl?.queryParameter("since_ts")).isEqualTo("2026-05-02T00:00:00Z") assertThat(req2.requestUrl?.queryParameter("since_id")).isEqualTo("2") diff --git a/app/src/test/java/io/theficos/ereader/data/ai/MockWebServerWaits.kt b/app/src/test/java/io/theficos/ereader/data/ai/MockWebServerWaits.kt new file mode 100644 index 0000000..3dd27b8 --- /dev/null +++ b/app/src/test/java/io/theficos/ereader/data/ai/MockWebServerWaits.kt @@ -0,0 +1,36 @@ +package io.theficos.ereader.data.ai + +import okhttp3.mockwebserver.MockWebServer +import okhttp3.mockwebserver.RecordedRequest +import java.util.concurrent.TimeUnit + +/** How long a drain waits for a request that should already have landed. */ +private const val REQUEST_WAIT_SECONDS = 10L + +/** + * Bounded replacement for `MockWebServer.takeRequest()`. + * + * The no-argument overload is a bare `LinkedBlockingQueue.take()`: if the suite + * drains more requests than actually reached the server, it parks the calling + * thread forever. That is a live hazard in this package because + * [AiRepository.refresh] wraps both of its HTTP calls in `runCatching` and is + * documented as "silent on failure" — a call that dies before its request hits + * the wire (an OkHttp `callTimeout` expiring on a contended CI runner, a + * connect-time reset) leaves `refresh()` returning normally with only one + * recorded request, and the second drain never comes back. + * + * Blocking the thread is what makes that fatal rather than flaky. On the JVM + * `runTest` runs inside `runBlocking` and schedules its 60-second wall-clock + * guard on that same event loop, so a test that blocks the loop's own thread + * can never be timed out: the worker hangs until CI kills the job. That is how + * `v2026.07.28.223` came to be tagged with no APK attached (issue #93). + * + * Waiting with a deadline turns the hang into an ordinary test failure. The + * budget is deliberately well past the 5-second `callTimeout` these suites + * configure, so a merely slow runner still passes. + */ +internal fun MockWebServer.awaitRequest(): RecordedRequest = + checkNotNull(takeRequest(REQUEST_WAIT_SECONDS, TimeUnit.SECONDS)) { + "MockWebServer recorded no request within ${REQUEST_WAIT_SECONDS}s: the client " + + "call never reached the server, so this drain is off by one." + } diff --git a/build.gradle.kts b/build.gradle.kts index aa34aeb..f206998 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -1,3 +1,5 @@ +import java.time.Duration + plugins { alias(libs.plugins.android.application) apply false alias(libs.plugins.android.library) apply false @@ -8,3 +10,33 @@ plugins { alias(libs.plugins.ksp) apply false alias(libs.plugins.aboutlibraries) apply false } + +// Wall-clock budget for every unit-test task, in every module. +// +// Why this exists: on 2026-07-28 a `:app` unit-test task wedged mid-run and the +// android-ci `build` job sat there until GitHub killed it at the 6-hour +// maximum-execution ceiling. The version bump and the `v2026.07.28.223` tag had +// already been pushed by then, and the `release` job never got to run, so the +// tag shipped with no APK and F-Droid could not verify the release (issue #93). +// The same stall had happened once before on a feature branch, so a test task +// that stops making progress has to fail the build rather than idle. +// +// Gradle's per-task `timeout` interrupts the task's execution thread once the +// budget is spent and marks the build FAILED. Gradle documents its built-in +// tasks — `Test` included — as responsive to that interrupt, but a task wedged +// somewhere unresponsive could still outlive it, so treat this as the inner of +// two nets: the CI job carries its own `timeout-minutes` as the outer one. +// +// Sizing: on CI a whole `./gradlew test` across every module takes about 35 +// seconds wall-clock (run 29286428853), and no single module's task runs for +// more than a few of those. Ten minutes is far past any legitimate run — even a +// cold, CPU-starved runner — while still bounding a wedged task at minutes +// instead of hours. +// +// `withType` covers AGP's `testDebugUnitTest` / `testReleaseUnitTest` +// too: `AndroidUnitTest` extends `org.gradle.api.tasks.testing.Test`. +subprojects { + tasks.withType().configureEach { + timeout.set(Duration.ofMinutes(10)) + } +} diff --git a/data/library/src/test/java/io/theficos/ereader/data/library/sync/LibraryMirrorPushWorkerTest.kt b/data/library/src/test/java/io/theficos/ereader/data/library/sync/LibraryMirrorPushWorkerTest.kt index aec61cd..7e6219f 100644 --- a/data/library/src/test/java/io/theficos/ereader/data/library/sync/LibraryMirrorPushWorkerTest.kt +++ b/data/library/src/test/java/io/theficos/ereader/data/library/sync/LibraryMirrorPushWorkerTest.kt @@ -115,7 +115,7 @@ class LibraryMirrorPushWorkerTest { assertThat(result).isInstanceOf(ListenableWorker.Result.Success::class.java) assertThat(server.requestCount).isEqualTo(1) - val req = server.takeRequest() + val req = server.awaitRequest() assertThat(req.path).isEqualTo("/library/v1/sync") assertThat(req.method).isEqualTo("POST") val body = req.body.readUtf8() @@ -164,7 +164,7 @@ class LibraryMirrorPushWorkerTest { buildWorker().doWork() - val body = server.takeRequest().body.readUtf8() + val body = server.awaitRequest().body.readUtf8() // Defense in depth: the legacy server column name must never // appear on this endpoint's wire — Pydantic rejects it. assertThat(body).doesNotContain("content_hash") @@ -188,7 +188,7 @@ class LibraryMirrorPushWorkerTest { assertThat(result).isInstanceOf(ListenableWorker.Result.Success::class.java) assertThat(server.requestCount).isEqualTo(3) val sizes = (1..3).map { - val req = server.takeRequest() + val req = server.awaitRequest() val items = Json.parseToJsonElement(req.body.readUtf8()).jsonObject["items"]!!.jsonArray items.size } @@ -204,8 +204,8 @@ class LibraryMirrorPushWorkerTest { buildWorker().doWork() - val ts1 = firstLastSeenAt(server.takeRequest()) - val ts2 = firstLastSeenAt(server.takeRequest()) + val ts1 = firstLastSeenAt(server.awaitRequest()) + val ts2 = firstLastSeenAt(server.awaitRequest()) assertThat(ts1).isNotNull() assertThat(ts1).isEqualTo(ts2) } @@ -307,7 +307,7 @@ class LibraryMirrorPushWorkerTest { buildWorker().doWork() - val items = Json.parseToJsonElement(server.takeRequest().body.readUtf8()) + val items = Json.parseToJsonElement(server.awaitRequest().body.readUtf8()) .jsonObject["items"]!!.jsonArray val byHash = items.associateBy { it.jsonObject["identity_hash"]!!.jsonPrimitive.content } assertThat(byHash["h1"]!!.jsonObject["identity_hash_version"]!!.jsonPrimitive.int()) @@ -324,7 +324,7 @@ class LibraryMirrorPushWorkerTest { buildWorker().doWork() - val items = Json.parseToJsonElement(server.takeRequest().body.readUtf8()) + val items = Json.parseToJsonElement(server.awaitRequest().body.readUtf8()) .jsonObject["items"]!!.jsonArray val authors = items[0].jsonObject["authors"]!!.jsonArray assertThat(authors.map { it.jsonPrimitive.content }).containsExactly("A", "B").inOrder() diff --git a/data/library/src/test/java/io/theficos/ereader/data/library/sync/MockWebServerWaits.kt b/data/library/src/test/java/io/theficos/ereader/data/library/sync/MockWebServerWaits.kt new file mode 100644 index 0000000..ea8e666 --- /dev/null +++ b/data/library/src/test/java/io/theficos/ereader/data/library/sync/MockWebServerWaits.kt @@ -0,0 +1,34 @@ +package io.theficos.ereader.data.library.sync + +import java.util.concurrent.TimeUnit +import okhttp3.mockwebserver.MockWebServer +import okhttp3.mockwebserver.RecordedRequest + +/** How long a drain waits for a request that should already have landed. */ +private const val REQUEST_WAIT_SECONDS = 10L + +/** + * Bounded replacement for `MockWebServer.takeRequest()`. + * + * The no-argument overload is a bare `LinkedBlockingQueue.take()`, so draining + * one request more than actually reached the server parks the calling thread + * forever. A `ListenableWorker` reports trouble by returning `Result.retry()` + * or `Result.failure()` rather than by throwing, so a push that dies before its + * request hits the wire leaves `doWork()` returning normally with a short + * request queue and the next drain never comes back. + * + * Blocking the thread is what makes that fatal rather than flaky: on the JVM + * `runTest` runs inside `runBlocking` and schedules its 60-second wall-clock + * guard on the same event loop, so a test that blocks that loop's own thread + * can never be timed out — the worker hangs until CI kills the job. An + * equivalent unbounded drain in `:app` is what stalled a build for six hours + * and left `v2026.07.28.223` tagged with no APK attached (issue #93). + * + * The budget is well past the 5-second `callTimeout` this suite configures, so + * a merely slow runner still passes. + */ +internal fun MockWebServer.awaitRequest(): RecordedRequest = + checkNotNull(takeRequest(REQUEST_WAIT_SECONDS, TimeUnit.SECONDS)) { + "MockWebServer recorded no request within ${REQUEST_WAIT_SECONDS}s: the worker's " + + "call never reached the server, so this drain is off by one." + }