Skip to content

Surviving mutations in the crank-rollback / GC / vat-lifecycle stack's tests #1075

Description

@sirtimid

Collected from a mutation-style review of #1021, #1022 and #1023. Each item names the exact mutation that survives the suite today. Ordered by the severity of the bug it lets through.

#1023

  1. performVatRestart never throws out of the crank is unpinned. Mutation: append throw error; after settle?.reject(error). Everything passes, because the test harness mocks enqueueRestartVat: vi.fn((vatId) => { vatManager.performVatRestart(vatId).catch(() => undefined); }) — the .catch swallows exactly the failure the tests exist to detect. Its comment claims "a regression on that shows up as a failing assertion"; it does not. Fix: record the outcome (.then(() => 'completed', (e) => e)) and assert it. See A failing performVatRestart catch can escape the crank and replay the same failing restart forever #1065 for the underlying bug.
  2. terminateVat has no fallback mark when a death write throws, and no test. Make deleteVat throw during terminateVat: the handle is gone, the store is unmarked, isVatActive still true → the next delivery kills the run loop. See stopVat records a vat's death before killing its worker, so a failed record strands a live worker #1067.
  3. The terminateVatstopVat swap in Kernel.ts has no test. Mutation: revert to terminateVat. Kernel.test.ts mocks KernelQueue wholesale, so the callback is never invoked from a real crank; a revert hangs the kernel silently (withStoreOutOfCrank would wait for the crank that is calling it).
  4. The whole #vatsInFlux / provideVat mechanism can be deleted and the suite passes. The intended pin cannot see the difference: withStoreOutOfCrank is mocked as async (work) => work(), so #retireVat has already run synchronously by the time the test calls provideVat. Pin the ordering — flag from the promise's catch, drainMicrotasks(), assert not settled, then release. Also unpinned: Kernel.ts's getVatprovideVat swap in #getEndpoint.
  5. enqueueRestartVat waking a sleeping run loop is unpinned. Mutation: call kernelStore.enqueueRun directly instead of #enqueueRun. Consequence: a restart requested while the queue is empty never runs until unrelated traffic arrives.
  6. performVatRestart's !#vats.has early return can be deleted; both existing assertions still hold. Pin with expect(deleteVat).toHaveBeenCalledTimes(1).
  7. getImporters' deduplication can be dropped: the new test calls deleteVat('v2') before markVatAsTerminated, so the duplicate never arises. Swap the two calls.
  8. gc-finalize.ts's two await delay(0) drains have no test; the integration test that would have noticed was reworked into a retry loop that masks the latency.

#1022

  1. A remote that refuses a GC delivery — the test asserts only the returned { didDelivery: 'r1' }. Mutation: move the live.forEach teardown after the successful await endpoint[method](erefs). Everything passes, while in production the action is already spent and the kref is never freed.
  2. No real-store test that an aborted GC delivery restores the release. The existing test proves the router returns { abort: true, terminate } against a mock store. gc-delivery.test.ts already has the fixture to prove the store comes back; retireExports is the case that matters, since it is the only one that also orphans.
  3. Every new error log in the PR is unasserted — four logger?.error calls, all deletable with a green suite, in a PR whose framing is "stop hiding GC delivery failures". Both seams (makeKernelStore(kdb, logger?), new KernelRouter(…, logger?)) already exist.
  4. Remote GC delivery has no real-store coverage at all; gc-delivery.test.ts registers only vats, and the remote leg is where the two divergent branches live. One added test closes this and item 9.
  5. The sort-before-harden fix pins the non-throwing half but not the ordering half — setGCActions sorts upstream, so deleting krefs.sort() outright passes.

#1021

  1. RemoteHandle.handleRemoteMessage is never tested against an open crank. Mutation: replace await withStoreOutOfCrank(() => {…}) with an immediately-invoked version. Every test passes — the two new turn tests assert outOfCrankWorkPending() is undefined after the call, which is trivially true when no turn was taken. RemoteManager.test.ts does open a crank first; RemoteHandle, which carries every ordinary deliver, does not.
  2. The wasm driver no longer exercises assertSafeIdentifier — the savepoint functionality describe was deleted and not replaced. All three calls can be removed with a green suite. Savepoint names are string-interpolated into SQL and include peer-supplied input (peerIncarnation_${peerId}, receive_${remoteId}_${seq}). The nodejs driver kept its equivalent, so this is a one-sided regression on the browser driver.
  3. The wasm No such savepoint guard is uncovered in both rollbackSavepoint and releaseSavepoint; the test that looks like it covers this asserts SQLite's own lowercase message instead.
  4. Dropping verbose from the nodejs driver is untested — a one-line revert away from logging every statement with values inlined. See The browser kernel worker logs every kv set and delete with its value #1073.
  5. crank.cross-crank-gc.test.ts runs against makeMapKernelDatabase, whose savepoint methods are empty functions, so runCrank({ rollback: true }) performs no database rollback at all. The RAM assertions are valid, but the file's cross-crank durability claim is not tested.

Small, mechanical

  • crank.test.ts assigns context.savepoints = ['test'] — a string[] where the type is now Savepoint[].
  • beginOutOfCrank / endOutOfCrank are mocked in three test stores but are private to crank.ts and on no store surface. Inert, and misleading to the next reader.
  • vat-death-and-cranks.test.ts spies on VatHandle.make inside its fixture with no restore.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions