diff --git a/atest/test/01_Browser_Management/persistent_state.robot b/atest/test/01_Browser_Management/persistent_state.robot index e5c0357e4..d230e0678 100644 --- a/atest/test/01_Browser_Management/persistent_state.robot +++ b/atest/test/01_Browser_Management/persistent_state.robot @@ -45,7 +45,7 @@ Switching Between Two Persistent Contexts Works New Context Fails With Persistent Context New Persistent Context Run Keyword And Expect Error - ... Error: Trying to create a new context when a persistentContext is active + ... Error: Cannot create a new context in chromium, because its context is persistent or adopted. Open a new browser first. ... New Context New Persistent Context Can Use An URL diff --git a/atest/test/05_JS_Tests/adopt.js b/atest/test/05_JS_Tests/adopt.js index 4dce58c26..cc6cefd21 100644 --- a/atest/test/05_JS_Tests/adopt.js +++ b/atest/test/05_JS_Tests/adopt.js @@ -1,13 +1,12 @@ let adoptedContextClosed = false; let onCloseCalled = false; -async function openAdoptedPersistentContext(userDataDir, url, headless, playwright, adoptContext, defaultTimeout) { +async function openAdoptedPersistentContext(userDataDir, url, headless, playwright, adoptContext) { const context = await playwright.chromium.launchPersistentContext(userDataDir, { headless: String(headless).toLowerCase() !== 'false', }); context.on('close', () => (adoptedContextClosed = true)); await context.pages()[0].goto(url); - context.setDefaultTimeout(defaultTimeout); return adoptContext(context, { onClose: async () => { onCloseCalled = true; @@ -49,10 +48,8 @@ async function openAdoptedPersistentContextWithRecording(userDataDir, url, headl tracesDir, ...contextOptions, }); - const page = context.pages()[0]; - await page.goto(url); - const adopted = await adoptContext(context, { tracing, contextOptions }); - return { ...adopted, videoPath: await page.video().path() }; + await context.pages()[0].goto(url); + return adoptContext(context, { tracing, contextOptions }); } exports.__esModule = true; diff --git a/atest/test/05_JS_Tests/adopt_context.robot b/atest/test/05_JS_Tests/adopt_context.robot index 1f8841b46..35a1a30da 100644 --- a/atest/test/05_JS_Tests/adopt_context.robot +++ b/atest/test/05_JS_Tests/adopt_context.robot @@ -17,7 +17,7 @@ Browser Keywords Work On A Context Adopted By A JavaScript Extension ${on_close_called} = Adopted Context On Close Was Called Should Be True ${on_close_called} -An Extension Gives An Adopted Context The Library Timeout +An Adopted Context Gets The Library Timeout Set Browser Timeout 1s scope=Test Open Adopted Persistent Context ${OUTPUT_DIR}/adopted-profile-timeout ${LOGIN_URL} ${HEADLESS} ${message} = Message Of A Failed Wait diff --git a/node/playwright-wrapper/__tests__/playwright-state.test.ts b/node/playwright-wrapper/__tests__/playwright-state.test.ts index 005185230..05dcf5e84 100644 --- a/node/playwright-wrapper/__tests__/playwright-state.test.ts +++ b/node/playwright-wrapper/__tests__/playwright-state.test.ts @@ -19,6 +19,7 @@ import { closeContext, extensionKeywordCall, locatorCache, + newContext, newPage, PlaywrightState, removeFailedPage, @@ -484,7 +485,7 @@ describe('removeFailedPage', () => { }); function makeMockPage() { - return { on: jest.fn() } as any; + return { on: jest.fn(), video: () => null } as any; } function makeMockContext(pages: any[], close = jest.fn().mockResolvedValue(undefined)) { @@ -574,6 +575,33 @@ describe('adoptContext', () => { expect(adopted.pageId).toBe(state.activeBrowser?.page?.id); }); + it('returns the video path of the active page', async () => { + const state = new PlaywrightState(); + const first = { on: jest.fn(), video: () => ({ path: async () => '/videos/first.webm' }) } as any; + const second = { on: jest.fn(), video: () => ({ path: async () => '/videos/second.webm' }) } as any; + + const adopted = await state.adoptContext(makeMockContext([first, second])); + + expect(adopted.videoPath).toBe('/videos/first.webm'); + }); + + it('returns no video path when the context records no video', async () => { + const state = new PlaywrightState(); + const page = { on: jest.fn(), video: () => null } as any; + + const adopted = await state.adoptContext(makeMockContext([page])); + + expect(adopted.videoPath).toBeNull(); + }); + + it('returns no video path when the context has no pages', async () => { + const state = new PlaywrightState(); + + const adopted = await state.adoptContext(makeMockContext([])); + + expect(adopted.videoPath).toBeNull(); + }); + it('keeps an existing browser state with a null browser separate', async () => { const state = new PlaywrightState(); @@ -638,6 +666,50 @@ describe('adoptContext', () => { }); }); +describe('newContext', () => { + it('explains that an adopted context cannot get a sibling context', async () => { + const state = new PlaywrightState(); + await state.adoptContext(makeMockContext([makeMockPage()]), { name: 'electron' }); + + await expect(newContext({ rawOptions: '{}', defaultTimeout: 0, traceFile: '' }, state)).rejects.toThrow( + 'Cannot create a new context in electron, because its context is persistent or adopted. ' + + 'Open a new browser first.', + ); + }); +}); + +describe('addBrowser', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('removes a page that existed when the browser was connected once it closes', async () => { + const listeners: Record void> = {}; + const page = { on: (event: string, listener: () => void) => (listeners[event] = listener), video: () => null }; + const context = makeMockContext([page]); + const browser = { version: () => '1', contexts: () => [context] } as any; + const state = new PlaywrightState(); + + await state.addBrowser({ browser, browserType: 'chromium', headless: true }); + listeners['close']?.(); + + expect(state.activeBrowser?.context?.pageStack).toEqual([]); + }); + + it('indexes a page once when the context reports it while the browser is being added', async () => { + const page = makeMockPage(); + const context = makeMockContext([page]); + context.on = (event: string, listener: (page: unknown) => void) => event === 'page' && listener(page); + const browser = { version: () => '1', contexts: () => [context] } as any; + const state = new PlaywrightState(); + + await state.addBrowser({ browser, browserType: 'chromium', headless: true }); + await new Promise((resolve) => setTimeout(resolve, 0)); + + expect(state.activeBrowser?.context?.pageStack.map((indexed) => indexed.p)).toEqual([page]); + }); +}); + describe('extensionKeywordCall', () => { beforeEach(() => { jest.clearAllMocks(); @@ -646,11 +718,7 @@ describe('extensionKeywordCall', () => { it('lets an extension function adopt a context it created', async () => { const state = new PlaywrightState(); const context = makeMockContext([makeMockPage()]); - async function openOwnContext( - defaultTimeout: number, - adoptContext: (c: unknown, options?: unknown) => Promise, - ) { - context.setDefaultTimeout(defaultTimeout); + async function openOwnContext(adoptContext: (c: unknown, options?: unknown) => Promise) { return adoptContext(context, { name: 'own' }); } state.extensions.push({ openOwnContext } as any); @@ -660,10 +728,7 @@ describe('extensionKeywordCall', () => { { name: 'openOwnContext', arguments: JSON.stringify({ - arguments: [ - ['defaultTimeout', 5000], - ['adoptContext', 'RESERVED'], - ], + arguments: [['adoptContext', 'RESERVED']], }), }, call, @@ -674,6 +739,30 @@ describe('extensionKeywordCall', () => { expect(adopted.browserId).toBe(state.activeBrowser?.id); expect(state.activeBrowser?.context?.c).toBe(context); expect(state.activeBrowser?.name).toBe('own'); + }); + + it('gives an adopted context the default timeout of the call', async () => { + const state = new PlaywrightState(); + const context = makeMockContext([makeMockPage()]); + async function openOwnContext(adoptContext: (c: unknown) => Promise) { + return adoptContext(context); + } + state.extensions.push({ openOwnContext } as any); + + await extensionKeywordCall( + { + name: 'openOwnContext', + arguments: JSON.stringify({ + arguments: [ + ['adoptContext', 'RESERVED'], + ['defaultTimeout', 5000], + ], + }), + }, + { write: jest.fn() } as any, + state, + ); + expect(context.setDefaultTimeout).toHaveBeenCalledWith(5000); }); diff --git a/node/playwright-wrapper/playwright-state.ts b/node/playwright-wrapper/playwright-state.ts index 8f349f212..e2d7aae57 100644 --- a/node/playwright-wrapper/playwright-state.ts +++ b/node/playwright-wrapper/playwright-state.ts @@ -174,7 +174,7 @@ export async function extensionKeywordCall( apiArguments.set('logger', (msg: string) => call.write(jsonResponse('', msg))); apiArguments.set('playwright', playwright); apiArguments.set('adoptContext', (context: BrowserContext, options?: AdoptContextOptions) => - state.adoptContext(context, options), + state.adoptContext(context, options, namedArguments.defaultTimeout as number | undefined), ); // Injected values can be null, for example the browser of an adopted context. const functionArguments = argNames.map((argName) => @@ -525,7 +525,7 @@ export class PlaywrightState { ); } - public addBrowser(browserAndConfs: BrowserAndConfs): BrowserState { + public async addBrowser(browserAndConfs: BrowserAndConfs): Promise { const adding_browser = browserAndConfs.browser; logger.info(`Adding browser to stack: ${browserAndConfs.browserType}, version: ${adding_browser?.version()}`); let browserState = this.browserStack.find((b) => b.browser === adding_browser); @@ -538,26 +538,25 @@ export class PlaywrightState { if (browserContexts !== undefined) { logger.info(`Adding ${browserContexts.length} contexts to browser`); for (const context of browserContexts) { - const indexedPages = context.pages().map((page) => indexedPage(page)); - logger.info(`Adding ${indexedPages.length} pages to context`); - const indexedContext = { - id: `context=${uuidv4()}`, - c: context, - pageStack: indexedPages, - options: {}, - traceFile: '', - }; - indexedContext.c.on('page', async (page) => { - indexedContext.pageStack.unshift(await _newPage(indexedContext, page)); - }); - browserState?.pushContext(indexedContext); + const indexedContext = await _createIndexedContext(context, undefined, '', {}); + for (const page of context.pages()) { + if (!indexedContext.pageStack.some((indexed) => indexed.p === page)) { + indexedContext.pageStack.push(await _newPage(indexedContext, page)); + } + } + logger.info(`Adding ${indexedContext.pageStack.length} pages to context`); + browserState.pushContext(indexedContext); } } this.browserStack.push(browserState); return browserState; } - public async adoptContext(context: BrowserContext, options: AdoptContextOptions = {}): Promise { + public async adoptContext( + context: BrowserContext, + options: AdoptContextOptions = {}, + defaultTimeout?: number, + ): Promise { const browserState = new BrowserState({ browser: null, name: options.name ?? 'adopted', @@ -566,13 +565,19 @@ export class PlaywrightState { browserState.onClose = options.onClose; const indexedContext = await _indexContextWithPages( context, - undefined, + defaultTimeout, options.tracing ?? '', options.contextOptions, ); browserState.pushContext(indexedContext); this.browserStack.push(browserState); - return { browserId: browserState.id, contextId: indexedContext.id, pageId: browserState.page?.id }; + const page = browserState.page; + return { + browserId: browserState.id, + contextId: indexedContext.id, + pageId: page?.id, + videoPath: (await page?.p.video()?.path()) ?? null, + }; } public addBrowserServer = (browserServer: BrowserServer): void => { @@ -694,6 +699,7 @@ type AdoptedContext = { browserId: Uuid; contextId: Uuid; pageId: Uuid | undefined; + videoPath: string | null; }; type FailedPage = { @@ -962,7 +968,10 @@ export async function newContext( logger.info(`Trace file: ${traceFile}`); if (browserState.browser.browser === null) { - throw new Error('Trying to create a new context when a persistentContext is active'); + throw new Error( + `Cannot create a new context in ${browserState.browser.name}, because its context is persistent or ` + + 'adopted. Open a new browser first.', + ); } const indexedContext = await _createIndexedContext( await browserState.browser.browser.newContext(options), @@ -1003,7 +1012,7 @@ export async function newBrowser(request: Request_Browser, openBrowsers: Playwri options['timeout'] as number, options, ); - const browserState = openBrowsers.addBrowser(browserAndConfs); + const browserState = await openBrowsers.addBrowser(browserAndConfs); return stringResponse(browserState.id, 'Successfully created browser with options: ' + JSON.stringify(options)); } @@ -1044,7 +1053,7 @@ export async function newPersistentContext( browser: null, headless: options?.headless || true, } as BrowserAndConfs; - openBrowsers.addBrowser(browserAndConfs); + await openBrowsers.addBrowser(browserAndConfs); const browserState = await openBrowsers.getOrCreateActiveBrowser(null, timeout); if (browserState.newBrowser === true) { throw new Error('A new browser was created in error while trying to create a persistent context'); @@ -1073,7 +1082,7 @@ export async function connectToBrowser( const connectCDP = request.connectCDP; const timeout = request.timeout; const browserAndConfs = await _connectBrowser(browserType, url, connectCDP, timeout); - const browserState = openBrowsers.addBrowser(browserAndConfs); + const browserState = await openBrowsers.addBrowser(browserAndConfs); return stringResponse(browserState.id, 'Successfully connected to browser'); }