New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Skip empty video chunks in writeVideoFrame #6818
Conversation
Thanks for the contribution! Below are some guidelines Cypress uses when doing PR reviews.
PR Review ChecklistIf any of the following requirements can't be met, leave a comment in the review selecting 'Request changes', otherwise 'Approve'. User Experience
Functionality
Maintainability
Quality
Internal
|
Test summaryRun details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Can you write a single unit test for this? Otherwise this guard may get lost at some point.
- Also can you add a note above the conditional explaining why this is important. The context is lost unless you read the comments.
const { expect, sinon } = require('../spec_helper') | ||
import videoCapture from '../../lib/video_capture' | ||
import path from 'path' | ||
import fse from 'fs-extra' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's a packages/fs
you could use... but it will yell at you for using *sync
methods
Released in This comment thread has been locked. If you are still experiencing this issue after upgrading to |
No users have run into this, but video recording can fail in
writeVideoFrame
and crash the process if an empty video chunk is received. This only seems to happen if there's some other error recording the video, but that will be caught later on, so this patch prevents errors with video recording from crashing the process immediately by skipping empty video chunks altogether.