From bb0af314ba36ba2db236ef3a815b362d0f567075 Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Mon, 8 Feb 2021 10:59:48 -0800 Subject: [PATCH] fix(video): set default size to fit into 800x800 (#5333) --- src/server/browserContext.ts | 14 ++++++++++++++ test/screencast.spec.ts | 36 +++++++++++++++++++++++++++++------- 2 files changed, 43 insertions(+), 7 deletions(-) diff --git a/src/server/browserContext.ts b/src/server/browserContext.ts index 4e9e8e77a90a7..5d6201ea4a2c8 100644 --- a/src/server/browserContext.ts +++ b/src/server/browserContext.ts @@ -414,6 +414,20 @@ export function validateBrowserContextOptions(options: types.BrowserContextOptio throw new Error(`"isMobile" option is not supported with null "viewport"`); if (!options.viewport && !options.noDefaultViewport) options.viewport = { width: 1280, height: 720 }; + if (options.recordVideo && !options.recordVideo.size) { + if (options.noDefaultViewport) { + options.recordVideo.size = { width: 800, height: 600 }; + } else { + const size = options.viewport!; + const scale = 800 / Math.max(size.width, size.height); + if (scale < 1) { + options.recordVideo.size = { + width: Math.floor(size.width * scale), + height: Math.floor(size.height * scale) + }; + } + } + } if (options.proxy) { if (!browserOptions.proxy) throw new Error(`Browser needs to be launched with the global proxy. If all contexts override the proxy, global proxy will be never used and can be any string, for example "launch({ proxy: { server: 'per-context' } })"`); diff --git a/test/screencast.spec.ts b/test/screencast.spec.ts index 501ed5022403f..73cf4e0c9d1b0 100644 --- a/test/screencast.spec.ts +++ b/test/screencast.spec.ts @@ -45,6 +45,8 @@ export class VideoPlayer { const lines = this.output.split('\n'); let framesLine = lines.find(l => l.startsWith('frame='))!; + if (!framesLine) + throw new Error(`No frame data in the output:\n${this.output}`); framesLine = framesLine.substring(framesLine.lastIndexOf('frame=')); const framesMatch = framesLine.match(/frame=\s+(\d+)/); const streamLine = lines.find(l => l.trim().startsWith('Stream #0:0')); @@ -398,8 +400,8 @@ describe('screencast', suite => { } }); - it('should use viewport as default size', async ({browser, testInfo}) => { - const size = {width: 800, height: 600}; + it('should use viewport scaled down to fit into 800x800 as default size', async ({browser, testInfo}) => { + const size = {width: 1600, height: 1200}; const context = await browser.newContext({ recordVideo: { dir: testInfo.outputPath(''), @@ -413,11 +415,11 @@ describe('screencast', suite => { const videoFile = await page.video().path(); const videoPlayer = new VideoPlayer(videoFile); - expect(videoPlayer.videoWidth).toBe(size.width); - expect(videoPlayer.videoHeight).toBe(size.height); + expect(videoPlayer.videoWidth).toBe(800); + expect(videoPlayer.videoHeight).toBe(600); }); - it('should be 1280x720 by default', async ({browser, testInfo}) => { + it('should be 800x450 by default', async ({browser, testInfo}) => { const context = await browser.newContext({ recordVideo: { dir: testInfo.outputPath(''), @@ -430,8 +432,28 @@ describe('screencast', suite => { const videoFile = await page.video().path(); const videoPlayer = new VideoPlayer(videoFile); - expect(videoPlayer.videoWidth).toBe(1280); - expect(videoPlayer.videoHeight).toBe(720); + expect(videoPlayer.videoWidth).toBe(800); + expect(videoPlayer.videoHeight).toBe(450); + }); + + it('should be 800x600 with null viewport', (test, { headful, browserName }) => { + test.fixme(browserName === 'firefox' && !headful, 'Fails in headless on bots'); + }, async ({ browser, testInfo }) => { + const context = await browser.newContext({ + recordVideo: { + dir: testInfo.outputPath(''), + }, + viewport: null + }); + + const page = await context.newPage(); + await new Promise(r => setTimeout(r, 1000)); + await context.close(); + + const videoFile = await page.video().path(); + const videoPlayer = new VideoPlayer(videoFile); + expect(videoPlayer.videoWidth).toBe(800); + expect(videoPlayer.videoHeight).toBe(600); }); it('should capture static page in persistent context', async ({launchPersistent, testInfo}) => {