From 93dc8d044d99bda9087d9adb1bc2c8e959ec8588 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Tue, 10 Nov 2026 15:56:59 -0500 Subject: [PATCH] test: strengthen provider contracts and consolidate duplicate cases --- src/adb/AdbProvider.test.ts | 73 ++++++++++++------- src/adb/AdbShellOutput.test.ts | 10 --- src/android/JadxOperationTimeout.test.ts | 13 +++- src/apktool/ApktoolProvider.test.ts | 35 ++------- .../AnalysisContextQueries.test.ts | 55 ++------------ src/application/WebModuleTraceService.test.ts | 2 +- .../WebSourceLocationService.test.ts | 16 ---- src/application/WorkflowEvidence.test.ts | 34 --------- .../binary/AnalysisSnapshotCache.test.ts | 13 ++-- .../binary/BundleResourceSession.test.ts | 14 +++- 10 files changed, 87 insertions(+), 178 deletions(-) diff --git a/src/adb/AdbProvider.test.ts b/src/adb/AdbProvider.test.ts index 51e6686057..e5712e77f9 100644 --- a/src/adb/AdbProvider.test.ts +++ b/src/adb/AdbProvider.test.ts @@ -779,20 +779,40 @@ if [ "$1" = "version" ]; then ${VERSION_BLOCK}; fi`, }); it("reads a bounded logcat dump with the caller count and buffer", async () => { - const { binary } = await writeAdbStub( - shellStub({ - logcat: - "08-10 13:00:00.000 I/Tag( 123): first\n08-10 13:00:01.000 I/Tag( 123): second\n", - }), + const { binary, root } = await writeAdbStub( + (root) => `printf '%s\\n' "$@" >> '${join(root, "argv.log")}' +${shellStub({ + logcat: + "08-10 13:00:00.000 I/Tag( 123): first\n08-10 13:00:01.000 I/Tag( 123): second\n", +})(root)}`, ); const result = await evidenceResult(providerOver(binary), { operation: "read_adb_logcat", - input: { serial: "emulator-5554", count: 500, buffer: "main" }, + input: { serial: "emulator-5554", count: 1, buffer: "system", pid: 123 }, }); - expect(result.lines).toEqual([ - "08-10 13:00:00.000 I/Tag( 123): first", - "08-10 13:00:01.000 I/Tag( 123): second", + expect( + (await readFile(join(root, "argv.log"), "utf8")).trim().split("\n"), + ).toEqual([ + "version", + "-s", + "emulator-5554", + "shell", + "logcat", + "-d", + "-b", + "system", + "-t", + "1", + "--pid", + "123", ]); + expect(result).toMatchObject({ + count: 1, + buffer: "system", + pid: 123, + coverage: "partial", + lines: ["08-10 13:00:01.000 I/Tag( 123): second"], + }); }); it("reports UTF-8 byte counts in total_bytes for non-ASCII logcat output", async () => { @@ -1667,13 +1687,15 @@ exit 1`, operation: "capture_adb_screen", input: { serial: "emulator-5554", output_directory: output }, }); - expect(result).toMatchObject({ width: 1, height: 1 }); - expect( - await readFile(join(output, "screen.png")).then( - (bytes) => bytes.subarray(1, 4).toString("latin1") === "PNG", - () => false, - ), - ).toBe(true); + expect(result).toMatchObject({ + width: 1, + height: 1, + bytes: SCREEN_PNG.byteLength, + sha256: createHash("sha256").update(SCREEN_PNG).digest("hex"), + }); + expect((await readFile(join(output, "screen.png"))).equals(SCREEN_PNG)).toBe( + true, + ); }); it("stops a screen capture aborted after adb returns but before publication", async () => { @@ -1889,11 +1911,12 @@ it("reads Android 16 focus declarations from displays when windows omits them", "mCurrentFocus=Window{abc u0 com.example/.Main}\nmFocusedApp=ActivityRecord{def u0 com.example/.Main t1}\n", }), ); - const result = await evidenceResult(providerOver(binary), { + const execution = await providerOver(binary).execute({ operation: "inspect_adb_window", input: { serial: "emulator-5554" }, }); - expect(result).toMatchObject({ + if (!execution.ok) throw execution.error; + expect(execution.value.result).toMatchObject({ focused_window: "Window{abc u0 com.example/.Main}", focused_app: "ActivityRecord{def u0 com.example/.Main t1}", observed_lines: [ @@ -1901,11 +1924,6 @@ it("reads Android 16 focus declarations from displays when windows omits them", "mFocusedApp=ActivityRecord{def u0 com.example/.Main t1}", ], }); - const execution = await providerOver(binary).execute({ - operation: "inspect_adb_window", - input: { serial: "emulator-5554" }, - }); - if (!execution.ok) throw execution.error; expect(execution.value.limitations).toContainEqual( expect.stringContaining("dumpsys window displays"), ); @@ -1918,16 +1936,15 @@ it("describes missing window focus as unknown after both sections omit declarati "dumpsys:window:displays": "no focus\\n", }), ); - const result = await evidenceResult(providerOver(binary), { - operation: "inspect_adb_window", - input: { serial: "emulator-5554" }, - }); - expect(result).toMatchObject({ focused_window: null, focused_app: null }); const execution = await providerOver(binary).execute({ operation: "inspect_adb_window", input: { serial: "emulator-5554" }, }); if (!execution.ok) throw execution.error; + expect(execution.value.result).toMatchObject({ + focused_window: null, + focused_app: null, + }); expect(execution.value.limitations).toContainEqual( expect.stringContaining("focus is unknown"), ); diff --git a/src/adb/AdbShellOutput.test.ts b/src/adb/AdbShellOutput.test.ts index 4dac2b1bca..09027c208f 100644 --- a/src/adb/AdbShellOutput.test.ts +++ b/src/adb/AdbShellOutput.test.ts @@ -235,16 +235,6 @@ describe("parseWindowFocusOutput declaration presence", () => { }); describe("parseDumpsysPackageOutput", () => { - it.each(["userId", "appId"])( - "retains the printed %s package identifier", - (key) => { - expect( - parseDumpsysPackageOutput( - `Packages:\n Package [com.example] (abcdef):\n ${key}=10234\n`, - ).userId, - ).toBe(10234); - }, - ); it("reports unknown requested permissions when the package is null", () => { expect( parseDumpsysPackageOutput("Package [com.example] (abcdef):\n pkg=null\n") diff --git a/src/android/JadxOperationTimeout.test.ts b/src/android/JadxOperationTimeout.test.ts index e9ce1f8722..ca1acaf5d9 100644 --- a/src/android/JadxOperationTimeout.test.ts +++ b/src/android/JadxOperationTimeout.test.ts @@ -2,7 +2,6 @@ import { expect, it } from "vitest"; import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; import { captureJadxFailure } from "./JadxFailure.js"; -import { jadxOperationTimeoutGuidance } from "./JadxOperationTimeout.js"; it("names REA_JADX_OPERATION_TIMEOUT_MS when the operation deadline elapses", () => { const timeout = AbortSignal.abort(new Error("deadline")); @@ -18,9 +17,15 @@ it("names REA_JADX_OPERATION_TIMEOUT_MS when the operation deadline elapses", () session: undefined, }); const projected = projectAnalysisError(present()); - const guidance = jadxOperationTimeoutGuidance(1500); - expect(projected.message).toBe(guidance.userMessage); - expect(projected.remediation.action).toBe(guidance.remediationAction); + expect(projected.message).toContain("1500 ms"); + expect(projected.remediation.action).toContain( + "REA_JADX_OPERATION_TIMEOUT_MS", + ); + expect(projected.remediation.action).toContain("update_analysis_settings"); + expect(projected.remediation.action).toContain("jadx.operation_timeout_ms"); + expect(projected.remediation.action).toContain( + "next operation without restarting REA", + ); expect(projected.details).toMatchObject({ limit: { setting: "jadx.operation_timeout_ms", diff --git a/src/apktool/ApktoolProvider.test.ts b/src/apktool/ApktoolProvider.test.ts index ed54eba263..273ed72318 100644 --- a/src/apktool/ApktoolProvider.test.ts +++ b/src/apktool/ApktoolProvider.test.ts @@ -686,44 +686,20 @@ it("projects one locale's strings on request", async () => { ]); }); -it("notes a missing locale table instead of failing", async () => { - const { command, root } = await writeApktoolStub(decodeStub()); - const apk = await apkFixture(root); - const result = await evidenceResult(providerOver(command), { - operation: "decode_android_resources", - input: { path: apk.path, include_strings: true, locale: "fr" }, - }); - expect(result.strings).toEqual([]); -}); - it("notes a missing default strings table without strings", async () => { const { command, root } = await writeApktoolStub( decodeStub({ withStrings: false }), ); const apk = await apkFixture(root); - const result = await evidenceResult(providerOver(command), { + const execution = await providerOver(command).execute({ operation: "decode_android_resources", input: { path: apk.path, include_strings: true }, }); - expect(result.strings).toEqual([]); -}); - -it("reports apktool's own refusal for undecodable targets", async () => { - const { command, root } = await writeApktoolStub( - decodeStub({ failDecode: true }), + if (!execution.ok) throw execution.error; + expect(execution.value.result).toMatchObject({ strings: [] }); + expect(execution.value.limitations).toContain( + "No default res/values/strings.xml was found in the decoded workspace", ); - const apk = await apkFixture(root); - const projected = await refusedBy(providerOver(command), { - operation: "decode_android_resources", - input: { path: apk.path, include_strings: true }, - }); - expect(projected).toMatchObject({ - code: "unsupported_target", - details: { path: apk.path }, - }); - expect(projected.message).toContain(apk.path); - expect(projected.message).not.toContain(command); - expect(projected.message).toContain("Could not decode"); }); it("reports a missing launcher as a capability gap with recovery", async () => { @@ -1525,6 +1501,7 @@ it.each(["first-selected.apk", "second-selected.apk"])( details: { path, captured_output: { exit_code: 1 } }, }); expect(projected.message).toContain(path); + expect(projected.message).toContain("Could not decode"); expect(projected.message).not.toContain(command); } finally { await provider.close(); diff --git a/src/application/AnalysisContextQueries.test.ts b/src/application/AnalysisContextQueries.test.ts index 392a9b1add..b82227ebc0 100644 --- a/src/application/AnalysisContextQueries.test.ts +++ b/src/application/AnalysisContextQueries.test.ts @@ -23,7 +23,6 @@ import { type AnalysisError, } from "../domain/analysisErrorBase.js"; import type { Result } from "../domain/result.js"; -import type { JsonValue } from "../domain/jsonValue.js"; import { err, ok } from "../domain/result.js"; const provider = { id: "fixture", name: "Fixture", version: "1" }; @@ -137,52 +136,6 @@ describe("analysis context queries: navigation", () => { ); }); - it.each([ - null, - {}, - { found: false, procedure: null }, - { query_address: "0x401000", found: true, procedure: null }, - ])( - "keeps malformed containing-procedure output unknown: %j", - async (value) => { - const analysis: AnalysisOperationPort = { - execute: async (operation) => - ok( - createAnalysisExecution( - operation === "resolve_containing_procedure" - ? value - : operation === "current_document" - ? "fixture" - : "0x401000", - provider, - ), - ), - }; - expect(await getNavigationContext(analysis, {})).toMatchObject({ - ok: false, - error: { - _tag: "AnalysisOutputError", - operation: "resolve_containing_procedure", - }, - }); - }, - ); - - it("does not infer procedure absence from a provider error message", async () => { - const failure = new AnalysisProtocolError( - "not in a procedure: malformed reply", - ); - const analysis: AnalysisOperationPort = { - execute: async (operation) => - operation === "resolve_containing_procedure" - ? err(failure) - : ok(createAnalysisExecution("0x401000", provider)), - }; - expect( - await getNavigationContext(analysis, { document: "fixture" }), - ).toEqual(err(failure)); - }); - it("uses an explicit document without passing it to current_document and preserves procedure failures", async () => { const calls: string[] = []; const failure = new AnalysisProtocolError("provider response malformed"); @@ -209,6 +162,14 @@ describe("navigation procedure admission", () => { it.each([ { condition: "missing result", value: null }, { condition: "missing discriminator", value: {} }, + { + condition: "missing absence reason", + value: { found: false, procedure: null }, + }, + { + condition: "found procedure is null", + value: { query_address: "0x401000", found: true, procedure: null }, + }, { condition: "missing query identity", value: { found: false, procedure: null, reason: "not_in_procedure" }, diff --git a/src/application/WebModuleTraceService.test.ts b/src/application/WebModuleTraceService.test.ts index 8949a4c783..04d54711b4 100644 --- a/src/application/WebModuleTraceService.test.ts +++ b/src/application/WebModuleTraceService.test.ts @@ -40,7 +40,7 @@ it.each([ }); }, ); -it("rejects source text changed across the artifact port while retaining the original digest", async () => { +it("rejects source text changed across the artifact port before acquiring the resolver", async () => { const artifacts = { ...webModuleArtifactsFixture("import(name)"), source: "import(other)", diff --git a/src/application/WebSourceLocationService.test.ts b/src/application/WebSourceLocationService.test.ts index 818d7f42ff..9f4cfeffba 100644 --- a/src/application/WebSourceLocationService.test.ts +++ b/src/application/WebSourceLocationService.test.ts @@ -304,22 +304,6 @@ it("honors cancellation both before acquisition and across decoder completion", ].map((path) => ({ kind: "artifact-path", path })), }); - const malformedDecoder: WebSourceMapPort = { - trace: async (input, options) => { - const decoded = await decoder.trace(input, options); - return decoded.ok - ? ok({ ...decoded.value, source_map_sha256: "b".repeat(64) }) - : decoded; - }, - }; - const malformed = await new WebSourceLocationService( - { load: () => Promise.resolve(ok(webSourceLocationFixture())) }, - malformedDecoder, - ).trace(args); - expect(malformed.ok).toBe(false); - if (!malformed.ok) - expect(malformed.error).toBeInstanceOf(AnalysisOutputError); - const invalidController = new AbortController(); const malformedLate = await new WebSourceLocationService( { load: () => Promise.resolve(ok(webSourceLocationFixture())) }, diff --git a/src/application/WorkflowEvidence.test.ts b/src/application/WorkflowEvidence.test.ts index 1c91734d65..19d12a17a4 100644 --- a/src/application/WorkflowEvidence.test.ts +++ b/src/application/WorkflowEvidence.test.ts @@ -725,40 +725,6 @@ describe("workflow residual question retention", () => { ); }); - it.each([ - ["spaces", " Which boundary? "], - ["line feed and tab", "\nWhich boundary?\t"], - ["byte order mark", "\uFEFFWhich boundary?\uFEFF"], - ])( - "normalizes edge whitespace from %s without changing workflow Evidence", - (_, question) => { - const records = new InvestigationRecords(); - const result = { residual_unknowns: [question] }; - const evidence = createWorkflowEvidence({ - target: undefined, - operation: "inspect_native_api", - parameters: {}, - result, - upstreamProfile: undefined, - upstreamLimitations: [], - }); - - expect(records.recordEvidence(evidence).ok).toBe(true); - expect( - recordWorkflowUnknowns({ - name: "inspect_native_api", - result, - evidenceId: evidence.evidence_id, - recordUnknown: (unknown) => records.recordUnknown(unknown), - }).ok, - ).toBe(true); - expect(records.listUnknowns()).toMatchObject([ - { question: `Which boundary? (Evidence ${evidence.evidence_id})` }, - ]); - expect(evidence.normalized_result).toEqual(result); - }, - ); - it("rejects questions that are empty after whitespace normalization", () => { const records = new InvestigationRecords(); const result = { residual_unknowns: [" \n\t "] }; diff --git a/src/application/binary/AnalysisSnapshotCache.test.ts b/src/application/binary/AnalysisSnapshotCache.test.ts index 8c8b2126b3..d3789582eb 100644 --- a/src/application/binary/AnalysisSnapshotCache.test.ts +++ b/src/application/binary/AnalysisSnapshotCache.test.ts @@ -36,7 +36,7 @@ import { isSourceEvidenceRetainable, } from "./AnalysisSnapshotCache.js"; import { EvidenceLedger } from "../investigation/EvidenceLedger.js"; -import { enhancedOutputSchemas } from "../../contracts/toolOutputSchemaGroups.js"; +import { nativeCallGraphSchema } from "../../domain/native/nativeCallGraph.js"; it.each(["provider", "workflow"] as const)( "owns nested %s execution data before returning control to callers", @@ -1270,12 +1270,13 @@ it.each([ evidenceProfile: revisedProfile, }), ).toBeUndefined(); - if (operation === "get_call_graph") + if (operation === "get_call_graph") { + expect(nativeCallGraphSchema.safeParse(oldResult).success).toBe(false); expect( - enhancedOutputSchemas.get_call_graph.safeParse({ - normalized_result: oldResult, - }).success, - ).toBe(false); + nativeCallGraphSchema.safeParse({ graph: oldResult, limitations: [] }) + .success, + ).toBe(true); + } }, ); diff --git a/src/application/binary/BundleResourceSession.test.ts b/src/application/binary/BundleResourceSession.test.ts index 8ef291200e..e5241a3a1c 100644 --- a/src/application/binary/BundleResourceSession.test.ts +++ b/src/application/binary/BundleResourceSession.test.ts @@ -18,6 +18,8 @@ const identity: ProviderIdentity = { it("retains both child close failures when a resource owner rejects", async () => { const attempts: string[] = []; + const artifactCause = new Error("artifact close rejected"); + const nativeCause = new Error("native close rejected"); const artifact: AnalysisClient = { execute: async () => ({ ok: true, @@ -25,7 +27,7 @@ it("retains both child close failures when a resource owner rejects", async () = }), close: async () => { attempts.push("artifact"); - throw new Error("artifact close rejected"); + throw artifactCause; }, }; const native: AnalysisClient = { @@ -35,14 +37,20 @@ it("retains both child close failures when a resource owner rejects", async () = }), close: async () => { attempts.push("native"); - throw new Error("native close rejected"); + throw nativeCause; }, }; const closed = await combineBundleResourceClients(artifact, native).close(); expect(attempts.sort()).toEqual(["artifact", "native"]); expect(closed.ok).toBe(false); - if (!closed.ok) expect(closed.error.cleanupIncomplete).toBe(true); + if (closed.ok) throw new Error("Expected both child close failures"); + expect(closed.error.cleanupIncomplete).toBe(true); + expect(closed.error.cause).toBeInstanceOf(AggregateError); + expect((closed.error.cause as AggregateError).errors).toEqual([ + expect.objectContaining({ cause: artifactCause }), + expect.objectContaining({ cause: nativeCause }), + ]); }); it("reports the first mixed child failure before awaiting the other owner and retries both", async () => {