Repository navigation
test: move root src tests off the daemon's testing exports - #3728
Conversation
There was a problem hiding this comment.
All reported issues were addressed across 22 files
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
Size Report
Startup median (7 runs, lower is better):
|
|
I found two test gaps in 549ee51 that I'd like fixed before merge. The diff changes no production source, so the risk is lost coverage. Checks are pending: both non-passing Smoke Tests runs are queued, not failed, and I see no script or smoke path that imports the removed daemon testing exports. There are no conflicts. The abort-timer test in src/daemon-client/tests/daemon-client-abort-timeout.test.ts:54 now runs The deleted test "a failing suite reaches the reporter with the failure message, hint fields, and exit code" was the only check of the terminal, non-retrying On the open threads, these still apply: the http-only abort test (#3728 (comment), same as the first finding), the weakened I did not re-run the mutation results, the 108-case count or the full affected check, and the PR says that run is still in progress. I did not diff the moved root tests line by line, though their counts match the table. I also did not confirm that Before merge, please restore the socket abort-timer case and the final-failure hint assertion, then fix the three open review threads above. Smoke Tests must also finish green. |
|
Re #3728 (comment): fixed on |
|
This PR is ready at 46f1c4d. Both gaps from the earlier review (#3728 (comment)) are closed: the composed restart and expired-lease case is back in src/provider-device-runtimes.test.ts, and the socket abort test now asserts the exact armed timer handle was cleared. Not blocking, and you can take or leave these: a second test file for src/provider-device-runtimes.ts now sits beside src/tests/provider-device-runtimes.test.ts, so a name that says what it composes (for example provider-device-runtimes-limrun-lease-restart.test.ts) would help, and the title of the socket abort test in src/daemon-client/tests/daemon-client-abort-timeout.test.ts says it avoids the timeout path, when the clearTimeout handle assertion is what makes it fail on a deleted clear. All 20 checks pass, and this delta touches only test files. There are no conflicts. Nothing else needs to happen before merge. On the open inline threads, these five are fixed at this head and can be resolved: the restart and expired-lease pairing (#3728 (comment)), the socket abort timer handle (#3728 (comment)), the requestId string assertion (#3728 (comment)), the post-start cleanup in public-daemon-runtime.ts (#3728 (comment)), and the onCanceled flag (#3728 (comment)). No other open threads still apply. I judged the delete-clearTimeout and drop- |
Summary
Removes the daemon testing-export imports from the 15 owned root
src/**test files. Root behavior remains beside root modules; daemon logic now has package-local tests; composed CLI/client cases start the daemon through publicstartDaemonRuntime. No public daemon entry, test export, barrel, orexport *was added. R78/R82 remain satisfied.Placement and test counts
Counts are expanded Vitest cases (
test.eachrows expanded). The 15 source paths onorigin/maintotal 108. Vitest lists 101 declarations; the HarmonyOS selection table adds 2 cases and the gateway lifecycle/mismatch tables add 5.origin/mainsrc/__tests__/cli-remote-diagnostics.test.tspackages/remote/src/__tests__/remote-request-diagnostics.test.ts: 4 (existing)src/__tests__/lease-provider-profile-fields.test.tssrc/__tests__/limrun-runtime.test.tspackages/daemon/src/server/__tests__/provider-lease-expiry.test.ts: 4 (existing)src/__tests__/platform-runtime-runtime-hints.test.tspackages/daemon/src/sessions/session-runtime-transport-hints.test.ts: 4src/__tests__/remote-connection-harmonyos.test.tssrc/__tests__/remote-connection-macos-app.test.tssrc/__tests__/takeover-command.test.tssrc/client/human-control.fixtures.tssrc/cli/replay-test/__tests__/session-test-reporter-values-maestro.test.tspackages/daemon/src/sessions/handlers/session-progress.test.ts: 3; existing reporter projectionsrc/cli/replay-test/reporters/__tests__/progress.test.ts: 10src/cli/replay-test/__tests__/session-test-reporter-values.test.tsreporters/__tests__/registry.test.ts: 20src/client/lease-client.test.tssrc/client/human-control.fixtures.tssrc/commands/cli-grammar/flag-declaration-admission.test.tspackages/daemon/src/core/session-action-recorder-admission.test.ts: 1src/daemon-client/__tests__/daemon-client-abort-default-transport.test.tssrc/daemon-client/__tests__/daemon-client-abort-timeout.test.tssrc/daemon-client/__tests__/daemon-client-abort.test.tssrc/platform-runtime-gateway.test.tspackages/daemon/src/sessions/handlers/__tests__/session-appstate-facts.test.ts: 1The lease-profile, HarmonyOS, and macOS-app tests still assert root connection and lease command behavior; their persisted-state imports now come from
@agent-device/remote. The package helper is covered by its existingpackages/remote/src/__tests__/remote-connection-state.test.ts; none of these files tests plugin-package internals.Destination counts for existing files are their full head counts; those files were already selected before this change. The remote request-diagnostics assertion is owned by
packages/remote/src/__tests__/remote-request-diagnostics.test.ts, test “an unreachable record leaves no path and names the daemon and request instead.” The Limrun daemon-retry assertion is owned byprovider-lease-expiry.test.ts, test “retries a persisted expired provider lease after daemon restart.” Recorder and appstate assertions now live in the new package tests named “recorder sanitization follows the declaration: an undeclared key is dropped, a planted declaration is recorded” and “appstate refuses unavailable provider facts before binding the device.”The two deleted reporter files were split, not moved wholesale. Daemon event production is covered by the three tests in
session-progress.test.ts: “native suite progress carries skip, retry hint, and attempt session values,” “Maestro progress carries its title, step payload, and retry attempt identity,” and “sharded suite progress carries each device and shard-scoped session.” Reporter projection remains in the existingprogress.test.tstests for suite start, test start, steps, pass/fail/skip status, and skipped entries. Exit-code assertions remain in registry tests “getExitCode sees the same suite value onSuiteEnd received” and “reporter exit codes can raise but never lower the suite exit code.” No assertion was dropped.The daemon fixture rewrites
daemon.jsonafter public startup to publishcurrentDaemonCodeSignature(), matching existing client fixtures that publish that same signature so those clients accept the test daemon.Selection
vitest.config.tsprojectunit-coreincludessrc/**/*.test.tsandpackages/*/src/**/*.test.ts, which select every root destination and all four new daemon tests. CI selects that project through the registeredunit-cigate (test:coverage:ci) inscripts/check-affected/checks.tsand.github/workflows/ci.yml;check:gate-manifestpassed.Mocks and spies
takeover-command.test.ts: shared-output mock resolves the latch the foreground test awaits; the test then asserts the recorded renew/release requests.limrun-runtime.test.ts:@limrun/apimock has positive create/list/delete call and argument assertions.limrun-runtime.test.ts: iOS instance-client mock factory has positive call/argument assertions.limrun-runtime.test.ts: Android instance-client mock factory has positive call/argument assertions.limrun-runtime.test.ts: partialhost-kit/commandmock recordsrunCmdcalls and arguments asserted by the tests.lease-provider-profile-fields.test.ts:@limrun/apifail-fastiosCreatemock is guarded by negative assertions; its break-proof bypassedrejectRefusedLimrunProfileFieldsand the focused test failed.daemon-client-abort-timeout.test.ts: timeout-seam mock has a positive no-signal control (handleRequestTimeoutcalled once); breaking abort timer clearing makes the paired abort test fail.platform-runtime-gateway.test.ts:inspectFactsspy has a positive once assertion.platform-runtime-gateway.test.ts:bindspy is negative-only; its break-proof injected an owner bind and the focused test failed.The former
snapshot-interactor-capturemock was deleted with the rewritten reporter test; no mock for that subpath survives in this scope.Mutation checks
Each listed behavior was broken, its focused test failed, and the mutation was reverted (18 destination checks):
src/__tests__/cli-remote-diagnostics.test.ts—localizeRemoteDaemonError.src/__tests__/lease-provider-profile-fields.test.ts—rejectRefusedLimrunProfileFields.src/__tests__/limrun-runtime.test.ts—recoverExpiredLease.src/__tests__/platform-runtime-runtime-hints.test.ts—applyRuntimeHintValues.src/__tests__/remote-connection-harmonyos.test.ts—resolveRequestedLeaseBackend.src/__tests__/remote-connection-macos-app.test.ts—materializeRemoteConnectionForCommand.src/__tests__/takeover-command.test.ts—takeoverCommand.src/client/lease-client.test.ts—createAgentDeviceClient.src/commands/cli-grammar/flag-declaration-admission.test.ts—resolveConfigBackedFlagDefaults.src/daemon-client/__tests__/daemon-client-abort-default-transport.test.ts—sendRequest.src/daemon-client/__tests__/daemon-client-abort-timeout.test.ts—sendRequest.src/daemon-client/__tests__/daemon-client-abort.test.ts—sendRequest.src/platform-runtime-gateway.test.ts—createComposedPlatformRuntimeGateway.packages/daemon/src/core/session-action-recorder-admission.test.ts—recordActionEntry.packages/daemon/src/sessions/handlers/__tests__/session-appstate-facts.test.ts—handleAppStateCommand.packages/daemon/src/sessions/handlers/session-progress.test.ts—handleReplayTestCommand.packages/daemon/src/sessions/session-runtime-transport-hints.test.ts—applyDeviceDefaultMetroHost.src/cli/replay-test/reporters/__tests__/progress.test.ts—toReplayTestReporterProgressEvent.Exports and remaining owners
Daemon exports go from 78 to 71 total (77 to 70
./testing/*entries). This PR removes:./testing/__tests__/human-control-fixtures./testing/__tests__/platform-services-fixture./testing/handlers/__tests__/*./testing/handlers/lease./testing/handlers/session-appstate./testing/server/transport./testing/session-runtimeThese ten original subpaths remain in use by other owners:
./testing/__tests__/support/session-factories— provider scenariosios-fold.test.tsandios-wait-selector.test.ts../testing/__tests__/support/store-factory— stale provider-admission fixture../testing/lease-registry— nightly concurrency torture and provider-scenario harness/parity tests../testing/provider-lease-expiry— cloud WebDriver runtime scenario../testing/request-dispatch-ledger— stale provider-admission fixture../testing/request-runtime-binding— stale provider-admission fixture../testing/server/http-server— provider-scenario HTTP, remote-proxy, and artifact tests../testing/session-action-recorder— iOS fold integration../testing/session-store— nightly concurrency torture and provider-scenario harness../testing/snapshot-interactor-capture—src/__tests__/daemon-composition/**, owner threadmcp:57b5c3dd, branchrefactor/daemon-testing-exports-composition.Validation
Before the rebase, head
e4ff5146epassedpnpm install --frozen-lockfile && pnpm build, repository-widepnpm format,pnpm check:quick,pnpm check:layering,pnpm check:fallow,pnpm check:gate-manifest, andpnpm check:affected --run. Thatcheck:affectedrun passed the full unit run (1,854 files; 15,564 passed, 1 skipped), integration-node (137 passed, 12 skipped), and all other selected gates. On rebased head549ee5168,CI=true pnpm install --frozen-lockfile && pnpm build,pnpm check:quick, andpnpm check:layeringpassed; the 18 affected Vitest files passed (119 tests). The fullcheck:affected --runis being rerun on this rebased head.Commits
git diff -M90% --summary origin/main...HEADreports no renames: the two reporter files were rewritten/split and the four daemon tests were added. No file moved wholesale, so the rebased test migration commita653fe44edid not need splitting. The export deletion is isolated in549ee5168.🤖 Generated with Claude Code