From 93546ed724a8355262c70db210bd72d6af5bd305 Mon Sep 17 00:00:00 2001 From: Sprite Date: Sat, 12 Sep 2026 17:36:11 +0000 Subject: [PATCH] Opt-in live test: the real metadata lookup, against both real sources MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Investigating the crash the user hit on the first scan after the Google Books key landed. The structural suspicion was sound: keyless Google Books 429'd every caller, so GoogleBooksClient's 2xx branch, toBookMetadata(), normalizeCoverUrl() and the two-source merge had never once executed in production before 486f6eb. First exercise of a code path is where a first crash belongs. It does not reproduce here. Nine live lookups through the real MetadataRepository.lookup — both of the user's previously-failing ISBNs plus a control, three times each, with the real key — all returned Found with cover art, 254ms to 4.7s, nothing thrown. So it is not a parse, merge or cover-URL bug in any form this machine can provoke, which is worth knowing before anyone spends a wave rewriting that code. The test asserts the contract rather than the content: that lookup RETURNS instead of throwing. That is what both clients' KDoc claims ("Never throws") and what nothing currently enforces. Gated on LIVE_METADATA=1 like LiveSyncTest, so the normal suite stays offline and deterministic. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PPpdG8VnRfS3KkisR3HUAE --- .../livemetadata/LiveMetadataLookupTest.kt | 108 ++++++++++++++++++ 1 file changed, 108 insertions(+) create mode 100644 app/app/src/test/java/org/modg/bookshelf/livemetadata/LiveMetadataLookupTest.kt diff --git a/app/app/src/test/java/org/modg/bookshelf/livemetadata/LiveMetadataLookupTest.kt b/app/app/src/test/java/org/modg/bookshelf/livemetadata/LiveMetadataLookupTest.kt new file mode 100644 index 0000000..4b5c267 --- /dev/null +++ b/app/app/src/test/java/org/modg/bookshelf/livemetadata/LiveMetadataLookupTest.kt @@ -0,0 +1,108 @@ +package org.modg.bookshelf.livemetadata + +import java.util.concurrent.TimeUnit +import kotlinx.coroutines.runBlocking +import kotlinx.serialization.json.Json +import okhttp3.OkHttpClient +import org.junit.Assert.assertTrue +import org.junit.Assume.assumeTrue +import org.junit.Test +import org.modg.bookshelf.data.metadata.LookupResult +import org.modg.bookshelf.data.metadata.MetadataRepository + +/** + * OPT-IN live test — skipped unless `LIVE_METADATA=1` is set, exactly like + * [org.modg.bookshelf.livesync.LiveSyncTest]. It talks to the real Open Library + * and Google Books APIs over the real network. + * + * Written 2026-09-12 to investigate a crash the user hit on the FIRST scan after + * the Google Books key landed (commit 486f6eb): the ISBN was displayed, the + * spinner ran, then the app died. It has never reproduced. + * + * The structural reason to suspect this path: keyless Google Books returned 429 + * to every caller on the internet (docs/METADATA-SOURCES.md), so until that + * commit, `GoogleBooksClient.classify`'s 2xx branch, `toBookMetadata()`, + * `normalizeCoverUrl()` and the two-source merge had **never executed in + * production**. The first real exercise of a code path is where a first crash + * belongs. + * + * What this test is actually asserting is not "the lookup found the book" — that + * is the subject of other tests and depends on flaky third parties. It asserts + * that [MetadataRepository.lookup] **returns rather than throws**, which is the + * contract both clients document ("Never throws: every outcome ... comes back as + * a SourceResult") and which nothing currently enforces: `fetch` catches only + * `IOException`, `classify` only `SerializationException`/`IllegalArgumentException`, + * and `ScanViewModel.runLookup` has no try/catch at all, so anything else that + * escapes kills the process. + * + * A JVM pass here does NOT clear the code — the phone differs in TLS provider, + * memory pressure and Android API level. A JVM FAILURE, on the other hand, would + * be the bug itself. + */ +class LiveMetadataLookupTest { + + /** The two the user scanned that failed in wave 5/6, plus a mainstream control. */ + private val isbns = listOf( + "9781883937386", // Hittite Warrior — Bethlehem Books + "9781883937676", // Shadow Hawk — Bethlehem Books + "9780140449136", // Crime and Punishment — Penguin + ) + + /** Same shape as AppContainer.metadataHttpClient, including the measured timeouts. */ + private val httpClient = OkHttpClient.Builder() + .callTimeout(25, TimeUnit.SECONDS) + .connectTimeout(20, TimeUnit.SECONDS) + .readTimeout(20, TimeUnit.SECONDS) + .build() + + private val json = Json { + ignoreUnknownKeys = true + encodeDefaults = false + explicitNulls = false + } + + @Test + fun `real lookups against both live sources return an outcome instead of throwing`() { + assumeTrue( + "Live metadata test skipped (set LIVE_METADATA=1, and GOOGLE_BOOKS_API_KEY for the keyed path)", + System.getenv("LIVE_METADATA") != null, + ) + val apiKey = System.getenv("GOOGLE_BOOKS_API_KEY").orEmpty() + println("live metadata lookup: key ${if (apiKey.isBlank()) "ABSENT (keyless path)" else "present"}") + + val repository = MetadataRepository(httpClient, json, apiKey) + + // Each ISBN is run repeatedly: the crash under investigation happened once + // and never again, so a single green pass proves very little. Any throw + // from any iteration fails the test and names the iteration. + val results = mutableListOf() + for (isbn in isbns) { + repeat(REPEATS) { attempt -> + val elapsed = System.currentTimeMillis() + val result = try { + runBlocking { repository.lookup(isbn) } + } catch (t: Throwable) { + throw AssertionError( + "lookup($isbn) attempt ${attempt + 1} THREW ${t.javaClass.name}: ${t.message} " + + "— this is the crash; MetadataRepository.lookup must never throw", + t, + ) + } + val took = System.currentTimeMillis() - elapsed + val summary = when (result) { + is LookupResult.Found -> "Found(${result.metadata.title}, cover=${result.metadata.coverUrl != null})" + is LookupResult.NotFound -> "NotFound" + is LookupResult.Unavailable -> "Unavailable(${result.reason})" + } + results += "$isbn #${attempt + 1} ${took}ms $summary" + } + } + + results.forEach(::println) + assertTrue("expected one result per lookup", results.size == isbns.size * REPEATS) + } + + private companion object { + const val REPEATS = 3 + } +}