test(bindx): fix the chronic author-select flake, capture failure screenshots, cover collectFieldsData

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

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

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' })