From 38bf14358005458f2f317f24ec8dea0a17a84147 Mon Sep 17 00:00:00 2001 From: David Matejka Date: Tue, 25 Aug 2026 17:41:56 +0200 Subject: [PATCH 1/2] test(bindx): cover collectFieldsData as a relation-cycle entry point MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `collectFieldsData` is the other public way into relation collection, and it reaches `collectHasOneOperation` the same way `collectUpdateData` does — so it walks a hasOne cycle and, without the in-progress guard, overflows the stack. It had no coverage because nothing in the persist pipeline routes through it: a fields-scoped persist uses BatchPersister's own scalar-only variant instead. Refs #88 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01EmqgPtZvAfpvWKymfFZCEX --- .../persistence/relationCycleCollection.test.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/unit/persistence/relationCycleCollection.test.ts b/tests/unit/persistence/relationCycleCollection.test.ts index bf85eea..ca6e3e2 100644 --- a/tests/unit/persistence/relationCycleCollection.test.ts +++ b/tests/unit/persistence/relationCycleCollection.test.ts @@ -119,6 +119,22 @@ describe('relation cycles during collection', () => { expect(pageCall?.changes).toEqual({ title: 'Renamed page' }) }) + /** + * `collectFieldsData` is the other public entry into relation collection. Nothing in + * the persist pipeline routes through it today — `BatchPersister` has its own + * scalar-only variant for a fields scope — so it needs its own coverage. + */ + test('collectFieldsData walks into the cycle and terminates', () => { + const collector = new MutationCollector(store, new ContemberSchemaMutationAdapter(schema)) + store.setFieldValue('Revision', 'rev-published', ['name'], 'Renamed published') + + const data = collector.collectFieldsData('Revision', 'rev-draft', ['page']) + + expect(data).toEqual({ + page: { update: { publishedRevision: { update: { name: 'Renamed published' } } } }, + }) + }) + test('a create whose nested target points back at it does not recurse forever', async () => { const pageId = store.createEntity('Page', { title: 'New page' }) const revisionId = store.createEntity('Revision', { name: 'New revision' }) From c333b0f430510250906a586cd321eca9c39469cf Mon Sep 17 00:00:00 2001 From: David Matejka Date: Tue, 25 Aug 2026 17:45:09 +0200 Subject: [PATCH 2/2] test(bindx): make the author-select popover click self-healing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Article with Author Select > changing author enables save` failed on roughly half of all CI attempts since 2026-08-20, on branches with nothing to do with it. The popover was opened once, outside the retry loop. A click that lands while the option list is remounting is lost *and* dismisses the popover, so every remaining `clickUntil` attempt found no option to click: the whole budget burned without a single real try, and the test spent 15s waiting for an outcome nothing could produce. Open the popover inside the retry callback instead, and only when the option is not already there — so a dismissed popover is reopened rather than toggled shut. Also make `waitFor` screenshot the page when it times out. CI has always uploaded `/tmp/browser-test-*.png` on failure, but nothing ever wrote one, so the artifact was empty on every red run. `clickUntil` opts out for its intermediate polls, which are expected to fail and retry. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01EmqgPtZvAfpvWKymfFZCEX --- tests/browser/authorSelect.test.ts | 14 ++++++++++---- tests/browser/browser.ts | 21 ++++++++++++++++++--- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/tests/browser/authorSelect.test.ts b/tests/browser/authorSelect.test.ts index a9cf84d..2376355 100644 --- a/tests/browser/authorSelect.test.ts +++ b/tests/browser/authorSelect.test.ts @@ -16,13 +16,19 @@ browserTest('Article with Author Select', () => { }) test('changing author enables save and updates display', () => { - // Open the author SelectField popover - el(`${tid('article-with-author-select')} [aria-haspopup="dialog"]`).click() - // Select from the stable initial list; filtering remounts options asynchronously. + const trigger = () => el(`${tid('article-with-author-select')} [aria-haspopup="dialog"]`) + // Pick from the stable initial list; filtering remounts options asynchronously. const janeOption = () => el('[role="dialog"] button[data-entity-id="00000000-0000-0000-0000-000000000a02"]') - waitFor(() => janeOption().exists) + clickUntil( () => { + // Re-open on every attempt. A click that lands mid-remount is lost *and* + // dismisses the popover, so without this the remaining attempts have no + // option to click and the whole budget burns without a single real try. + if (!janeOption().exists) { + trigger().click() + waitFor(() => janeOption().exists, { capture: false }) + } const option = janeOption() expect(option.text).toContain('Jane') return option diff --git a/tests/browser/browser.ts b/tests/browser/browser.ts index e375403..7289d1b 100644 --- a/tests/browser/browser.ts +++ b/tests/browser/browser.ts @@ -40,7 +40,13 @@ function resolveSelector(selectorOrTestId: string): string { */ export function waitFor( condition: () => boolean, - { timeout = POLL_TIMEOUT, interval = POLL_INTERVAL, message }: { timeout?: number; interval?: number; message?: string } = {}, + { timeout = POLL_TIMEOUT, interval = POLL_INTERVAL, message, capture = true }: { + timeout?: number + interval?: number + message?: string + /** Screenshot the page on timeout. Off for polls a caller expects to fail and retry. */ + capture?: boolean + } = {}, ): void { const start = Date.now() while (Date.now() - start < timeout) { @@ -55,7 +61,16 @@ export function waitFor( if (!condition()) { const elapsed = Date.now() - start const hint = message ?? condition.toString().slice(0, 120) - throw new Error(`waitFor timed out after ${elapsed}ms: ${hint}`) + // CI uploads /tmp/browser-test-*.png on failure; without this nothing ever writes one. + let shot = '' + if (capture) { + try { + shot = ` (screenshot: ${screenshot()})` + } catch { + // a broken session must not mask the real timeout + } + } + throw new Error(`waitFor timed out after ${elapsed}ms: ${hint}${shot}`) } } @@ -133,7 +148,7 @@ export function clickUntil( // target gone — the previous click may have registered and closed the popover } try { - waitFor(condition, { timeout: settle }) + waitFor(condition, { timeout: settle, capture: false }) return } catch { // outcome didn't materialize — re-click