Skip to content
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

chore(webkit): add video recording #23579

Merged
merged 32 commits into from
Sep 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
297e41b
factor console logging out of run.ts
flotwig Aug 25, 2022
173de9f
fix print-run
flotwig Aug 25, 2022
e493600
minimize diff
flotwig Aug 25, 2022
beb3f2d
chore(server): convert browsers/index to typescript
flotwig Aug 25, 2022
4957a83
fix tests
flotwig Aug 25, 2022
c255548
update stubbed tests
flotwig Aug 25, 2022
b01e8c0
convert electron.js to .ts
flotwig Aug 25, 2022
ce15192
Suggestions from code review
flotwig Aug 26, 2022
8bd49f3
Clean up new type errors
flotwig Aug 26, 2022
8e8f62a
electron.connectToExisting can be sync
flotwig Aug 26, 2022
501895a
more type errors for the type god
flotwig Aug 26, 2022
5e74399
Suggestions from code review
flotwig Aug 26, 2022
d25b9e0
refactor: move more of video capture into browser automations
flotwig Aug 27, 2022
9b8cfbe
unit tests
flotwig Aug 29, 2022
a7f6d4b
Merge remote-tracking branch 'origin/develop' into refactor-defaultBr…
flotwig Aug 29, 2022
e3b1979
refactor: move videoCapture to browsers
flotwig Aug 30, 2022
c3f6915
fix snapshots
flotwig Aug 30, 2022
ce0a27f
fix multi-spec videos?
flotwig Aug 30, 2022
e41dd70
webkit video recording works!
flotwig Aug 30, 2022
645a8d7
webkit system tests
flotwig Aug 30, 2022
059748b
skip system-tests that won't be fixed in this PR
flotwig Aug 31, 2022
84554e4
fix single-tab mode
flotwig Aug 31, 2022
2b9f33f
cleanup/api renames
flotwig Aug 31, 2022
115a07c
fix more tests
flotwig Aug 31, 2022
4ad2279
minimize diff, fix ff
flotwig Aug 31, 2022
a449c8f
fix unit tests
flotwig Aug 31, 2022
2468f9f
fix tests
flotwig Aug 31, 2022
d38ae27
cleanup
flotwig Aug 31, 2022
47adf21
Merge remote-tracking branch 'origin/develop' into webkit-video
flotwig Sep 1, 2022
22ddb94
clean up wantsWrite logic
flotwig Sep 2, 2022
45e9b8f
Clean up setVideoController/newFfmpegVideoController naming
flotwig Sep 6, 2022
edff645
Merge branch 'develop' into webkit-video
flotwig Sep 6, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 29 additions & 5 deletions circle.yml
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,14 @@ commands:
mv ~/cypress/system-tests/node_modules /tmp/node_modules_cache/system-tests_node_modules
mv ~/cypress/globbed_node_modules /tmp/node_modules_cache/globbed_node_modules

install-webkit-deps:
steps:
- run:
name: Install WebKit dependencies
command: |
npx playwright install webkit
npx playwright install-deps webkit

build-and-persist:
description: Save entire folder as artifact for other jobs to run without reinstalling
steps:
Expand Down Expand Up @@ -462,6 +470,11 @@ commands:
- install-chrome:
channel: <<parameters.install-chrome-channel>>
version: $(node ./scripts/get-browser-version.js chrome:<<parameters.install-chrome-channel>>)
- when:
condition:
equal: [ webkit, << parameters.browser >> ]
steps:
- install-webkit-deps
- run:
name: Run driver tests in Cypress
environment:
Expand All @@ -470,11 +483,6 @@ commands:
echo Current working directory is $PWD
echo Total containers $CIRCLE_NODE_TOTAL

if [[ "<<parameters.browser>>" = "webkit" ]]; then
npx playwright install webkit
npx playwright install-deps webkit
fi

if [[ -v MAIN_RECORD_KEY ]]; then
# internal PR
if <<parameters.experimentalSessionAndOrigin>>; then
Expand Down Expand Up @@ -610,6 +618,11 @@ commands:
steps:
- restore_cached_workspace
- restore_cached_system_tests_deps
- when:
condition:
equal: [ webkit, << parameters.browser >> ]
steps:
- install-webkit-deps
- run:
name: Run system tests
command: |
Expand Down Expand Up @@ -1448,6 +1461,13 @@ jobs:
- run-system-tests:
browser: firefox

system-tests-webkit:
<<: *defaults
parallelism: 8
steps:
- run-system-tests:
browser: webkit

system-tests-non-root:
<<: *defaults
steps:
Expand Down Expand Up @@ -2363,6 +2383,10 @@ linux-x64-workflow: &linux-x64-workflow
context: test-runner:performance-tracking
requires:
- system-tests-node-modules-install
- system-tests-webkit:
context: test-runner:performance-tracking
requires:
- system-tests-node-modules-install
- system-tests-non-root:
context: test-runner:performance-tracking
executor: non-root-docker-user
Expand Down
32 changes: 19 additions & 13 deletions packages/server/lib/browsers/chrome.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,11 @@ import { fs } from '../util/fs'
import { CdpAutomation, screencastOpts } from './cdp_automation'
import * as protocol from './protocol'
import utils from './utils'
import type { Browser } from './types'
import type { Browser, BrowserInstance } from './types'
import { BrowserCriClient } from './browser-cri-client'
import type { LaunchedBrowser } from '@packages/launcher/lib/browsers'
import type { CriClient } from './cri-client'
import type { Automation } from '../automation'
import type { BrowserLaunchOpts, BrowserNewTabOpts, WriteVideoFrame } from '@packages/types'
import type { BrowserLaunchOpts, BrowserNewTabOpts, RunModeVideoApi } from '@packages/types'

const debug = debugModule('cypress:server:browsers:chrome')

Expand Down Expand Up @@ -249,10 +248,12 @@ const _disableRestorePagesPrompt = function (userDir) {
.catch(() => { })
}

async function _recordVideo (cdpAutomation: CdpAutomation, writeVideoFrame: WriteVideoFrame, browserMajorVersion: number) {
const opts = browserMajorVersion >= CHROME_VERSION_WITH_FPS_INCREASE ? screencastOpts() : screencastOpts(1)
async function _recordVideo (cdpAutomation: CdpAutomation, videoOptions: RunModeVideoApi, browserMajorVersion: number) {
const screencastOptions = browserMajorVersion >= CHROME_VERSION_WITH_FPS_INCREASE ? screencastOpts() : screencastOpts(1)

await cdpAutomation.startVideoRecording(writeVideoFrame, opts)
const { writeVideoFrame } = await videoOptions.useFfmpegVideoController()

await cdpAutomation.startVideoRecording(writeVideoFrame, screencastOptions)
}

// a utility function that navigates to the given URL
Expand Down Expand Up @@ -552,7 +553,7 @@ export = {

if (!options.url) throw new Error('Missing url in connectToNewSpec')

await this.attachListeners(browser, options.url, pageCriClient, automation, options)
await this.attachListeners(options.url, pageCriClient, automation, options)
},

async connectToExisting (browser: Browser, options: BrowserLaunchOpts, automation: Automation) {
Expand All @@ -570,17 +571,21 @@ export = {
await this._setAutomation(pageCriClient, automation, browserCriClient.resetBrowserTargets, options)
},

async attachListeners (browser: Browser, url: string, pageCriClient: CriClient, automation: Automation, options: BrowserLaunchOpts | BrowserNewTabOpts) {
async attachListeners (url: string, pageCriClient: CriClient, automation: Automation, options: BrowserLaunchOpts | BrowserNewTabOpts) {
const browserCriClient = this._getBrowserCriClient()

if (!browserCriClient) throw new Error('Missing browserCriClient in attachListeners')

debug('attaching listeners to chrome %o', { url, options })

const cdpAutomation = await this._setAutomation(pageCriClient, automation, browserCriClient.resetBrowserTargets, options)

await pageCriClient.send('Page.enable')

await options['onInitializeNewBrowserTab']?.()

await Promise.all([
options.writeVideoFrame && this._recordVideo(cdpAutomation, options.writeVideoFrame, browser.majorVersion),
options.videoApi && this._recordVideo(cdpAutomation, options.videoApi, Number(options.browser.majorVersion)),
this._handleDownloads(pageCriClient, options.downloadsFolder, automation),
])

Expand All @@ -590,9 +595,11 @@ export = {
await this._handlePausedRequests(pageCriClient)
_listenForFrameTreeChanges(pageCriClient)
}

return cdpAutomation
},

async open (browser: Browser, url, options: BrowserLaunchOpts, automation: Automation): Promise<LaunchedBrowser> {
async open (browser: Browser, url, options: BrowserLaunchOpts, automation: Automation): Promise<BrowserInstance> {
const { isTextTerminal } = options

const userDir = utils.getProfileDir(browser, isTextTerminal)
Expand Down Expand Up @@ -644,7 +651,7 @@ export = {
// first allows us to connect the remote interface,
// start video recording and then
// we will load the actual page
const launchedBrowser = await launch(browser, 'about:blank', port, args) as LaunchedBrowser & { browserCriClient: BrowserCriClient}
const launchedBrowser = await launch(browser, 'about:blank', port, args) as unknown as BrowserInstance & { browserCriClient: BrowserCriClient}

la(launchedBrowser, 'did not get launched browser instance')

Expand All @@ -671,7 +678,6 @@ export = {

launchedBrowser.browserCriClient = browserCriClient

/* @ts-expect-error */
launchedBrowser.kill = (...args) => {
debug('closing remote interface client')

Expand All @@ -686,7 +692,7 @@ export = {

const pageCriClient = await browserCriClient.attachToTargetUrl('about:blank')

await this.attachListeners(browser, url, pageCriClient, automation, options)
await this.attachListeners(url, pageCriClient, automation, options)

// return the launched browser process
// with additional method to close the remote connection
Expand Down
32 changes: 17 additions & 15 deletions packages/server/lib/browsers/electron.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import * as errors from '../errors'
import type { Browser, BrowserInstance } from './types'
import type { BrowserWindow, WebContents } from 'electron'
import type { Automation } from '../automation'
import type { BrowserLaunchOpts, Preferences } from '@packages/types'
import type { BrowserLaunchOpts, Preferences, RunModeVideoApi } from '@packages/types'

// TODO: unmix these two types
type ElectronOpts = Windows.WindowOptions & BrowserLaunchOpts
Expand Down Expand Up @@ -44,7 +44,7 @@ const tryToCall = function (win, method) {
}
}

const _getAutomation = async function (win, options, parent) {
const _getAutomation = async function (win, options: BrowserLaunchOpts, parent) {
async function sendCommand (method: CdpCommand, data?: object) {
return tryToCall(win, () => {
return win.webContents.debugger.sendCommand
Expand Down Expand Up @@ -72,7 +72,7 @@ const _getAutomation = async function (win, options, parent) {
// after upgrading to Electron 8, CDP screenshots can hang if a screencast is not also running
// workaround: start and stop screencasts between screenshots
// @see https://github.com/cypress-io/cypress/pull/6555#issuecomment-596747134
if (!options.writeVideoFrame) {
if (!options.videoApi) {
await sendCommand('Page.startScreencast', screencastOpts())
const ret = await fn(message, data)

Expand Down Expand Up @@ -109,6 +109,12 @@ function _installExtensions (win: BrowserWindow, extensionPaths: string[], optio
}))
}

async function recordVideo (cdpAutomation: CdpAutomation, videoApi: RunModeVideoApi) {
const { writeVideoFrame } = await videoApi.useFfmpegVideoController()

await cdpAutomation.startVideoRecording(writeVideoFrame)
}

export = {
_defaultOptions (projectRoot: string | undefined, state: Preferences, options: BrowserLaunchOpts, automation: Automation): ElectronOpts {
const _this = this
Expand Down Expand Up @@ -179,7 +185,7 @@ export = {

_getAutomation,

async _render (url: string, automation: Automation, preferences, options: { projectRoot?: string, isTextTerminal: boolean }) {
async _render (url: string, automation: Automation, preferences, options: ElectronOpts) {
const win = Windows.create(options.projectRoot, preferences)

if (preferences.browser.isHeadless) {
Expand All @@ -192,7 +198,7 @@ export = {
win.maximize()
}

const launched = await this._launch(win, url, automation, preferences)
const launched = await this._launch(win, url, automation, preferences, options.videoApi)

automation.use(await _getAutomation(win, preferences, automation))

Expand Down Expand Up @@ -225,7 +231,7 @@ export = {
return this._launch(win, url, automation, electronOptions)
},

async _launch (win: BrowserWindow, url: string, automation: Automation, options) {
async _launch (win: BrowserWindow, url: string, automation: Automation, options: ElectronOpts, videoApi?: RunModeVideoApi) {
if (options.show) {
menu.set({ withInternalDevTools: true })
}
Expand All @@ -239,9 +245,7 @@ export = {

this._attachDebugger(win.webContents)

let ua

ua = options.userAgent
const ua = options.userAgent

if (ua) {
this._setUserAgent(win.webContents, ua)
Expand Down Expand Up @@ -273,8 +277,9 @@ export = {
const cdpAutomation = await this._getAutomation(win, options, automation)

automation.use(cdpAutomation)

await Promise.all([
options.writeVideoFrame && cdpAutomation.startVideoRecording(options.writeVideoFrame),
videoApi && recordVideo(cdpAutomation, videoApi),
this._handleDownloads(win, options.downloadsFolder, automation),
])

Expand Down Expand Up @@ -445,7 +450,7 @@ export = {
})
},

async connectToNewSpec (browser, options, automation) {
async connectToNewSpec (browser: Browser, options: ElectronOpts, automation: Automation) {
if (!options.url) throw new Error('Missing url in connectToNewSpec')

await this.open(browser, options.url, options, automation)
Expand Down Expand Up @@ -480,10 +485,7 @@ export = {

debug('launching browser window to url: %s', url)

const win = await this._render(url, automation, preferences, {
projectRoot: options.projectRoot,
isTextTerminal: options.isTextTerminal,
})
const win = await this._render(url, automation, preferences, electronOptions)

await _installExtensions(win, launchOptions.extensions, electronOptions)

Expand Down
14 changes: 10 additions & 4 deletions packages/server/lib/browsers/firefox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import Debug from 'debug'
import getPort from 'get-port'
import path from 'path'
import urlUtil from 'url'
import { debug as launcherDebug, launch, LaunchedBrowser } from '@packages/launcher/lib/browsers'
import { debug as launcherDebug, launch } from '@packages/launcher/lib/browsers'
import { doubleEscape } from '@packages/launcher/lib/windows'
import FirefoxProfile from 'firefox-profile'
import * as errors from '../errors'
Expand All @@ -20,7 +20,7 @@ import type { BrowserCriClient } from './browser-cri-client'
import type { Automation } from '../automation'
import { getCtx } from '@packages/data-context'
import { getError } from '@packages/errors'
import type { BrowserLaunchOpts, BrowserNewTabOpts } from '@packages/types'
import type { BrowserLaunchOpts, BrowserNewTabOpts, RunModeVideoApi } from '@packages/types'

const debug = Debug('cypress:server:browsers:firefox')

Expand Down Expand Up @@ -379,6 +379,12 @@ export function connectToExisting () {
getCtx().onWarning(getError('UNEXPECTED_INTERNAL_ERROR', new Error('Attempting to connect to existing browser for Cypress in Cypress which is not yet implemented for firefox')))
}

async function recordVideo (videoApi: RunModeVideoApi) {
const { writeVideoFrame } = await videoApi.useFfmpegVideoController({ webmInput: true })

videoApi.onProjectCaptureVideoFrames(writeVideoFrame)
}

export async function open (browser: Browser, url: string, options: BrowserLaunchOpts, automation: Automation): Promise<BrowserInstance> {
// see revision comment here https://wiki.mozilla.org/index.php?title=WebDriver/RemoteProtocol&oldid=1234946
const hasCdp = browser.majorVersion >= 86
Expand Down Expand Up @@ -456,6 +462,7 @@ export async function open (browser: Browser, url: string, options: BrowserLaunc
utils.ensureCleanCache(browser, options.isTextTerminal),
utils.writeExtension(browser, options.isTextTerminal, options.proxyUrl, options.socketIoRoute),
utils.executeBeforeBrowserLaunch(browser, defaultLaunchOptions, options),
options.videoApi && recordVideo(options.videoApi),
])

if (Array.isArray(launchOptions.extensions)) {
Expand Down Expand Up @@ -529,7 +536,7 @@ export async function open (browser: Browser, url: string, options: BrowserLaunc
// user can overwrite this default with these env vars or --height, --width arguments
MOZ_HEADLESS_WIDTH: '1280',
MOZ_HEADLESS_HEIGHT: '721',
}) as LaunchedBrowser & { browserCriClient: BrowserCriClient}
}) as unknown as BrowserInstance
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't seen this before, tricky.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah... I don't know the "right" way to change the type of an object, type-safely, without using class ... extends .... If there even is a way without as unknown. Would be nice to eventually remove the need for this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one seems a quick fix #23693

I think what you are referring to is something like

function foo<T extends SomeBaseClass>(arg: T): T {
  // 
}

Now as long as arg extends SomeBaseClass you should get type safety (error if you don't extend the base class).


try {
browserCriClient = await firefoxUtil.setup({ automation, extensions: launchOptions.extensions, url, foxdriverPort, marionettePort, remotePort, onError: options.onError, options })
Expand All @@ -543,7 +550,6 @@ export async function open (browser: Browser, url: string, options: BrowserLaunc
// monkey-patch the .kill method to that the CDP connection is closed
const originalBrowserKill = browserInstance.kill

/* @ts-expect-error */
browserInstance.kill = (...args) => {
debug('closing remote interface client')

Expand Down
Loading