VideoTimelineEditor keyboard test flakes in full-suite runs: sync getByRole races the clip lane's commit

[atomantic]

What fails

VideoTimelineEditor.keyboard.test.jsx → "still selects the block on Enter, which the drag must not swallow" fails only in a full-suite run (cd client && npx vitest run, 960 files), with:

TestingLibraryElementError: Unable to find an accessible element with the role "button"
and name `Remove second.png from timeline`
 ❯ blockFor src/pages/VideoTimelineEditor.keyboard.test.jsx:77:4
 ❯ src/pages/VideoTimelineEditor.keyboard.test.jsx:117:20

Observed once in a local full-suite run on 2026-09-13 (959 passed / 1 failed). It does not reproduce:

• running the file alone (3/3 pass)
• running src/pages (1426 tests) three times in a row

So it is load/timing sensitive, not a deterministic failure.

Likely cause

renderEditor() waits only for the "Loading project…" placeholder to disappear:

const renderEditor = async () => {
  render(<VideoTimelineEditor />);
  await waitFor(() => expect(screen.queryByText('Loading project…')).not.toBeInTheDocument());
};

Every test then reaches for a clip block with a synchronous query:

const blockFor = (assetFile) => screen
  .getByRole('button', { name: `Remove ${assetFile} from timeline` })
  .closest('[aria-roledescription="sortable"]');

The suite's getTimelineProject, getGalleryImages, listVideoHistory, listMusicLibrary and listImageGalleryPage mocks are all separate promises. "Loading project…" clearing is not proof that the segment lane has committed its blocks, so under load the sync getByRole can run a tick early. The first two tests happen to await pressKey(...) right after, which lets a straggling microtask land; the Enter test asserts sooner, which is consistent with it being the one that failed.

Suggested fix

Make the block lookup await its element rather than assume it is already committed — either:

• const blockFor = async (assetFile) => (await screen.findByRole('button', { name: ... })).closest('[aria-roledescription="sortable"]'), awaiting it at the three call sites; or
• extend renderEditor() to await screen.findByRole('button', { name: /Remove .* from timeline/ }) so every test starts from a committed lane.

Prefer whichever keeps the three tests reading the same way. Do not paper over it with an arbitrary sleep.

Verify

Run the whole client suite (cd client && npx vitest run) a few times — the file must pass in the full 960-file run, not just standalone. The three assertions the file guards (#7243: Space/arrow/Space reorder, Escape cancel, Enter still selects) must all still hold.

[LouisDeconinck]

I'd like to claim this issue — implementing option (b): extend renderEditor() to await a committed lane block via findByRole('button', { name: /Remove .* from timeline/ }), so the three tests keep reading the same way.

One wrinkle I found while looking at it: the existing wait is not just early — it is vacuous. queryByText('Loading project…') has matched nothing since #5659 moved the page to PageSkeleton (the label is an aria-label on role="status", not a text node), so waitFor resolves on its first poll and the sync getByRole in blockFor races the whole refresh() fetch chain — two sequential awaits before setLanes commits. That is why it only loses under full-suite load.

VideoTimelineEditor.test.jsx carries the identical dead wait (and its tests make the same sync queries right after), so I plan to fix that copy too — with a skeleton-unmount wait rather than a lane button, since its default project renders empty lanes with no Remove buttons to find.

[atomantic]

Thanks for the detailed analysis and proposed fix, Louis. Agree that extending renderEditor() to await a committed lane block is the right approach to ensure tests are stable under full-suite load. Please also make sure to apply the same fix to VideoTimelineEditor.test.jsx as you identified the same issue there.