Skip to content

Commit d7102da

Browse files
rekmarksclaude
andcommitted
test(ocap-kernel): restore branch coverage to baseline after netlayer extraction
Extracting the well-covered netlayer machinery into @metamask/netlayer lowered ocap-kernel's aggregate branch coverage (an arithmetic artifact). Add behavior-asserting tests for specified guard/error paths to bring it back above the pre-extraction baseline (85.91%): - reconnectPeer handler: defaults omitted hints to [] - RemoteHandle: rejects invalid inbound message seq (non-number/non-integer/<1); rejects outbound sends once the pending queue is at capacity; gives up and fires onGiveUp after MAX_RETRIES unacknowledged retransmits; rejects pending + gives up on a terminal outbound send error; cleans up the pending redemption when the redeem send is rejected - GC store: collectGarbage deletes an unreferenced promise ocap-kernel branch coverage: 85.9% -> 86.3% (>= baseline on two consecutive runs); statements/functions/lines also above baseline. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent a775362 commit d7102da

3 files changed

Lines changed: 140 additions & 0 deletions

File tree

packages/ocap-kernel/src/remotes/kernel/RemoteHandle.test.ts

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -584,6 +584,79 @@ describe('RemoteHandle', () => {
584584
);
585585
});
586586

587+
it.each([
588+
{ label: 'a non-number seq', seq: 'x' },
589+
{ label: 'a non-integer seq', seq: 1.5 },
590+
{ label: 'a zero seq', seq: 0 },
591+
{ label: 'a negative seq', seq: -3 },
592+
])('handleRemoteMessage rejects $label', async ({ seq }) => {
593+
const remote = makeRemote();
594+
const delivery = JSON.stringify({
595+
seq,
596+
method: 'deliver',
597+
params: ['dropExports', ['ro+1']],
598+
});
599+
await expect(remote.handleRemoteMessage(delivery)).rejects.toThrow(
600+
`invalid message seq: ${seq}`,
601+
);
602+
remote.cleanup();
603+
});
604+
605+
it('rejects a new outbound message once the pending queue is at capacity', async () => {
606+
const remote = makeRemote();
607+
// Fill the pending queue to MAX_PENDING_MESSAGES (200). The mock transport
608+
// resolves each send but never ACKs, so the queue never drains.
609+
for (let index = 0; index < 200; index += 1) {
610+
await remote.deliverDropExports(['ro+1']);
611+
}
612+
await expect(remote.deliverDropExports(['ro+1'])).rejects.toThrow(
613+
'pending queue at capacity (200)',
614+
);
615+
remote.cleanup();
616+
});
617+
618+
it('rejects pending and fires give-up when an outbound delivery hits a terminal send error', async () => {
619+
const onGiveUp = vi.fn();
620+
const remote = RemoteHandle.make({
621+
remoteId: mockRemoteId,
622+
peerId: mockRemotePeerId,
623+
kernelStore: mockKernelStore,
624+
kernelQueue: mockKernelQueue,
625+
remoteComms: mockRemoteComms,
626+
onGiveUp,
627+
});
628+
// isTerminalSendError matches by `error.name`; the real class is
629+
// transport-internal.
630+
const terminal = Object.assign(new Error('Remote peer restarted'), {
631+
name: 'PeerRestartedError',
632+
});
633+
vi.mocked(mockRemoteComms.sendRemoteMessage).mockRejectedValueOnce(
634+
terminal,
635+
);
636+
637+
await remote.deliverNotify([['rp+1', false, { body: '"a"', slots: [] }]]);
638+
// The send is fire-and-forget; drain microtasks so its catch handler runs.
639+
for (let i = 0; i < 5; i += 1) {
640+
await Promise.resolve();
641+
}
642+
643+
expect(onGiveUp).toHaveBeenCalledWith(mockRemotePeerId);
644+
remote.cleanup();
645+
});
646+
647+
it('cleans up the pending redemption when the redeem send is rejected', async () => {
648+
const remote = makeRemote();
649+
// Fill the outbound queue so the redeemURL send itself is rejected at
650+
// capacity, exercising redeemOcapURL's catch-path cleanup.
651+
for (let index = 0; index < 200; index += 1) {
652+
await remote.deliverDropExports(['ro+1']);
653+
}
654+
await expect(remote.redeemOcapURL('ocap:test@peer')).rejects.toThrow(
655+
'pending queue at capacity (200)',
656+
);
657+
remote.cleanup();
658+
});
659+
587660
it('handleRemoteMessage handles redeemURL request', async () => {
588661
const remote = makeRemote();
589662
const mockOcapURL = 'as if it was a URL';
@@ -1673,6 +1746,40 @@ describe('RemoteHandle', () => {
16731746
expect(mockRemoteComms.sendRemoteMessage).toHaveBeenCalledTimes(2);
16741747
expect(onGiveUp).not.toHaveBeenCalled();
16751748
});
1749+
1750+
it('gives up and fires onGiveUp after MAX_RETRIES unacknowledged retransmits', async () => {
1751+
const onGiveUp = vi.fn();
1752+
const remote = RemoteHandle.make({
1753+
remoteId: mockRemoteId,
1754+
peerId: mockRemotePeerId,
1755+
kernelStore: mockKernelStore,
1756+
kernelQueue: mockKernelQueue,
1757+
remoteComms: mockRemoteComms,
1758+
ackTimeoutMs: 100,
1759+
onGiveUp,
1760+
});
1761+
1762+
// One pending message that is never ACKed; sends resolve so each timeout
1763+
// retransmits rather than aborting.
1764+
vi.mocked(mockRemoteComms.sendRemoteMessage).mockResolvedValue(undefined);
1765+
await remote.deliverNotify([['rp+1', false, { body: '"a"', slots: [] }]]);
1766+
1767+
// MAX_RETRIES is 3: the first three timeouts retransmit; the fourth
1768+
// exhausts retries and gives up.
1769+
for (let attempt = 0; attempt < 4; attempt += 1) {
1770+
fireLastAckTimer();
1771+
for (let i = 0; i < 5; i += 1) {
1772+
await Promise.resolve();
1773+
}
1774+
}
1775+
1776+
expect(onGiveUp).toHaveBeenCalledWith(mockRemotePeerId);
1777+
expect(onGiveUp).toHaveBeenCalledTimes(1);
1778+
// Initial send + one per retry (MAX_RETRIES = 3) = 4; the give-up
1779+
// timeout does not retransmit again.
1780+
expect(mockRemoteComms.sendRemoteMessage).toHaveBeenCalledTimes(4);
1781+
remote.cleanup();
1782+
});
16761783
});
16771784

16781785
describe('first-send terminal errors', () => {

packages/ocap-kernel/src/rpc/platform-services/reconnectPeer.test.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,26 @@ describe('reconnectPeer', () => {
177177
expect(result).toBeNull();
178178
});
179179

180+
it('defaults hints to an empty array when the params omit them', async () => {
181+
const mockReconnectPeer: ReconnectPeer = vi.fn(async () => null);
182+
183+
const hooks = {
184+
reconnectPeer: mockReconnectPeer,
185+
};
186+
187+
// The struct requires `hints`, but the handler defensively defaults a
188+
// missing value to `[]`; exercise that guard directly.
189+
const params = { peerId: 'peer-789' } as unknown as {
190+
peerId: string;
191+
hints: string[];
192+
};
193+
194+
const result = await reconnectPeerHandler.implementation(hooks, params);
195+
196+
expect(mockReconnectPeer).toHaveBeenCalledWith('peer-789', []);
197+
expect(result).toBeNull();
198+
});
199+
180200
it('calls the reconnectPeer hook with empty hints array', async () => {
181201
const mockReconnectPeer: ReconnectPeer = vi.fn(async () => null);
182202

packages/ocap-kernel/src/store/methods/gc.test.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,5 +267,18 @@ describe('GC methods', () => {
267267
// Object should still exist
268268
expect(kernelStore.getObjectRefCount(ko1)).toBeDefined();
269269
});
270+
271+
it('deletes an unreferenced promise', () => {
272+
const [kpid] = kernelStore.initKernelPromise();
273+
// initKernelPromise sets refCount to 1; dropping it to zero enqueues the
274+
// promise for collection.
275+
kernelStore.decrementRefCount(kpid, 'gc-test');
276+
277+
kernelStore.collectGarbage();
278+
279+
expect(() => kernelStore.getKernelPromise(kpid)).toThrow(
280+
`unknown kernel promise ${kpid}`,
281+
);
282+
});
270283
});
271284
});

0 commit comments

Comments
 (0)