fix: wait for two animation frames to prevent flaky test#206
fix: wait for two animation frames to prevent flaky test#206Johan Gorter (johan-gorter) merged 2 commits intomainfrom
Conversation
Johan Gorter (johan-gorter)
force-pushed
the
fix-flaky-browser-test
branch
from
52c8214 to
7001649
Compare
…ky test
Maquette only updates the DOM input value if it differs from the previous
render's value (to preserve cursor position). When fill() and press('Enter')
happen without an intervening render, this race condition occurs:
1. Initial render has value: '', previousValue = ''
2. fill() sets DOM value, schedules render
3. press('Enter') fires BEFORE animation frame
4. Handler clears newTodoTitle to ''
5. Render happens with value: ''
6. propValue ('') === previousValue (''), so DOM is NOT updated!
The fix adds a waitForAnimationFrame() after fill() to ensure maquette
renders the filled value before pressing Enter, so previousValue is updated.
Johan Gorter (johan-gorter)
requested review from
Copilot and
Joeri van Oostveen (jvanoostveen)
There was a problem hiding this comment.
Pull request overview
This PR fixes a flaky test issue in the TodoMVC browser tests by modifying the waitForAnimationFrame() helper to wait for two animation frames instead of one. The change addresses a race condition where Playwright's page.evaluate() round-trip latency could cause maquette's render to fire before the callback was registered.
Changes:
- Modified
waitForAnimationFrame()to wait for two animation frames using nestedrequestAnimationFramecalls - Added a new wait before pressing Enter in
addTodo()to ensure maquette renders the filled input value first - Updated comments to explain the timing requirements and clarify the purpose of each wait
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Joeri van Oostveen (jvanoostveen)
left a comment
Joeri van Oostveen (jvanoostveen)
left a comment
There was a problem hiding this comment.
Ik zou eigenlijk verwachten dat die hele 'waitForAnimationFrame' niet nodig is, Playwright zou op zich retry-ability moeten toepassen?
Klopt, maar zo waren de tests initieel niet opgezet, ze zijn letterlijk omgeschreven. |
Fixes flaky test by waiting for two animation frames instead of one in waitForAnimationFrame helper. The round-trip time of Playwright's page.evaluate could cause maquette's render to fire before our callback was registered.