fix(expect): don't sleep past the deadline in retry loop#40290
fix(expect): don't sleep past the deadline in retry loop#40290yury-s wants to merge 1 commit intomicrosoft:mainfrom
Conversation
The retry loop in retryWithProgressAndTimeouts could sleep past the assertion deadline, aborting mid-sleep without ever performing a final check. An element appearing during that final sleep was missed even when the DOM matched before the deadline. Clamp each sleep to the remaining budget (minus a 100ms margin for the next action) so the loop always wakes up in time to run another check. Fixes microsoft#40281
|
Not sure we want this |
Test results for "tests 1"6 flaky39249 passed, 847 skipped Merge workflow run. |
Test results for "MCP"13 failed 6382 passed, 976 skipped Merge workflow run. |
| const timeout = timeouts[Math.min(timeoutIndex++, timeouts.length - 1)]; | ||
| let timeout = timeouts[Math.min(timeoutIndex++, timeouts.length - 1)]; | ||
| if (timeout && progress.deadline) { | ||
| // Clamp the sleep to the remaining budget (minus a small margin for the |
There was a problem hiding this comment.
100ms is arbitrary here, it can take any amount of time, so we are trading a 1000ms race for 100ms race. not quite sure that is what we want, but I don't have a good idea on it either. as an idea, you could measure the action and give it twice its budget?
There was a problem hiding this comment.
Perhaps we can try to run the "one-shot" logic at the end of the timeout, instead of at the start? Not sure how that will look like, but worth a try.
There was a problem hiding this comment.
I didn't like that as we could fall far behind the deadline, but if the operation is long it's not much different than starting 100ms before the deadline, we can try that.
3 participants
Summary
retryWithProgressAndTimeoutscould sleep past the assertion deadline and abort mid-sleep without a final check, so an element that appeared during that last sleep was missed.Fixes #40281