test(device): local emulator layer — real Chrome/Gboard as normal CI - #3034
Closed
YousefED wants to merge 35 commits into
Closed
test(device): local emulator layer — real Chrome/Gboard as normal CI#3034YousefED wants to merge 35 commits into
YousefED wants to merge 35 commits into
Conversation
Puts the harnesses in place that the mobile fixes are verified with, so those changes arrive with their tests rather than their scaffolding. - A fourth browser instance running Android-emulated Chromium, for `end-to-end/mobile/`. Per-instance `contextOptions` are silently ignored by the runner, so the emulation is applied through the provider; `ensureTouchEmulation` asserts it actually took effect rather than letting a stubbed-out context pass as coverage. - `imeComposition`, a browser command driving Chromium's real IME pipeline over CDP. Synthetic `CompositionEvent`s are untrusted and never mutate the DOM, so they cannot reproduce what a mobile keyboard does; `Input.imeSetComposition` can. - A BrowserStack real-device suite (`tests/device/`) and its workflow. The gesture layer is where per-platform quirks are recorded. The copypaste and keyboardhandlers suites gain skips for the cases that don't translate to a touch-emulated context — positional mouse drags have no touch equivalent, so those tests would fail for reasons unrelated to what they cover.
Running the device suite meant exporting BROWSERSTACK_* by hand each time. The config now loads the repo root's `.env` (gitignored; the entries are documented in `.env.sample`, the repo's one sample file) — dotenv parsing accepts its shell-style `export KEY=value` lines, so the same file keeps working for `source`. Real environment variables take precedence, so CI is unaffected, and the missing-credentials error points at the file.
zizmor (repo policy) requires actions pinned to hashes; the workflow also diverged from how every other workflow sets up — checkout + setup-vp with the shared pins, persist-credentials off, an explicit least-privilege permissions block, and vp for install/run. The test:device script drops its pnpm invocation for the same reason: CI only provides vp.
Review feedback: this should be part of normal CI, not a scheduled job. Runs on pushes to main and on PRs — fork PRs have no secrets, so the suite self-skips and the job is a green no-op there. Device minutes are metered, so a superseding push cancels the in-flight PR run. Also silences the actionlint unused-loop-variable warning.
Review feedback asked to simplify the tunnel setup, and the simplest form turned out to be deleting the host-rewriting proxy entirely: it existed only to satisfy Vite's allowedHosts check, and the playground config already whitelists a hostname for the docker e2e setup — bs-local.com joins it, so devices browse the dev server directly through the BrowserStackLocal tunnel. Also removes the CodeQL-flagged error echo in the proxy's 502 path, by removing the proxy. The binary download now maps platform/arch explicitly — the old fallback handed Windows and Linux-ARM the linux-x64 archive — and fails with guidance on unsupported hosts. The header documents that this file runs identically locally and in CI (a parity choice over BrowserStack's GitHub Action, which wraps the same daemon).
Review feedback: the emulation is configured per instance already (the playwright provider's contextOptions) — this util existed to self-heal the one known way it gets lost, Playwright dropping the context's touch emulation after an iframe-element screenshot. Stubbing detection back made tests pass in a context where actual touch input behaves like a desktop. Now it fails loudly, naming the cause, if the loss ever happens — which the android instance's include list is supposed to prevent.
Two review findings about stacking hygiene: - The link-popover device helpers (selectFirstWord, openLinkPopover, the LINK_* selectors, typeAndSubmit) lived in the shared lib but serve only the link tests — they move up to the layer that adds those tests, next to them. - The copypaste/keyboardhandlers touch-emulation skips were carried here while the android instance only runs mobile/**; they belong in the layer that widens the instance to those suites.
Flagged by review: the sample carried a real-looking Nx remote-cache access token (committed with the nx 21 upgrade in July 2025). A sample file should hold placeholders; the value has been public in git history the whole time, so if it is a live credential it needs rotating — see the PR discussion.
…package Review pushback was right to be surprised by the hand-rolled download script: BrowserStack's documented Node.js integration is their browserstack-local package, which downloads and manages the right daemon for the host platform itself. The custom binary fetch, platform/arch map, and daemon spawning all go away; the same code path runs locally and in CI.
Review asked where the docs bless the client half of this rig — and for the hand-rolled REST plumbing (or a bare protocol package) they don't. The client BrowserStack's Automate Node.js documentation and samples actually use is selenium-webdriver, with auth inside the capabilities' bstack:options — which devices.ts already had. DeviceSession now wraps that client and keeps only the domain layer: lifecycle with retry, script polling, artifact screenshots, and the dashboard annotation (a BrowserStack REST API, not a WebDriver route). Public API unchanged, so gestures and tests don't move. (The browserstack-node-sdk layered on top of selenium-webdriver wraps supported runners — Jest, Mocha — and manages the tunnel and platform matrix from a yml. Adopting it would mean a second test runner in a vitest-standardized repo, for tunnel management we already get from the official browserstack-local binding and a device matrix devices.ts already expresses. Deliberately not taken; revisit if Test Observability becomes interesting.)
Review feedback: the reasoning about which BrowserStack packages to use (and why browserstack-node-sdk deliberately isn't) lived in a commit message — durable documentation belongs in the README. Also removes a doc comment orphaned by typeAndSubmit's move.
build.yml runs on the edited event; the device workflow now does too, but only when the edit changed the base branch — that's what changes the merge result (routine in a PR stack), while title and body edits would just spend device minutes.
Creating a link on Android didn't work: the popover's URL never became a link and focus jumped to the next editor instead. The cause is that a mobile IME picks the action its Enter key performs, and with a lone text field it picks "Next" — advancing focus and dispatching no key event at all. A popover that only listens for Enter therefore has nothing to hear. Putting the fields in a real `<form>` is what makes the IME offer a submitting action instead, confirmed on a device; `Form.Root` was a `<div>`, so `onSubmit` could never fire. `Form.Root` now renders a `<form>`, and submission runs off its `submit` event. That has three consequences worth calling out: - HTML only submits implicitly when a form has a submit button or exactly one field, so the link *edit* form — url plus title — would still reach nothing. `Form.Root` renders a submit button to cover any field count. It is visually hidden rather than absent so assistive technology still has a labelled control, and outside the tab order so sighted keyboard users never land on a control they can't see. - The browser performs implicit submission for an Enter that arrives with `isComposing: true`, so accepting an IME candidate would submit the popover mid-word. `useFormSubmit` guards that centrally, replacing the per-callsite `isComposing` checks that had already drifted apart. - With one submission path, the five Enter handlers are redundant and are removed. `EmbedTab` had no form at all and gains one; the AI prompt menu's handler and `onSubmit` disagreed about whether Enter picks the highlighted suggestion or submits the typed text, and now share one decision. `TextInput` also loses its `onSubmit` prop: every skin forwarded it to the `<input>`, and `submit` only fires on a form and bubbles upward, so it could never have fired. `EditLinkMenuItems` passed it, which is plausibly why the gap went unnoticed.
Review follow-ups: - The embed panel ended up with two submit controls: its own Embed button plus the hidden one `Form.Root` adds, so a screen reader announced two separate actions for the one thing that panel does. `Form.Root` now takes `hasOwnSubmitButton` for callers that supply their own. - The three `TextInput`s hand-rolled ref merging. `mergeRefs` already exists here, but returns a fresh callback per call — which detaches and reattaches the ref every render — so this adds `useMergeRefs` alongside it, memoized the way `react-merge-refs` does, and uses that. - The mantine popover keyed two behaviours off `portalRoot` while its comments explained them in terms of mobile. Same condition, but named, so the reason isn't hidden behind an unrelated prop. - `useFormSubmit` documents that it exists for `Form.Root` implementations rather than applications.
…'t fail Second review round, checking whether the tests added in the first one can actually fail. Two could not: - The composition tests built a synthetic form replicating what `Form.Root` does, so deleting the guard from `useFormSubmit` left them all green — the shipped code had no coverage at all. A test now drives the real link popover through a CDP composition, and fails when the guard is removed. The synthetic ones stay as what they are: the platform fact that a browser submits for an Enter carrying `isComposing: true`. - "the embed tab commits exactly once" asserted one image was present, which is true whether the update ran once or twice. Its replacement counted the form's submit events, but that cannot fail either: only mantine runs in this suite and its panel button already defaults to `type="button"`. The structural check — no button inside the form — is what actually guards both the double-commit and the duplicate-control problems, and it does fail when the button is moved inside, so that one is kept and the outcome-based tests are dropped rather than left as decoration. Also renames `hasOwnSubmitButton` to `omitSubmitButton`: EmbedTab's button sits outside the form, so the form has no submit button at all and relies on single-field implicit submission. The old name asserted something untrue of its only caller, and hid the constraint the flag carries.
…oundary The `from + 1` probe fixed the left-edge case (`marks()` excludes a link at its left boundary) but is still fragile: browsers disagree by a position on where a selection over a link starts, so a single-position lookup can land outside the mark either way. For a non-empty selection, scan the selected range for the first link mark instead; an empty selection keeps the plain position lookup.
…es IMEs The guard answered the wrong category of problem. `isComposing` checks are needed in *keydown* handlers, because an IME-consumed key still dispatches to JS — that is what the five removed Enter handlers were. Native form submission never sees that key: the IME consumes the confirming Enter (it reaches the page as keyCode 229, which the browser runs no default action for), so implicit submission cannot fire mid-composition. This is why no plain form on the web carries composition handling. The state the guard defended — composition open, unconsumed trusted Enter delivered — is one only CDP emulation can fabricate: `imeSetComposition` sets composition state with no IME in the loop to consume the key. No real IME produces the sequence. Worse, the guard carried real risk in the other direction: Gboard's action key commits the composition and submits in one press, so if any IME delivers `submit` before `compositionend`, the guard would swallow a legitimate submission — the original bug, reintroduced for exactly the users it claimed to protect. `Form.Root` goes back to plain `preventDefault` wiring, `useFormSubmit` is deleted, and the composition tests now pin the *native* contract against the real popover: accepting a candidate does not submit, Enter afterwards does.
… form Review feedback (two threads): the link-flow device helpers belong next to the tests that use them, not in the shared lib — moved here from editorPage/gestures. typeAndSubmit also changes how it submits, answering why it dispatched a synthetic Enter: the on-screen keyboard's action key is unreachable by any automation channel (see README), and the dispatched keydown only worked while the popovers had key handlers. With submission running off the form's submit event, an untrusted keydown does nothing — the helper was silently broken by the form rework. requestSubmit() is the browser's own submission path and exercises the popover's real onSubmit wiring; the IME's own action-key choice stays a manual release check.
Two review questions shaped this. First: the old dispatched KeyboardEvent could never submit once the popovers moved to the form's submit event — synthetic events trigger no default action. Second: 'why not hit the Enter key?' — no reason not to, and the rig already knew how: on iOS, pressSoftKeyboardEnter taps the on-screen keyboard's actual return key (the RETURN_KEY_RATIOS offset ladder) — the real user gesture. On Android, where BrowserStack blocks native taps, a W3C protocol Enter is used instead: trusted input, so the browser runs its default action and the real path is exercised (key press -> implicit form submission -> the popover's submit handling). Only Gboard's own choice of *which* action its key performs stays out of reach, on the manual release checklist. Callers supply the verify script the iOS tap ladder needs.
The 'no input channel can press that key' claim was BrowserStack-scoped truth stated as absolute. A local Android emulator runs real Chrome and real Gboard, and adb can tap the on-screen action key — verified end to end (action key tapped, link created in the correct editor, focus retained). Recorded as the known path to automating the release-checklist item.
On Android, prosemirror-view deliberately bails out of its keydown handling: the IME reports composing keys as keyCode 229, so the key identity can't be trusted. Enter therefore never reached the keymap and pressing it did nothing — no new block, no list continuation. `beforeinput` carries the intent unambiguously (`insertParagraph` / `insertLineBreak`) regardless of what the IME reports, so the shortcuts extension intercepts it there and runs the same keymap command. Only on Android, and only when not composing, so every other platform keeps the existing path. This also unblocks running the core behavioural suites under Android emulation. They were held out of the android instance in the test-infra change precisely because of this bug — every test that presses Enter to make a second block failed there — so the instance's include list grows here, where it can be green.
The beforeinput interception only covers the IME path. With a hardware or synthetic keyboard, Enter arrives as a keypress instead — and prosemirror-view's own keypress handler cancels the browser default for cross-block selections without doing anything in their place (its cross-parent branch skips newline characters), so Enter over a selection spanning two blocks was a silent no-op. Intercepting keypress too closes that hole, and the two paths now share one `dispatchSynthesizedEnter` helper rather than repeating the flush-then- synthesize sequence. The `domObserver` reach-through is typed against `EditorView` instead of `typeof view`. Test coverage goes from one path to three — keypress, beforeinput, and the cross-block selection — and `Check Enter when selection is not empty` no longer has to be skipped on the android instance, which is the suite-level proof that the keypress hole is closed. Also makes `Check Delete before shallower block` deterministic: it relied on ArrowUp's goal-x landing on a particular side of a character boundary, which varies with subpixel metrics and had been flaking across engines.
The popover form-submission tests exist because of Android bugs, yet only ran on the desktop engines. The android instance is chromium, so even the CDP composition tests run there. All 14 pass under the emulation.
…ening These skips guard tests that drive selection/resizing with positional mouse drags, which have no touch-emulation equivalent. They used to ship with the test infrastructure; review pointed out they belong here, where the android instance actually starts running the copypaste suite.
…terface The device suite's session becomes an interface with three backends, so tests and page helpers are written once and run against whatever a machine can drive (activeDevices probes availability): - browserstack.ts: the existing selenium-webdriver client, unchanged behaviour. - localAndroid.ts: a local Android emulator via Playwright's first-party (experimental) Android support — real Chrome as a Playwright page over CDP, input as genuine OS events via adb shell (Playwright's device.input needs a companion APK; shell does not). Page-to-screen coordinates come from a one-time calibration tap on an app page — never Chrome's initial page, whose 980px virtual viewport poisons the mapping. The backend can press the on-screen keyboard itself, which no cloud channel can: pressImeActionKey drives Gboard's action key with a verify ladder. - localIos.ts: Apple's safaridriver against a booted simulator (real iOS Safari, no tunnel — the simulator shares the host network). Known limitation, found empirically: safaridriver's synthetic input never summons the software keyboard, and mixing HID injection with an automation session trips Safari's "stop the current automated test session?" guardrail — so keyboard-gated flows don't run here yet. The sanctioned full-fidelity route is Appium's XCUITest driver; follow-up. imeAction.device.test.ts is the previously-manual release-checklist item as a test: Gboard's real action key submits the link popover and focus stays in the editor. All three suites green on the emulator (6 tests) with Chrome 124 on an API-35 image.
Real Chrome + real Gboard on a KVM-backed emulator (API 35, x86_64) on every PR and push to main — free minutes, no credentials. Runs the same tests/device suite as local development (DEVICE_FILTER=local-android), including the previously-manual IME action-key check. Actions pinned per repo policy.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
'vp -C tests exec vitest' resolved through vite-plus's transitive bin locally but not on a fresh CI install; a direct devDependency links the bin deterministically (pinned to the workspace's vitest override).
@blocknote/ariakit
@blocknote/code-block
@blocknote/core
@blocknote/diagram-block
@blocknote/mantine
@blocknote/math-block
@blocknote/react
@blocknote/server-util
@blocknote/shadcn
@blocknote/xl-ai
@blocknote/xl-docx-exporter
@blocknote/xl-email-exporter
@blocknote/xl-multi-column
@blocknote/xl-odt-exporter
@blocknote/xl-pdf-exporter
commit: |
|
…targets safaridriver turned out structurally unable to run keyboard-gated flows: its input is synthetic at the WebKit layer (never summons the software keyboard), and injecting real HID during its session trips Safari's "stop the current automated test session?" guardrail. The sanctioned stack is Appium's XCUITest driver — WebDriverAgent owns the HID, so the keyboard appears even on a headless simulator, and `mobile: tap` is the same channel the BrowserStack iOS backend uses, so the gesture layer's chrome-offset ladders apply unchanged (iOS now routes through them for every kind; Appium's web-context clicks are synthetic there too, nativeWebTap included). Setup boots (or reuses) a simulator and spawns Appium — an npm devDependency whose xcuitest driver Appium discovers; postinstalls stay off via allowBuilds, verified working. Appium needs an even-numbered Node (.node-version's 24 qualifies; the check message points there). CI gains a macos-15 job running the same suite. Local matrix: 11 tests green across the API-35 emulator (Chrome 124 + Gboard) and the iPhone simulator (real iOS Safari).
The README now leads with the target matrix (local-android, local-ios, browserstack), what each uniquely reaches, and how to run any subset locally; the iOS automation facts gained the safaridriver-guardrail and synthetic-click findings so nobody re-walks that path; the former manual IME checklist item is marked as automated by imeAction.device.test.ts.
The setup's env-var handoff never reached vitest's workers in CI (global setup and workers don't share an environment), and the discovery regex missed the UDID's closing paren, so sessions saw no booted simulator. Sessions now discover the booted device themselves with a short retry; the setup boots via 'bootstatus -b' — boots if needed, returns promptly when already up — with no simulator state strings to pattern-match.
The layering policy from the testing-foundation discussion, as decision rules: new mobile tests default to end-to-end/mobile/; the device suite is only for behavior that emulation fakes; paired tests where possible; flows-through-real-input, never editor-logic details.
The Booted-state polling raced on slow CI runners (one green run, one 'no booted simulator' on a docs-only push). The setup now writes the chosen UDID to .artifacts/.booted-simulator and sessions read it — deterministic, no cross-process environment, no state polling.
The #3001 test now goes through the keyboard itself where the backend can reach it (the same bottom-right-key ladder as the action key), closing the last emulated-only route — and proves the delivery: the page must see an IME-mediated keydown 229, not the bare keydown 13 a synthesized key event produces. Doing so surfaced that the IME's Enter has two genuine variants: phone Gboard emits `beforeinput: insertParagraph` (the route the beforeinput interception handles), the emulator's AOSP LatinIME emits 229 followed by a real keydown (the route the keypress interception handles). The test documents both and asserts the invariant, so the suite now validates the Android Enter fix against a second IME family.
The emulator/simulator targets reach everything the cloud backend did and more (they can press the on-screen keyboard; no cloud channel can), run as free per-PR CI, and need no credentials or tunnel. What only real hardware has left is OEM keyboards, which stays a manual release-checklist item. The backend stays revivable behind the unchanged session interface (PR #3034).
YousefED
force-pushed
the
mobile/android-enter
branch
from
September 1, 2026 06:01
b1ff1c7 to
789c9be
Compare
Collaborator
Author
|
Folded into the stack: the emulator/simulator suite now is the test-infra layer (#3029, with the device tests landing in #3030/#3031 alongside the fixes they verify). The final commits here also removed the BrowserStack backend entirely — the local targets reach everything it did and more (they can press the on-screen keyboard; no cloud channel can), as free per-PR CI with no credentials or tunnel. This PR's history is the reference for reviving the BrowserStack backend behind the same session interface if real-hardware coverage (OEM keyboards) ever earns automation. |
This was referenced Sep 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fifth layer of the stack, on #3031. Adds the OS-emulator layer between browser emulation and BrowserStack hardware, per the testing-foundation discussion.
What
The device suite's session becomes an interface with three backends; tests and page helpers are written once and run against whatever the machine can drive (
activeDevices()probes availability):localAndroid.ts— a local Android emulator via Playwright's first-party (experimental)_androidsupport: real Chrome as a Playwright page over CDP, input as genuine OS events viaadb shell(Playwright'sdevice.inputneeds a companion APK;shelldoesn't). Page→screen coordinates come from a one-time calibration tap on an app page — never Chrome's initial page, whose 980px virtual viewport poisons the mapping. This backend can press the on-screen keyboard itself:pressImeActionKeydrives Gboard's real action key with a verify ladder.browserstack.ts— the existing selenium client, unchanged behaviour, now one backend among several.localIos.ts— an iOS simulator via Appium/XCUITest (WebDriverAgent), driven by the same selenium client as BrowserStack: the actual iOS build and actual Safari, headless (XCUITest owns the HID stack, so the software keyboard works without the Simulator GUI), no tunnel.mobile: tapis the same channel BrowserStack iOS uses, so the gesture layer's chrome-offset ladders apply unchanged. Two dead ends are documented in code so nobody re-walks them: safaridriver's input is synthetic at the WebKit layer (never summons the keyboard; injecting real HID alongside trips Safari's "stop the current automated test session?" guardrail), and Appium's web-context clicks are synthetic too (nativeWebTapincluded).imeAction.device.test.tsis the previously-manual release-checklist item as a regression test: Gboard's real action key submits the link popover and focus stays in the editor — the flow no cloud automation can exercise (BrowserStack has no channel to the on-screen keyboard).CI
emulator-tests.ymlruns both targets on every PR and push to main — a KVM-backed API-35 emulator on ubuntu (3m29s) and an iPhone simulator via Appium on macos-15 (5m47s). Free minutes, no credentials, pinned actions, both green. Locally: boot any emulator (the setup boots the simulator itself) andpnpm run test:device— it runs whatever the machine can drive.Verified
Local matrix green: 11 tests across the API-35 emulator (Chrome 124 + Gboard) and the iPhone simulator (real iOS Safari), plus two consecutive iOS runs proving the boot/teardown cycle. CI green on both jobs. Bugs found along the way by this layer:
Array.findLastcrashes the editor on Chrome <97, andisTouchDevice()fails on Chrome 91'spointer: coarse— both filed for a support-matrix decision rather than silently patched.