From 55ab75e3e355da2183a02a963331ec4f12282fe7 Mon Sep 17 00:00:00 2001 From: vito Date: Wed, 29 Jul 2026 09:37:56 +0200 Subject: [PATCH] =?UTF-8?q?ci:=20=F0=9F=90=9B=20stop=20a=20hung=20build=20?= =?UTF-8?q?from=20stranding=20a=20tag=20with=20no=20APK=20(#93)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A merge to main on 2026-07-28 tagged v2026.07.28.223 and then hung in the unit-test step until GitHub killed the job at its 6-hour ceiling. The version bump and tag are pushed by the first step of the build job, so they were already public; the release job that builds the signed APK never started. F-Droid verifies its reproducible builds against the upstream APK, so a tag with nothing attached stops them dead. The same stall had happened once before on a feature branch, where it cost nothing but six hours of runner time. Four changes, addressing the hang, the blast radius, and the recovery: Bound the hang. MockWebServer's no-argument takeRequest() is a bare LinkedBlockingQueue.take(), so draining one request more than actually reached the server parks the thread with no deadline. AiRepository.refresh() wraps both its HTTP calls in runCatching and is documented "silent on failure", so a call that dies before hitting the wire returns normally having recorded one request while the test drains two. Blocking is what makes that fatal rather than flaky: runTest schedules its 60-second guard on the same runBlocking event loop the test body occupies, so blocking that thread means the timeout can never fire. awaitRequest() waits with a 10-second deadline instead, well past the 5-second callTimeout these suites configure. LibraryMirrorPushWorkerTest had the same defect, reached via a worker returning Result.retry() instead of throwing. Cap every test task at 10 minutes from the root build script, so a future wedge anywhere fails the build in minutes rather than idling. A whole ./gradlew test is about 35 seconds, so the margin is large. Push the bump and tag only after the tests and lint pass, with --atomic so the branch and the tag land together or not at all. A run that fails or hangs now leaves origin untouched. The old retry-on-rejection loop is gone: resetting onto a moved main and re-applying the bump would publish a tag whose tree the run never built, which trades a visible failure for a silently unverified release. Both jobs also carry timeout-minutes now. Add release-apk.yaml, a workflow_dispatch lever that rebuilds and publishes the signed APK for an existing tag. It never computes a version or creates a tag, so it repairs a release without burning a version number, and it refuses to run without the signing secrets — a debug-signed APK is worse than none, since F-Droid rejects the signature and Android then blocks any upgrade over it. --- .github/workflows/android-ci.yaml | 111 +++++++++----- .github/workflows/release-apk.yaml | 136 ++++++++++++++++++ .../data/ai/AiRepositoryLocalFirstTest.kt | 4 +- .../data/ai/AiRepositoryRefreshTest.kt | 2 +- .../ereader/data/ai/AiRepositoryStyleTest.kt | 14 +- .../data/ai/InsightSyncRepositoryTest.kt | 6 +- .../ereader/data/ai/MockWebServerWaits.kt | 36 +++++ build.gradle.kts | 32 +++++ .../sync/LibraryMirrorPushWorkerTest.kt | 14 +- .../data/library/sync/MockWebServerWaits.kt | 34 +++++ 10 files changed, 331 insertions(+), 58 deletions(-) create mode 100644 .github/workflows/release-apk.yaml create mode 100644 app/src/test/java/io/theficos/ereader/data/ai/MockWebServerWaits.kt create mode 100644 data/library/src/test/java/io/theficos/ereader/data/library/sync/MockWebServerWaits.kt 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." + }