Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to reduce browser UI test flakiness in CI by standardizing visibility timeouts, improving saved-recording visual validation, and making Learn startup tracing more deterministic across early frames.
Changes:
- Align editor overlay visibility waiting with the shared default visible timeout.
- Update the saved-recording harness to probe multiple video frames before concluding the recording is visually black.
- Add deterministic
requestAnimationFrame-based sampling to Learn startup tracing.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/PrompterOne.App.UITests/Media/recording-file-harness.js | Adds multi-frame probing for visible video detection in the recording analysis harness. |
| tests/PrompterOne.App.UITests/Learn/LearnStartupAlignmentTests.cs | Adds early-frame sampling via requestAnimationFrame to stabilize startup trace collection. |
| tests/PrompterOne.App.UITests/Editor/EditorOverlayInteractionTests.cs | Uses the shared default visible timeout when waiting for the editor source input. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| async function waitForNextVideoFrame(videoElement) { | ||
| if (typeof videoElement.requestVideoFrameCallback === "function") { | ||
| await new Promise(resolve => videoElement.requestVideoFrameCallback(() => resolve())); | ||
| return; | ||
| } | ||
|
|
||
| await new Promise(resolve => window.setTimeout(resolve, visibleVideoPollDelayMs)); | ||
| } |
There was a problem hiding this comment.
waitForNextVideoFrame can hang indefinitely when requestVideoFrameCallback never fires (e.g., if videoElement.play() fails or the video never advances frames). Because this await has no timeout, detectVisibleVideoAcrossFrames may never reach its deadline and the harness can stall the test run. Consider racing requestVideoFrameCallback with a setTimeout fallback (or check paused/ended and fall back to polling) so the probe always makes progress and respects visibleVideoProbeTimeoutMs.
| capture(); | ||
|
|
||
| if (window.__learnStartupTrace.length >= maxSamples || animationFramePasses >= maxAnimationFramePasses) { | ||
| return; | ||
| } | ||
|
|
||
| animationFramePasses += 1; |
There was a problem hiding this comment.
animationFramePasses is checked before it is incremented, so captureOnAnimationFrame will run one extra requestAnimationFrame callback beyond maxAnimationFramePasses (it still captures once more before returning). If the intent is to cap RAF sampling to exactly maxAnimationFramePasses, increment before the check or adjust the comparison so the maximum number of scheduled passes matches the constant.
| capture(); | |
| if (window.__learnStartupTrace.length >= maxSamples || animationFramePasses >= maxAnimationFramePasses) { | |
| return; | |
| } | |
| animationFramePasses += 1; | |
| if (window.__learnStartupTrace.length >= maxSamples || animationFramePasses >= maxAnimationFramePasses) { | |
| return; | |
| } | |
| animationFramePasses += 1; | |
| capture(); | |
| if (window.__learnStartupTrace.length >= maxSamples || animationFramePasses >= maxAnimationFramePasses) { | |
| return; | |
| } |
|
part of #9 |
Summary
Verification