fix: remove ffmpeg-dependent e2e test, document viewport regression in code#1236
Open
jin-2-kakaoent wants to merge 1 commit intovercel-labs:mainfrom
Open
fix: remove ffmpeg-dependent e2e test, document viewport regression in code#1236jin-2-kakaoent wants to merge 1 commit intovercel-labs:mainfrom
jin-2-kakaoent wants to merge 1 commit intovercel-labs:mainfrom
Conversation
…n code The e2e_recording_inherits_viewport test required ffmpeg at runtime, causing CI failures on ubuntu-latest where ffmpeg is not installed. Since a skip approach would leave a permanently dead test, the test is removed instead. The regression context (vercel-labs#1208) is preserved as a comment co-located with the viewport inheritance logic in actions.rs, matching the pattern used by adjacent download and HTTPS error comments.
Contributor
|
@hyunjinee is attempting to deploy a commit to the Vercel Labs Team on Vercel. A member of the Team first needs to authorize it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The
e2e_recording_inherits_viewporttest (added in #1208) was failing CI on every push tomain. The test verifies thatrecording_startinherits the current viewport dimensions into the newly created recording context, but it requiresffmpegto be installed at runtime.The
ubuntu-latestGitHub Actions runner does not haveffmpegpre-installed, so therecording_startcommand fails immediately with:This caused the
Native E2E Testsjob to fail on every push.Why not just skip when ffmpeg is missing?
An early-return skip (checking
which ffmpegat test start) would make the test permanently dead in CI — it would appear in the test suite but never execute, creating false confidence. A test that never runs provides no regression protection and is worse than no test at all.Why not install ffmpeg in CI?
Installing ffmpeg in the CI runner is a heavyweight solution for a single test. ffmpeg is a large dependency and would add meaningful overhead to every CI run.
Solution
Remove the test and instead preserve the regression context as a comment co-located with the viewport inheritance logic in
actions.rs. This matches the pattern already used by the adjacent download behavior and HTTPS error comments inhandle_recording_start:Anyone modifying
handle_recording_startwill see this comment directly in the code, making the regression risk visible at the point of change — which is more reliable than a test that never runs in CI.Test plan
Native E2E TestsCI job passes on this branch