Skip to content

Commit 180e6ac

Browse files
sirtimidclaude
andauthored
test(extension): derive vat krefs in the e2e rather than hardcoding them (#1024)
Fixes a pre-existing e2e flake on `main`, surfaced while rebasing the #1020#1023 stack. Small and self-contained so the whole stack inherits it. ## The defect `control-panel.test.ts` › `should collect garbage` asserted that Carol's root object is `ko6` and Bob's is `ko5`, and that their promises are `kp4` and `kp3`: ```js '{"key":"ko6.owner","value":"v3"}', '{"key":"v3.c.ko6","value":"R o+0"}', ``` Since #983, subcluster vats launch **in parallel**. Each vat's root is exported when its own launch finishes, so which of `ko5`/`ko6` belongs to Bob and which to Carol changes between runs. When they come back the other way round, the test fails — and `database-inspector.test.ts` fails alongside it, because it reads the same kv dump. Observed directly: a failing run had `ko6.owner = v2` and `ko5.owner = v3`, the exact inverse of what is asserted. The vat ids themselves are stable — they are handed out in config order, so alice is always `v1` — so only the object and promise krefs need deriving. ## Approach Three small helpers read the dump and look up what the assertions used to hardcode: `rootKrefOf(dump, vatId)` by owner, `promiseKrefOf(dump, vatId)` by c-list entry, and `erefOf(dump, vatId, kref)`. The erefs are derived in **full** rather than matched by prefix. A c-list entry's reverse direction is keyed by eref and valued by kref, so a loose `,"value":"ko5"}` also matches the *owning* vat's own `v2.c.o+0` entry. That passed while both vats were alive and broke the negative assertions the moment one outlived the other — which is what the test checks after terminating v3. ## Testing `yarn lint` clean. Extension e2e run three times: the kref failure is gone, and the two clean runs finish in ~50s rather than ~2.7m because no retries are needed. **What this does not fix.** The extension e2e suite has separate instability that this change does not touch and does not claim to: `object-registry.test.ts` failures, and a UI timing flake where `Terminated vat "v1"` does not render because the panel is still showing query output. One of the three runs hit those. They are unrelated to kref assignment and were present before this change. ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, `README.md`, `CHANGELOG.md`) as appropriate — test-only change, no changelog entry <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Test-only change to e2e assertions and helpers; no production or runtime behavior is modified. > > **Overview** > Fixes flaky **`should collect garbage`** assertions in `control-panel.test.ts` that assumed fixed kernel refs (`ko5`/`ko6`, `kp3`/`kp4`) for Bob and Carol. Parallel subcluster launches mean those object and promise krefs can swap between runs while vat ids (`v2`/`v3`) stay stable. > > Adds helpers to parse the Database Inspector kv dump and **derive** root krefs (via `.owner`), promise krefs (via c-list), and v1’s **erefs** (full c-list lookup so reverse entries don’t false-match). The garbage-collection expectations are built from those values instead of literals. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit d8e81f7. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 494fb5e commit 180e6ac

1 file changed

Lines changed: 105 additions & 13 deletions

File tree

packages/extension/test/e2e/control-panel.test.ts

Lines changed: 105 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,88 @@ test.describe('Control Panel', () => {
4040
).toContainText('Subcluster launched');
4141
}
4242

43+
/**
44+
* Read the Database Inspector's kv dump.
45+
*
46+
* @returns Every key/value pair it is showing.
47+
*/
48+
async function readKvDump(): Promise<{ key: string; value: string }[]> {
49+
const text =
50+
(await popupPage
51+
.locator('[data-testid="message-output"]')
52+
.textContent()) ?? '';
53+
const rows =
54+
/\{"key":"(?<key>[^"]+)","value":"(?<value>(?:[^"\\]|\\.)*)"\}/gu;
55+
return [...text.matchAll(rows)].map((match) => ({
56+
key: match.groups?.key ?? '',
57+
value: match.groups?.value ?? '',
58+
}));
59+
}
60+
61+
/**
62+
* Find the kref of a vat's root object.
63+
*
64+
* Derived rather than hardcoded: subcluster vats are launched in parallel, so
65+
* each vat's root is exported whenever its own launch finishes, and which of
66+
* `ko5`/`ko6` belongs to Bob and which to Carol changes between runs. The vat
67+
* ids themselves are stable, being handed out in config order.
68+
*
69+
* @param dump - The kv pairs to search.
70+
* @param vatId - The vat whose root is wanted.
71+
* @returns Its root object's kref.
72+
*/
73+
function rootKrefOf(
74+
dump: { key: string; value: string }[],
75+
vatId: string,
76+
): string {
77+
const owned = dump.find(
78+
({ key, value }) => key.endsWith('.owner') && value === vatId,
79+
);
80+
expect(owned, `no object owned by ${vatId}`).toBeDefined();
81+
return (owned as { key: string }).key.replace('.owner', '');
82+
}
83+
84+
/**
85+
* Find the kref of the promise a vat is the subject of, by its c-list entry.
86+
*
87+
* Unstable for the same reason as the roots: `kp3` and `kp4` are allocated as
88+
* the bootstrap's calls to Bob and Carol are answered.
89+
*
90+
* @param dump - The kv pairs to search.
91+
* @param vatId - The vat whose promise is wanted.
92+
* @returns The promise's kref.
93+
*/
94+
function promiseKrefOf(
95+
dump: { key: string; value: string }[],
96+
vatId: string,
97+
): string {
98+
const entry = dump.find(({ key }) => key.startsWith(`${vatId}.c.kp`));
99+
expect(entry, `no promise in ${vatId}'s c-list`).toBeDefined();
100+
return (entry as { key: string }).key.slice(`${vatId}.c.`.length);
101+
}
102+
103+
/**
104+
* Find the eref a vat knows a kref by.
105+
*
106+
* Needed in full rather than by prefix: a c-list entry's reverse direction is
107+
* keyed by eref and valued by kref, so matching on the kref alone would also
108+
* hit the owning vat's own entry for the same object.
109+
*
110+
* @param dump - The kv pairs to search.
111+
* @param vatId - The vat whose c-list to read.
112+
* @param kref - The kref it holds.
113+
* @returns The eref, e.g. `o-1`.
114+
*/
115+
function erefOf(
116+
dump: { key: string; value: string }[],
117+
vatId: string,
118+
kref: string,
119+
): string {
120+
const entry = dump.find(({ key }) => key === `${vatId}.c.${kref}`);
121+
expect(entry, `${vatId} has no c-list entry for ${kref}`).toBeDefined();
122+
return (entry as { value: string }).value.replace(/^R /u, '');
123+
}
124+
43125
test('should load popup with kernel panel', async () => {
44126
await expect(
45127
popupPage.locator('button:text("Control Panel")'),
@@ -157,26 +239,36 @@ test.describe('Control Panel', () => {
157239
await expect(
158240
popupPage.locator('[data-testid="message-output"]'),
159241
).toContainText('{"key":"vats.terminated","value":"[]"}');
242+
const dump = await readKvDump();
243+
const v2Root = rootKrefOf(dump, 'v2');
244+
const v3Root = rootKrefOf(dump, 'v3');
245+
const v2Promise = promiseKrefOf(dump, 'v2');
246+
const v3Promise = promiseKrefOf(dump, 'v3');
160247
const v3Values = [
161248
'{"key":"e.nextPromiseId.v3","value":"2"}',
162249
'{"key":"e.nextObjectId.v3","value":"1"}',
163-
'{"key":"ko6.owner","value":"v3"}',
164-
'{"key":"v3.c.ko6","value":"R o+0"}',
165-
'{"key":"v3.c.o+0","value":"ko6"}',
166-
'{"key":"v3.c.kp4","value":"R p-1"}',
167-
'{"key":"v3.c.p-1","value":"kp4"}',
168-
'{"key":"ko6.refCount","value":"1,1"}',
169-
'{"key":"kp4.refCount","value":"2"}',
250+
`{"key":"${v3Root}.owner","value":"v3"}`,
251+
`{"key":"v3.c.${v3Root}","value":"R o+0"}`,
252+
`{"key":"v3.c.o+0","value":"${v3Root}"}`,
253+
`{"key":"v3.c.${v3Promise}","value":"R p-1"}`,
254+
`{"key":"v3.c.p-1","value":"${v3Promise}"}`,
255+
`{"key":"${v3Root}.refCount","value":"1,1"}`,
256+
`{"key":"${v3Promise}.refCount","value":"2"}`,
170257
];
258+
// Derived too: v1 imports the two roots as the bootstrap's calls are
259+
// answered, so which of `o-1`/`o-2` names which root varies with the same
260+
// launch ordering the krefs do.
261+
const v2Eref = erefOf(dump, 'v1', v2Root);
262+
const v3Eref = erefOf(dump, 'v1', v3Root);
171263
const v1koValues = [
172-
'{"key":"v1.c.ko5","value":"R o-1"}',
173-
'{"key":"v1.c.o-1","value":"ko5"}',
174-
'{"key":"v1.c.ko6","value":"R o-2"}',
175-
'{"key":"v1.c.o-2","value":"ko6"}',
264+
`{"key":"v1.c.${v2Root}","value":"R ${v2Eref}"}`,
265+
`{"key":"v1.c.${v2Eref}","value":"${v2Root}"}`,
266+
`{"key":"v1.c.${v3Root}","value":"R ${v3Eref}"}`,
267+
`{"key":"v1.c.${v3Eref}","value":"${v3Root}"}`,
176268
];
177269
await expect(
178270
popupPage.locator('[data-testid="message-output"]'),
179-
).toContainText('{"key":"kp3.refCount","value":"2"}');
271+
).toContainText(`{"key":"${v2Promise}.refCount","value":"2"}`);
180272
await expect(
181273
popupPage.locator('[data-testid="message-output"]'),
182274
).toContainText('{"key":"vatConfig.v3","value"');
@@ -241,7 +333,7 @@ test.describe('Control Panel', () => {
241333
// kp4 reference dropped to 1
242334
await expect(
243335
popupPage.locator('[data-testid="message-output"]'),
244-
).toContainText('{"key":"kp4.refCount","value":"1"}');
336+
).toContainText(`{"key":"${v3Promise}.refCount","value":"1"}`);
245337
await popupPage.click('button:text("Control Panel")');
246338
await popupPage.locator('[data-testid="accordion-header"]').first().click();
247339
// delete v1

0 commit comments

Comments
 (0)