From a836be12e438b73ffcc220ee926e90560e934105 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 24 Sep 2026 20:35:45 +0200 Subject: [PATCH] test: drop cases that re-assert their own input or a stronger sibling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Eight tests could not fail for a reason worth hearing. Each was checked against its production owner, its callers, and the stronger test that already covers it. - `ax-digest.test.ts`: `computeAxDigest(nodes)` compared against a second call to the same pure function — true by construction. The empty-array case kept its `nodeCount` assertion and lost its self-comparison. - `ios-snapshot-engine/conformance.test.ts`: replayed `contracts/fixtures/ios-snapshot-capture-hint.json` through `createIosSnapshotRequest` + `deriveIosCaptureHint` with the same four assertions `ios-snapshot-planning.test.ts` makes one file away. The golden table keeps its owning leg; only the second copy goes. - `replay-coordinator-ownership.test.ts`: the divergence-chain import ban is a strict subset of the `src/daemon/replay/`-wide scan beside it, same module and same predicate, so it cannot fire without that one firing first. - `device-claim-rule.test.ts`: compared production against a hand-copied test table of the three rules the same file already asserts directly. Exhaustiveness is production's own default-less switch. - `snapshot-viewport-root.test.ts`: over the complete 31-name iOS vocabulary the preceding case fixes, "equals `Application` or `Window`" is the same proposition as "filters to exactly those two". - `install-source-public.test.ts`: asserted the URL its sibling asserts at the owner, plus a `typeof` probe. The subpath is proven by `package-exports` and by the installed-tarball probe that calls through it. - `metro-protocol-public.test.ts`: its runtime half repeated `metro-public.test.ts` verbatim; the compile-time wire fixtures it also carried move there, so the public metro surface has one test file mirroring `src/sdk/metro.ts`. - `session-replay-command-trust.test.ts`: mocked the replay façade and read the command field back. `session-replay-maestro-remote-evalscript.test.ts` drives the same two real handlers and asserts the terminal `trustedScripts` the engine receives. Proven by mutation: dropping the mapping in `replay-dispatch-envelope.ts` turns that suite's two remote cases red. --- .../ios-snapshot-engine/conformance.test.ts | 34 ----- .../src/snapshot/__tests__/ax-digest.test.ts | 16 +-- .../src/snapshot-viewport-root.test.ts | 7 -- src/__tests__/install-source-public.test.ts | 12 -- src/__tests__/metro-protocol-public.test.ts | 52 -------- src/__tests__/metro-public.test.ts | 43 ++++++- .../replay-coordinator-ownership.test.ts | 14 --- .../__tests__/device-claim-rule.test.ts | 21 +--- .../session-replay-command-trust.test.ts | 117 ------------------ 9 files changed, 48 insertions(+), 268 deletions(-) delete mode 100644 src/__tests__/install-source-public.test.ts delete mode 100644 src/__tests__/metro-protocol-public.test.ts delete mode 100644 src/daemon/handlers/__tests__/session-replay-command-trust.test.ts diff --git a/packages/capture-kit/src/ios-snapshot-engine/conformance.test.ts b/packages/capture-kit/src/ios-snapshot-engine/conformance.test.ts index c590b07415..76d4c27bbe 100644 --- a/packages/capture-kit/src/ios-snapshot-engine/conformance.test.ts +++ b/packages/capture-kit/src/ios-snapshot-engine/conformance.test.ts @@ -2,11 +2,6 @@ import fs from 'node:fs'; import assert from 'node:assert/strict'; import path from 'node:path'; import { test } from 'vitest'; -import type { CaptureHint, IosSnapshotRequestInput } from '@agent-device/contracts/ios-snapshot'; -import { - createIosSnapshotRequest, - deriveIosCaptureHint, -} from '@agent-device/capture-kit/ios-snapshot-planning'; import { IosSnapshotEngineError, presentIosSnapshot, publishIosSnapshot } from './index.ts'; import { runTypeScriptCase, writeDifferentialFailureArtifact } from './conformance-harness.ts'; import { @@ -16,35 +11,6 @@ import { requestForGoldenCase, } from './conformance-fixture.ts'; -const CAPTURE_HINT_FIXTURE_PATH = path.resolve( - import.meta.dirname, - '..', - '..', - '..', - '..', - 'contracts', - 'fixtures', - 'ios-snapshot-capture-hint.json', -); - -type CaptureHintFixture = Readonly<{ - name: string; - request: IosSnapshotRequestInput; - expected: CaptureHint; -}>; - -test('the independent capture-hint corpus agrees with the engine request boundary', () => { - const fixtures = JSON.parse( - fs.readFileSync(CAPTURE_HINT_FIXTURE_PATH, 'utf8'), - ) as CaptureHintFixture[]; - assert.ok(fixtures.length > 0); - assert.equal(new Set(fixtures.map((fixture) => fixture.name)).size, fixtures.length); - for (const fixture of fixtures) { - const request = createIosSnapshotRequest(fixture.request); - assert.deepEqual(deriveIosCaptureHint(request), fixture.expected, fixture.name); - } -}); - test('the authored iOS snapshot corpus covers each contract seam', () => { const fixture = readIosSnapshotEngineFixture(); assert.equal(fixture.version, 1); diff --git a/packages/capture-kit/src/snapshot/__tests__/ax-digest.test.ts b/packages/capture-kit/src/snapshot/__tests__/ax-digest.test.ts index bf739418dc..b92de8d5db 100644 --- a/packages/capture-kit/src/snapshot/__tests__/ax-digest.test.ts +++ b/packages/capture-kit/src/snapshot/__tests__/ax-digest.test.ts @@ -1,15 +1,6 @@ import { expect, test } from 'vitest'; import { computeAxDigest } from '../snapshot-evidence.ts'; -test('digest is stable across repeated calls for the same nodes', () => { - const nodes = [ - { type: 'button', label: 'Continue', identifier: 'continue-btn' }, - { type: 'text', label: 'Welcome' }, - ]; - - expect(computeAxDigest(nodes)).toEqual(computeAxDigest(nodes)); -}); - test('digest is order-independent over the node multiset', () => { const a = [ { type: 'button', label: 'Continue', identifier: 'continue-btn' }, @@ -54,11 +45,8 @@ test('digest changes when node count changes even with the same multiset otherwi expect(one.nodeCount).toBe(1); }); -test('digest for an empty node array is stable and reports zero nodes', () => { - const result = computeAxDigest([]); - - expect(result.nodeCount).toBe(0); - expect(result.digest).toBe(computeAxDigest([]).digest); +test('digest reports zero nodes for an empty node array', () => { + expect(computeAxDigest([]).nodeCount).toBe(0); }); test('digest is prefixed for forward-compatible versioning', () => { diff --git a/packages/contracts/src/snapshot-viewport-root.test.ts b/packages/contracts/src/snapshot-viewport-root.test.ts index 2e8a92ec02..5e0bd362aa 100644 --- a/packages/contracts/src/snapshot-viewport-root.test.ts +++ b/packages/contracts/src/snapshot-viewport-root.test.ts @@ -167,13 +167,6 @@ describe('isViewportRootNode over emitted backend vocabulary', () => { expect(roots).toEqual(['Application', 'Window']); }); - test('iOS: substring and equality agree, so the collapse of the `===` spelling was a no-op', () => { - for (const type of IOS_EMITTED_TYPES) { - const equality = type === 'Application' || type === 'Window'; - expect({ type, root: isViewportRootNode({ type }) }).toEqual({ type, root: equality }); - } - }); - // The load-bearing one. Android has no root node, so `resolveViewportRect`'s // third fallback (largest containing rect of any node) is the only arm that // ever returns on Android — and the resolvers that lack it return null there. diff --git a/src/__tests__/install-source-public.test.ts b/src/__tests__/install-source-public.test.ts deleted file mode 100644 index bc388482fa..0000000000 --- a/src/__tests__/install-source-public.test.ts +++ /dev/null @@ -1,12 +0,0 @@ -import assert from 'node:assert/strict'; -import { test } from 'vitest'; - -import { isTrustedInstallSourceUrl, validateDownloadSourceUrl } from '../sdk/install-source.ts'; - -test('public install-source entrypoint re-exports pure helpers', () => { - assert.equal( - isTrustedInstallSourceUrl('https://api.github.com/repos/acme/app/actions/artifacts/1/zip'), - true, - ); - assert.equal(typeof validateDownloadSourceUrl, 'function'); -}); diff --git a/src/__tests__/metro-protocol-public.test.ts b/src/__tests__/metro-protocol-public.test.ts deleted file mode 100644 index 32dd658cca..0000000000 --- a/src/__tests__/metro-protocol-public.test.ts +++ /dev/null @@ -1,52 +0,0 @@ -import { test } from 'vitest'; -import assert from 'node:assert/strict'; -import { - buildBundleUrl, - normalizeBaseUrl, - type MetroBridgeDescriptor, - type MetroTunnelRequestMessage, - type MetroTunnelResponseMessage, -} from '../sdk/metro.ts'; - -// Type-only contract fixtures — these verify that the public subpath types -// remain structurally stable. A rename or breaking shape change will fail -// the compile, not a runtime assertion. -({ - enabled: true, - base_url: 'https://bridge.example.test', - ios_runtime: { - metro_host: 'runtime-1.metro.agent-device.dev', - metro_port: 443, - metro_bundle_url: 'https://runtime-1.metro.agent-device.dev/index.bundle?platform=ios', - }, - android_runtime: { - metro_host: 'bridge.example.test', - metro_port: 443, - metro_bundle_url: - 'https://bridge.example.test/api/metro/runtimes/runtime-1/index.bundle?platform=android', - }, - upstream: { bundle_url: 'http://127.0.0.1:8081/index.bundle?platform=ios' }, - probe: { reachable: true, status_code: 200, latency_ms: 4, detail: 'ok' }, -}) satisfies MetroBridgeDescriptor; - -({ - type: 'ws-frame', - streamId: 'stream-1', - dataBase64: 'aGVsbG8=', - binary: false, -}) satisfies MetroTunnelRequestMessage; - -({ - type: 'http-response', - requestId: 'req-1', - status: 200, - headers: { 'content-type': 'application/json' }, -}) satisfies MetroTunnelResponseMessage; - -test('public metro exports expose stable url helpers', () => { - assert.equal(normalizeBaseUrl('https://bridge.example.test///'), 'https://bridge.example.test'); - assert.equal( - buildBundleUrl('https://bridge.example.test/', 'ios'), - 'https://bridge.example.test/index.bundle?platform=ios&dev=true&minify=false', - ); -}); diff --git a/src/__tests__/metro-public.test.ts b/src/__tests__/metro-public.test.ts index 1713c73866..4860388579 100644 --- a/src/__tests__/metro-public.test.ts +++ b/src/__tests__/metro-public.test.ts @@ -1,6 +1,13 @@ import { test } from 'vitest'; import assert from 'node:assert/strict'; -import { buildBundleUrl, normalizeBaseUrl, resolveRuntimeTransport } from '../sdk/metro.ts'; +import { + buildBundleUrl, + normalizeBaseUrl, + resolveRuntimeTransport, + type MetroBridgeDescriptor, + type MetroTunnelRequestMessage, + type MetroTunnelResponseMessage, +} from '../sdk/metro.ts'; test('public metro entrypoint exposes url and transport helpers', () => { assert.equal(normalizeBaseUrl('https://bridge.example.test///'), 'https://bridge.example.test'); @@ -27,3 +34,37 @@ test('public metro entrypoint does not expose runtime hint builders', async () = assert.equal(metro.buildIosRuntimeHints, undefined); assert.equal(metro.buildAndroidRuntimeHints, undefined); }); + +// Type-only contract fixtures — these verify that the public subpath types remain structurally +// stable. A rename or breaking shape change will fail the compile, not a runtime assertion. +({ + enabled: true, + base_url: 'https://bridge.example.test', + ios_runtime: { + metro_host: 'runtime-1.metro.agent-device.dev', + metro_port: 443, + metro_bundle_url: 'https://runtime-1.metro.agent-device.dev/index.bundle?platform=ios', + }, + android_runtime: { + metro_host: 'bridge.example.test', + metro_port: 443, + metro_bundle_url: + 'https://bridge.example.test/api/metro/runtimes/runtime-1/index.bundle?platform=android', + }, + upstream: { bundle_url: 'http://127.0.0.1:8081/index.bundle?platform=ios' }, + probe: { reachable: true, status_code: 200, latency_ms: 4, detail: 'ok' }, +}) satisfies MetroBridgeDescriptor; + +({ + type: 'ws-frame', + streamId: 'stream-1', + dataBase64: 'aGVsbG8=', + binary: false, +}) satisfies MetroTunnelRequestMessage; + +({ + type: 'http-response', + requestId: 'req-1', + status: 200, + headers: { 'content-type': 'application/json' }, +}) satisfies MetroTunnelResponseMessage; diff --git a/src/daemon/__tests__/replay-coordinator-ownership.test.ts b/src/daemon/__tests__/replay-coordinator-ownership.test.ts index 84bec5481d..2f4f94304c 100644 --- a/src/daemon/__tests__/replay-coordinator-ownership.test.ts +++ b/src/daemon/__tests__/replay-coordinator-ownership.test.ts @@ -429,20 +429,6 @@ test('the divergence-report chain never imports the coordinator factory', () => } }); -test('the divergence-report chain never imports the P4a ReplaySessionTransaction projection', () => { - for (const file of DIVERGENCE_CHAIN_FILES) { - for (const site of collectImportSites(file)) { - assert.equal( - importsAnyBinding(site, TRANSACTION_MODULE), - false, - `${file} imports session-replay-transaction.ts directly — repair-transaction writes must ` + - `route through the request's ReplayCoordinator/ReplayResumeStamper, never this P4a ` + - `projection reached from a lower handler.`, - ); - } - } -}); - test('the divergence-report chain never imports SessionStore', () => { for (const file of DIVERGENCE_CHAIN_FILES) { for (const site of collectImportSites(file)) { diff --git a/src/daemon/device/__tests__/device-claim-rule.test.ts b/src/daemon/device/__tests__/device-claim-rule.test.ts index 31c5380f51..106d274845 100644 --- a/src/daemon/device/__tests__/device-claim-rule.test.ts +++ b/src/daemon/device/__tests__/device-claim-rule.test.ts @@ -4,27 +4,14 @@ import { localRuntimeOwner, managedLocalRuntimeOwner, providerRuntimeOwner, - type RuntimeOwnerRef, } from '@agent-device/contracts/platform-runtime'; -import { deviceClaimRuleForOwner, type DeviceClaimRule } from '../device-claim-rule.ts'; - -// Every owner kind names its rule here, so a fourth kind fails to compile until it does. -const RULE_BY_OWNER_KIND: Readonly> = { - 'local-family': 'ordinary', - 'managed-local': 'allocator-held', - 'provider-runtime': 'none', -}; +import { deviceClaimRuleForOwner } from '../device-claim-rule.ts'; +// Production owns the exhaustiveness: `deviceClaimRuleForOwner` is a switch with no default arm, so +// a fourth owner kind fails to compile until it names a rule. This table would restate that claim +// as test data that no caller reads. test('the device-claim rule selects ordinary for a local family, allocator-held for a managed local owner, and none for a provider runtime', () => { assert.equal(deviceClaimRuleForOwner(localRuntimeOwner('android')), 'ordinary'); assert.equal(deviceClaimRuleForOwner(managedLocalRuntimeOwner('sim-a')), 'allocator-held'); assert.equal(deviceClaimRuleForOwner(providerRuntimeOwner('limrun', 'instance-1')), 'none'); }); - -test.each([ - localRuntimeOwner('apple'), - managedLocalRuntimeOwner('sim-b'), - providerRuntimeOwner('webdriver', 'tenant-a'), -])('the device-claim rule is exhaustive over owner kinds: $kind', (owner) => { - assert.equal(deviceClaimRuleForOwner(owner), RULE_BY_OWNER_KIND[owner.kind]); -}); diff --git a/src/daemon/handlers/__tests__/session-replay-command-trust.test.ts b/src/daemon/handlers/__tests__/session-replay-command-trust.test.ts deleted file mode 100644 index 67f0297e48..0000000000 --- a/src/daemon/handlers/__tests__/session-replay-command-trust.test.ts +++ /dev/null @@ -1,117 +0,0 @@ -/** - * #2663 PR A: the daemon handler is the ONLY producer of `ReplayCommand`, and the sole place - * that turns the request-private `internal.publicNetworkOnly` into the Maestro script-trust - * command input. If a later edit — such as the PR B move — drops the mapping on either entry - * point, the field reads absent, the engine trusts the flow, and a remote HTTP replay would run - * `evalScript`. These assertions pin the mapping for BOTH `replay` and `replay test` so a dropped - * line fails here instead of failing open. The engine-level assertions for both routes live in - * `session-replay-maestro-remote-evalscript.test.ts`, routed through these handlers. - */ -import path from 'node:path'; -import { beforeEach, expect, test, vi } from 'vitest'; -import { LeaseRegistry } from '../../lease-registry.ts'; -import { platformResourceCleanup } from '../../../platform-runtime-resource-cleanup.ts'; -import { createScreenRecordingAdmissionLedger } from '@agent-device/capture-kit/screen-recording-admission-ledger'; -import { - unavailableBindDevice, - unavailableBindExactDevice, -} from '../../__tests__/test-device-runtime-gateway.ts'; -import type { ReplayCommand, ReplayTestCommand } from '../../replay/internal/command-types.ts'; -import type { SessionCommandParams } from '../session-command-input.ts'; -import { makeSessionStore } from './session-test-harness.ts'; -import { mkdtempForTestSync } from '../../../__tests__/test-utils/tmp-dir.ts'; - -// Hoisted: the handler reaches the replay façade through a daemon module that this file's own -// imports pull in, so the factory below runs before a plain `const` would be initialized. -const { runReplayCommand, runReplayTestCommand } = vi.hoisted(() => ({ - runReplayCommand: vi.fn(async (_command: ReplayCommand) => ({ - ok: true as const, - data: {}, - })), - runReplayTestCommand: vi.fn(async (_command: ReplayTestCommand) => ({ - ok: true as const, - data: {}, - })), -})); - -// The doubles replace only the two entry points. The handler also binds the session and splits the -// request through this façade, and those bindings are part of what is pinned here, so the rest of -// the module stays real. -vi.mock('../../replay/index.ts', async (importOriginal) => ({ - ...(await importOriginal()), - runReplayCommand, - runReplayTestCommand, -})); - -const { handleReplayCommand, handleReplayTestCommand } = - await import('../session-replay-command.ts'); - -beforeEach(() => { - runReplayCommand.mockClear(); - runReplayTestCommand.mockClear(); -}); - -function baseParams( - command: string, - internal?: { publicNetworkOnly?: true }, -): SessionCommandParams { - const root = mkdtempForTestSync('agent-device-replay-trust-'); - return { - req: { - token: 'token', - session: 'default', - command, - positionals: [path.join(root, 'flow.ad')], - flags: { platform: 'ios' }, - ...(internal ? { internal } : {}), - meta: { cwd: root, requestId: `replay-trust-${command}` }, - }, - sessionName: 'default', - logPath: path.join(root, 'daemon.log'), - sessionStore: makeSessionStore(), - leaseRegistry: new LeaseRegistry(), - invoke: async () => ({ ok: true as const, data: {} }), - reconcileOrphanedDeviceClaim: async () => ({ - status: 'retained' as const, - reason: 'test' as const, - }), - } as unknown as SessionCommandParams; -} - -function testParams(internal?: { publicNetworkOnly?: true }): SessionCommandParams { - return { - ...baseParams('test', internal), - bindDevice: unavailableBindDevice, - bindExactDevice: unavailableBindExactDevice, - inspectFacts: async () => undefined, - screenRecordingAdmissionLedger: createScreenRecordingAdmissionLedger(), - requestScope: { - signal: new AbortController().signal, - diagnostics: { emit: () => {} }, - progress: { report: () => {} }, - }, - retainDeviceExecutionLock: async () => {}, - throwIfCanceled: () => {}, - platformResourceCleanup, - } as unknown as SessionCommandParams; -} - -test('replay handler maps internal.publicNetworkOnly to the command input', async () => { - await handleReplayCommand(baseParams('replay', { publicNetworkOnly: true })); - expect(runReplayCommand.mock.calls[0]?.[0].publicNetworkOnly).toBe(true); -}); - -test('replay handler marks a local request as trusted', async () => { - await handleReplayCommand(baseParams('replay')); - expect(runReplayCommand.mock.calls[0]?.[0].publicNetworkOnly).toBeUndefined(); -}); - -test('replay test handler maps internal.publicNetworkOnly to the command input', async () => { - await handleReplayTestCommand(testParams({ publicNetworkOnly: true })); - expect(runReplayTestCommand.mock.calls[0]?.[0].publicNetworkOnly).toBe(true); -}); - -test('replay test handler marks a local request as trusted', async () => { - await handleReplayTestCommand(testParams()); - expect(runReplayTestCommand.mock.calls[0]?.[0].publicNetworkOnly).toBeUndefined(); -});