Skip to content

test: move root src tests off the daemon's testing exports - #3728

Merged
thymikee merged 5 commits into
mainfrom
refactor/daemon-testing-exports-root-src
Oct 16, 2026
Merged

thymikee merged 5 commits into
mainfrom
refactor/daemon-testing-exports-root-src

Conversation

@thymikee

@thymikee thymikee commented Oct 16, 2026 •

Copy link
Copy Markdown
Member

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 public startDaemonRuntime. No public daemon entry, test export, barrel, or export * was added. R78/R82 remain satisfied.

Placement and test counts

Counts are expanded Vitest cases (test.each rows expanded). The 15 source paths on origin/main total 108. Vitest lists 101 declarations; the HarmonyOS selection table adds 2 cases and the gateway lifecycle/mismatch tables add 5.

Source on origin/main Cases Destination(s) on this head; full file cases Class
src/__tests__/cli-remote-diagnostics.test.ts 4 Same path: 3; packages/remote/src/__tests__/remote-request-diagnostics.test.ts: 4 (existing) b
src/__tests__/lease-provider-profile-fields.test.ts 2 Same path: 2 b
src/__tests__/limrun-runtime.test.ts 25 Same path: 25; daemon retry behavior remains in packages/daemon/src/server/__tests__/provider-lease-expiry.test.ts: 4 (existing) c
src/__tests__/platform-runtime-runtime-hints.test.ts 15 Same path: 15; packages/daemon/src/sessions/session-runtime-transport-hints.test.ts: 4 a+c
src/__tests__/remote-connection-harmonyos.test.ts 6 Same path: 6 c
src/__tests__/remote-connection-macos-app.test.ts 8 Same path: 8 c
src/__tests__/takeover-command.test.ts 5 Same path: 5; human-control fixture now lives in src/client/human-control.fixtures.ts c
src/cli/replay-test/__tests__/session-test-reporter-values-maestro.test.ts 2 packages/daemon/src/sessions/handlers/session-progress.test.ts: 3; existing reporter projection src/cli/replay-test/reporters/__tests__/progress.test.ts: 10 a+c
src/cli/replay-test/__tests__/session-test-reporter-values.test.ts 4 Same package progress test: 3; existing reporter projection: 10; existing reporters/__tests__/registry.test.ts: 20 a+c
src/client/lease-client.test.ts 4 Same path: 4; human-control fixture now lives in src/client/human-control.fixtures.ts c
src/commands/cli-grammar/flag-declaration-admission.test.ts 4 Same path: 3; packages/daemon/src/core/session-action-recorder-admission.test.ts: 1 a+c
src/daemon-client/__tests__/daemon-client-abort-default-transport.test.ts 2 Same path: 2 b
src/daemon-client/__tests__/daemon-client-abort-timeout.test.ts 3 Same path: 3 c
src/daemon-client/__tests__/daemon-client-abort.test.ts 4 Same path: 4 b
src/platform-runtime-gateway.test.ts 20 Same path: 20; packages/daemon/src/sessions/handlers/__tests__/session-appstate-facts.test.ts: 1 a+c

The 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 existing packages/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 by provider-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 existing progress.test.ts tests 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.json after public startup to publish currentDaemonCodeSignature(), matching existing client fixtures that publish that same signature so those clients accept the test daemon.

Selection

vitest.config.ts project unit-core includes src/**/*.test.ts and packages/*/src/**/*.test.ts, which select every root destination and all four new daemon tests. CI selects that project through the registered unit-ci gate (test:coverage:ci) in scripts/check-affected/checks.ts and .github/workflows/ci.yml; check:gate-manifest passed.

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/api mock 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: partial host-kit/command mock records runCmd calls and arguments asserted by the tests.
  • lease-provider-profile-fields.test.ts: @limrun/api fail-fast iosCreate mock is guarded by negative assertions; its break-proof bypassed rejectRefusedLimrunProfileFields and the focused test failed.
  • daemon-client-abort-timeout.test.ts: timeout-seam mock has a positive no-signal control (handleRequestTimeout called once); breaking abort timer clearing makes the paired abort test fail.
  • platform-runtime-gateway.test.ts: inspectFacts spy has a positive once assertion.
  • platform-runtime-gateway.test.ts: bind spy is negative-only; its break-proof injected an owner bind and the focused test failed.

The former snapshot-interactor-capture mock 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-runtime

These ten original subpaths remain in use by other owners:

  • ./testing/__tests__/support/session-factories — provider scenarios ios-fold.test.ts and ios-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 thread mcp:57b5c3dd, branch refactor/daemon-testing-exports-composition.

Validation

Before the rebase, head e4ff5146e passed pnpm install --frozen-lockfile && pnpm build, repository-wide pnpm format, pnpm check:quick, pnpm check:layering, pnpm check:fallow, pnpm check:gate-manifest, and pnpm check:affected --run. That check:affected run 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 head 549ee5168, CI=true pnpm install --frozen-lockfile && pnpm build, pnpm check:quick, and pnpm check:layering passed; the 18 affected Vitest files passed (119 tests). The full check:affected --run is being rerun on this rebased head.

Commits

git diff -M90% --summary origin/main...HEAD reports 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 commit a653fe44e did not need splitting. The export deletion is isolated in 549ee5168.

🤖 Generated with Claude Code

View guided diff Turn on auto-fix

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/__tests__/limrun-runtime.test.ts Outdated
Comment thread src/daemon-client/__tests__/daemon-client-abort-timeout.test.ts Outdated
Comment thread src/__tests__/test-utils/public-daemon-runtime.ts Outdated
Comment thread src/daemon-client/__tests__/daemon-client-abort-default-transport.test.ts Outdated
@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Size Report

Metric Base Current Diff
Installed (including dependencies) 5.30 MB 5.30 MB -439 B
Package (unpacked) 5.30 MB 5.30 MB -439 B
Package (download) 1.59 MB 1.59 MB -127 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 26.5 ms 26.9 ms +0.4 ms
CLI --help 78.1 ms 76.8 ms -1.3 ms

@thymikee

Copy link
Copy Markdown
Member Author

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 sendRequest over http only. The old socket variant was the only guard for the separate socket abort path, where rejectAborted clears its timer at https://github.com/callstack/agent-device/blob/549ee51/src/daemon-client/daemon-client-transport.ts#L631. If that clearTimeout is deleted, the timer fires after the abort and runs the runner sweep and daemon reset in handleRequestTimeout, and every remaining test still passes. Each transport's own abort path needs a timer-cleared assertion. Please parametrize runTimerDiscipline over socket and http, using a raw net.createServer that never replies with trackLoopbackSockets, so no daemon testing export is needed. Then run the delete-clearTimeout mutation on the socket path.

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 fail event. That event comes from buildReplayTestFailedResult at https://github.com/callstack/agent-device/blob/549ee51/packages/replay-test/src/internal/session-test-attempt.ts#L359-L380, which sets hint: error.hint. The three new tests in packages/daemon/src/sessions/handlers/session-progress.test.ts check hint and message only on the retrying emit, and nothing else in the head asserts the try replay --from hint. Dropping hint: error.hint would now pass every test and strip the hint from every non-retried failure in reporter output. Please add a single-attempt failing-suite case where invoke returns {ok:false, error:{code:'ASSERTION_FAILED', message, hint}}. It should assert a fail event with retrying undefined, attempt 1, maxAttempts 1, the message and the hint, plus data.failed of 1. The PR body's "No assertion was dropped" line is then accurate. Today it is not.

On the open threads, these still apply: the http-only abort test (#3728 (comment), same as the first finding), the weakened requestId guard (#3728 (comment), restore typeof details.requestId === 'string'), the runtime leak in public-daemon-runtime.ts (#3728 (comment)), and the onCanceled assert that can skip reject (#3728 (comment)). One thread does not apply and can be resolved: the daemon restart retry is covered by provider-lease-expiry.test.ts:48 and Limrun idempotent recovery by limrun-runtime.test.ts, so only the composed pairing is gone, which the daemon-composition thread covers (#3728 (comment)). A rename of that test, since it no longer involves the daemon, would be enough.

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 remote-request-diagnostics.test.ts renders through the real CLI like the deleted case did.

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.

@thymikee

Copy link
Copy Markdown
Member Author

Re #3728 (comment): fixed on 46f1c4d9f. The terminal-failure test checks the non-retrying event's message and try replay --from hint, and removing hint: error.hint made it fail. The real CLI diagnostics test remains in src/__tests__/cli-remote-diagnostics.test.ts, runs against a daemon started through the public runtime, and passed in the focused run. The exact-head affected run passed (15,606 passed, 0 failed, 1 skipped).

@thymikee

Copy link
Copy Markdown
Member Author

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-hint: error.hint mutations from the source and the assertions, and I did not run them. I also did not run the new Limrun restart test, since I only read its state-file name, env var and runtime wiring. I did not re-run the full affected suite or the 108-case count, and I did not diff every moved root test line outside this delta.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 16, 2026
@thymikee
thymikee merged commit 5df2303 into main Oct 16, 2026
20 of 21 checks passed
@thymikee
thymikee deleted the refactor/daemon-testing-exports-root-src branch October 16, 2026 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant