fix(playwright): replace monolithic waitForCanvas with scoped async helpers#176
Conversation
Agent-Logs-Url: https://github.com/MaximumTrainer/MaximumTrainer_Redux/sessions/bd4672d9-4702-4fc0-b11f-bf44907bfa99 Co-authored-by: MaximumTrainer <1376575+MaximumTrainer@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR refactors the Playwright WASM BLE API tests’ synchronization logic to avoid CI timeouts by breaking a monolithic “wait for everything” helper into smaller, scoped async helpers and only waiting for the FTMS write in the one test that needs it.
Changes:
- Replaced
waitForCanvas()withwaitForAppReady()(canvas visible) plustriggerBleScanAndWaitForRequestDevice()(scan hook + earliest mock signal). - Added
waitForFtmsWrite()and used it only in the FTMS control-point write test. - Made missing
window.mt_startBleScanfail fast instead of silently no-op’ing and timing out.
Comments suppressed due to low confidence (1)
tests/playwright/wasm_btle_api.spec.js:127
waitForFtmsWrite()resolves on the first recordedwriteValueWithResponse, but the comment and the test intent are specifically about the FTMS Control Point write (UUID 0x2AD9, opcode 0x00). To avoid false positives if other writes are introduced (e.g., viajs_sendFtmsCommand), tighten the wait predicate to check for a matching FTMS control-point write rather than any write.
await page.waitForFunction(
() => {
const calls = window._btleApiCalls;
return calls &&
Array.isArray(calls.writeValueWithResponse) &&
calls.writeValueWithResponse.length > 0;
},
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Earliest reliable signal: our injected mock recorded a requestDevice call. | ||
| await page.waitForFunction( | ||
| () => { | ||
| const calls = window._btleApiCalls; | ||
| return calls && calls.requestDeviceFilters !== null; | ||
| }, | ||
| { timeout: 45000 } |
There was a problem hiding this comment.
triggerBleScanAndWaitForRequestDevice() waits for calls.requestDeviceFilters !== null as a proxy for “requestDevice was called”, but requestDeviceFilters is initialized to null and can legitimately remain null if the app ever calls requestDevice() without filters. In that case the helper will hang until timeout rather than failing with a clear assertion. Consider recording an explicit requestDeviceCalled flag in the injected mock (or initializing requestDeviceFilters to undefined and waiting for it to become defined) and wait on that instead.
|
@copilot apply changes based on the comments in this thread |
…questDevice call Agent-Logs-Url: https://github.com/MaximumTrainer/MaximumTrainer_Redux/sessions/aa9f4129-a489-477a-8b88-fd1e05db76e0 Co-authored-by: MaximumTrainer <1376575+MaximumTrainer@users.noreply.github.com>
Done in 51286a2. |
All 4 WASM BLE API tests timed out because
waitForCanvas()required a successful FTMS control-point write before any assertion could run — a condition that frequently never completes on CI against the live GitHub Pages deployment.Changes
Split
waitForCanvas()into three focused helpers:waitForAppReady()— waits only for#qt-canvas-wrapperto become visibletriggerBleScanAndWaitForRequestDevice()— fails fast ifwindow.mt_startBleScanis absent, triggers the scan, then waits forrequestDeviceto be called (earliest reliable mock signal)waitForFtmsWrite()— waits forwriteValueWithResponseto be recorded; called only in the FTMS write testFail fast on missing scan hook — replaces the silent
if (typeof window.mt_startBleScan === 'function')no-op with an explicitexpect(...).toBeTruthy(), turning a mystery timeout into an actionable failure when the deployed build stops exporting the hookBefore / After