diff --git a/AGENTS.md b/AGENTS.md index 7c8fc56..fe54a52 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -21,7 +21,7 @@ Background, research and the initial design ideas are in [dev-docs/background.md Until a Browser release contains our changes, `robotframework-browser` comes from a local clone at `../robotframework-browser` (editable path source in the root `pyproject.toml`). -- The `adoptContext` hook (MarketSquare/robotframework-browser#5318) and the public methods `adopted_context_options` and `register_adopted_context` (MarketSquare/robotframework-browser#5326) are merged into Browser's `main`, but not released yet. +- The `adoptContext` hook (MarketSquare/robotframework-browser#5318) and the public method `adopted_context_options` (MarketSquare/robotframework-browser#5326) are merged into Browser's `main`, but not released yet. Browser registers an adopted context itself, with trace groups and the video in the log (MarketSquare/robotframework-browser#5372), and `adoptContext` takes the `activePage` to make active. - The clone's `main` tracks `upstream/main`, and CI checks out `main` of `MarketSquare/robotframework-browser`. - The clone's `origin` is the fork `d-biehl/robotframework-browser`, and `upstream` is `MarketSquare/robotframework-browser`. Push to the fork over SSH (`git@github.com:d-biehl/robotframework-browser.git`); the HTTPS `origin` has no credentials. diff --git a/packages/electron/src/Electron/__init__.py b/packages/electron/src/Electron/__init__.py index 4d18359..6f92802 100644 --- a/packages/electron/src/Electron/__init__.py +++ b/packages/electron/src/Electron/__init__.py @@ -267,24 +267,8 @@ def start_electron_application( if recording["tracing"]: trace_file = recording["tracing"] logger.info(f"The trace of {adopted['browserId']} is saved to {trace_file} when the application closes") - video_path = self.register_adopted_context(adopted["contextId"], recording, adopted.get("videoPath")) - page_id = self._activate_window_page(adopted["browserId"], adopted.get("pageId", "")) - return adopted["browserId"], NewPageDetails(page_id=page_id, video_path=video_path) - - def _activate_window_page(self, browser_id: str, page_id: str) -> str: - """Makes the first window's own page the active page and returns its id. - - Browser makes the page that Playwright saw first the active page. When the application shows - a WebContentsView or a from the start, that can be one of them. - """ - if not page_id or self.call_js_keyword("robotframeworkElectronIsWindowPage", browserId=browser_id): - return page_id - for candidate in self.get_page_ids(SelectionType.ALL, SelectionType.ACTIVE, SelectionType.ACTIVE): - self.switch_page(candidate) - if self.call_js_keyword("robotframeworkElectronIsWindowPage", browserId=browser_id): - return str(candidate) - self.switch_page(page_id) - return page_id + page_details = NewPageDetails(page_id=adopted.get("pageId", ""), video_path=adopted.get("videoPath") or "") + return adopted["browserId"], page_details @keyword def close_electron_application(self, application: SelectionType | str = SelectionType.CURRENT) -> None: diff --git a/packages/electron/src/Electron/electron.js b/packages/electron/src/Electron/electron.js index 357dd34..e115998 100644 --- a/packages/electron/src/Electron/electron.js +++ b/packages/electron/src/Electron/electron.js @@ -5,7 +5,7 @@ // The started applications by browser id, until Browser closes them. const applications = new Map(); -async function robotframeworkElectronLaunch(executablePath, args, env, cwd, timeout, recordVideo, recordHar, tracing, tracesDir, launchOptions, defaultTimeout, playwright, adoptContext) { +async function robotframeworkElectronLaunch(executablePath, args, env, cwd, timeout, recordVideo, recordHar, tracing, tracesDir, launchOptions, playwright, adoptContext) { // Browser releases without the hook pass a placeholder instead of a function. if (typeof adoptContext !== 'function') { throw new Error('The Browser library has no adoptContext hook, which the Electron library needs to take over the application.'); @@ -27,16 +27,22 @@ async function robotframeworkElectronLaunch(executablePath, args, env, cwd, time }); try { const window = await firstWindowPage(app, await app.firstWindow({ timeout }), timeout); - // The library timeout, as the contexts that Browser creates get it. - app.context().setDefaultTimeout(defaultTimeout); let browserId; const onClose = async () => { applications.delete(browserId); }; - const adopted = await adoptContext(app.context(), { name: 'electron', tracing: tracing || undefined, contextOptions, onClose }); + // A window that closed during the start is no page of the context any more. + const activePage = window.isClosed() ? undefined : window; + const adopted = await adoptContext(app.context(), { + name: 'electron', + tracing: tracing || undefined, + contextOptions, + onClose, + activePage, + }); browserId = adopted.browserId; applications.set(browserId, app); - return { ...adopted, videoPath: recordVideo ? await window.video()?.path() : null }; + return adopted; } catch (error) { await app.close().catch(() => app.process().kill()); throw error; @@ -77,13 +83,6 @@ async function firstWindowPage(app, first, timeout) { } } -// Whether the active `page` is a window's own page of the application with the browser id `browserId`. -async function robotframeworkElectronIsWindowPage(browserId, page) { - const application = applications.get(browserId); - if (!application || !page || page.context() !== application.context()) return false; - return isWindowPage(application, page); -} - // Runs the function text `script` in the main process of the application with // the browser id `browserId`, with the electron module, `arg`, and the // BrowserWindow of the active `page` if the page belongs to the application. @@ -108,4 +107,3 @@ async function robotframeworkElectronEvaluate(browserId, script, arg, page) { exports.__esModule = true; exports.robotframeworkElectronLaunch = robotframeworkElectronLaunch; exports.robotframeworkElectronEvaluate = robotframeworkElectronEvaluate; -exports.robotframeworkElectronIsWindowPage = robotframeworkElectronIsWindowPage; diff --git a/packages/electron/tests/test_electron_js.py b/packages/electron/tests/test_electron_js.py index a2c1768..bdd1772 100644 --- a/packages/electron/tests/test_electron_js.py +++ b/packages/electron/tests/test_electron_js.py @@ -35,10 +35,7 @@ const scenario = process.argv[3]; const calls = []; const options = {}; -const context = { - setDefaultTimeout: (timeout) => calls.push(`setDefaultTimeout ${timeout}`), - newCDPSession: async (page) => cdpSession(page), -}; +const context = { newCDPSession: async (page) => cdpSession(page) }; const windowPage = fakePage('window', context); const viewPage = fakePage('view', context); const app = { @@ -70,13 +67,14 @@ async function adoptContext(context, adoptOptions) { calls.push('adoptContext'); options.adopt = adoptOptions.contextOptions; + options.activePage = adoptOptions.activePage?.name ?? null; if (scenario === 'adopt-fails' || scenario === 'close-fails') throw new Error('adopt failed'); - return { browserId: 'browser', contextId: 'context', pageId: 'page' }; + return { browserId: 'browser', contextId: 'context', pageId: 'page', videoPath: null }; } const hook = scenario === 'no-hook' ? 'RESERVED' : adoptContext; const launchOptions = scenario === 'options' ? { colorScheme: 'dark', locale: 'de-DE', bypassCSP: true } : {}; const recordVideo = scenario === 'view-first' || scenario === 'first-closed' ? {} : null; -const launchArguments = ['/electron', [], {}, '/cwd', 1000, recordVideo, null, null, '/traces', launchOptions, 7000]; +const launchArguments = ['/electron', [], {}, '/cwd', 1000, recordVideo, null, null, '/traces', launchOptions]; robotframeworkElectronLaunch(...launchArguments, playwright, hook) .then((result) => ({ result }), (error) => ({ error: error.message })) .then((outcome) => console.log(JSON.stringify({ ...outcome, calls, options }))); @@ -84,12 +82,10 @@ EVALUATE_HARNESS = """ -const { robotframeworkElectronLaunch, robotframeworkElectronEvaluate, robotframeworkElectronIsWindowPage } = require( - process.argv[2] -); +const { robotframeworkElectronLaunch, robotframeworkElectronEvaluate } = require(process.argv[2]); const scenario = process.argv[3]; const calls = []; -const context = { setDefaultTimeout: () => {}, newCDPSession: async (page) => cdpSession(page) }; +const context = { newCDPSession: async (page) => cdpSession(page) }; const electron = { ...mainProcess, app: { getName: () => 'fixture' } }; const windowPage = fakePage('window', context); const app = { @@ -111,12 +107,9 @@ const page = scenario === 'other-page' ? { context: () => ({}) } : { context: () => context }; const script = '(electron, arg, window) => [electron.app.getName(), arg, window && window.title]'; (async () => { - const launchArguments = ['/electron', [], {}, '/cwd', 1000, null, null, null, '/traces', {}, 7000]; + const launchArguments = ['/electron', [], {}, '/cwd', 1000, null, null, null, '/traces', {}]; await robotframeworkElectronLaunch(...launchArguments, playwright, adoptContext); if (scenario === 'closed') await onClose(); - if (scenario === 'is-window') return robotframeworkElectronIsWindowPage('browser=app', windowPage); - if (scenario === 'is-view') return robotframeworkElectronIsWindowPage('browser=app', fakePage('view', context)); - if (scenario === 'is-other') return robotframeworkElectronIsWindowPage('browser=app', fakePage('window', {})); const browserId = scenario === 'unknown' ? 'browser=other' : 'browser=app'; return robotframeworkElectronEvaluate(browserId, script, 42, page); })() @@ -139,7 +132,7 @@ def test_returns_the_adopted_context(tmp_path): outcome = run(tmp_path, "success") assert outcome["result"] == {"browserId": "browser", "contextId": "context", "pageId": "page", "videoPath": None} - assert outcome["calls"] == ["launch", "firstWindow", "setDefaultTimeout 7000", "adoptContext"] + assert outcome["calls"] == ["launch", "firstWindow", "adoptContext"] def test_application_decides_the_color_scheme_by_default(tmp_path): @@ -174,12 +167,8 @@ def test_starts_nothing_without_the_hook(tmp_path): ("scenario", "error", "calls"), [ ("no-window", "no window", ["launch", "firstWindow", "close"]), - ("adopt-fails", "adopt failed", ["launch", "firstWindow", "setDefaultTimeout 7000", "adoptContext", "close"]), - ( - "close-fails", - "adopt failed", - ["launch", "firstWindow", "setDefaultTimeout 7000", "adoptContext", "close", "kill"], - ), + ("adopt-fails", "adopt failed", ["launch", "firstWindow", "adoptContext", "close"]), + ("close-fails", "adopt failed", ["launch", "firstWindow", "adoptContext", "close", "kill"]), ], ) def test_closes_the_application_when_the_start_fails(tmp_path, scenario, error, calls): @@ -210,15 +199,8 @@ def test_evaluating_fails_for_a_browser_that_is_no_open_application(tmp_path, sc assert outcome["error"] == f"Browser '{browser_id}' is not an application started by 'Start Electron Application'." -@pytest.mark.parametrize(("scenario", "video"), [("view-first", "window.webm"), ("first-closed", "closed.webm")]) -def test_takes_the_window_page_as_first_window(tmp_path, scenario, video): +@pytest.mark.parametrize(("scenario", "active_page"), [("view-first", "window"), ("first-closed", None)]) +def test_makes_the_window_page_the_active_page(tmp_path, scenario, active_page): outcome = run(tmp_path, scenario) - assert outcome["result"]["videoPath"] == video - - -@pytest.mark.parametrize(("scenario", "is_window"), [("is-window", True), ("is-view", False), ("is-other", False)]) -def test_tells_whether_a_page_is_a_window_page_of_the_application(tmp_path, scenario, is_window): - outcome = run(tmp_path, scenario, EVALUATE_HARNESS) - - assert outcome["result"] is is_window + assert outcome["options"]["activePage"] == active_page diff --git a/packages/electron/tests/test_library.py b/packages/electron/tests/test_library.py index c5a5668..0e44080 100644 --- a/packages/electron/tests/test_library.py +++ b/packages/electron/tests/test_library.py @@ -128,7 +128,6 @@ def test_recording_options_come_from_browser(fake_launch, tmp_path): "showActions": show_actions, } assert calls[0]["tracing"] == str(tmp_path / "browser" / "traces") - assert electron.tracing_contexts == ["context"] def test_har_options_come_from_browser(fake_launch, tmp_path): @@ -210,45 +209,14 @@ def test_evaluates_in_the_main_process_of_a_given_application(fake_launch): assert calls[0] == {"browserId": "browser=other", "script": "(electron, arg) => arg", "arg": {"answer": 42}} -@pytest.fixture -def pages(fake_launch, monkeypatch): - """Fakes an application whose active page after the start is a view, and records the page switches.""" +def test_returns_the_page_and_the_video_that_browser_reports(fake_launch, monkeypatch, tmp_path): electron, _ = fake_launch - active = ["page=view"] - switches: list[str] = [] - - def call_js_keyword(name, **arguments): - if name == "robotframeworkElectronIsWindowPage": - return active[0] == "page=window" - return {"browserId": "browser", "contextId": "context", "pageId": active[0]} - - def switch_page(page_id, *args): - switches.append(page_id) - active[0] = page_id - - monkeypatch.setattr(electron, "call_js_keyword", call_js_keyword) - monkeypatch.setattr(electron, "get_page_ids", lambda *args: ["page=guest", "page=window", "page=view"]) - monkeypatch.setattr(electron, "switch_page", switch_page) - return electron, active, switches + adopted = {"browserId": "browser", "contextId": "context", "pageId": "page=window", "videoPath": "window.webm"} + monkeypatch.setattr(electron, "call_js_keyword", lambda name, **arguments: adopted) + application, page = electron.start_electron_application(tmp_path / "bin" / "electron") -def test_switches_to_the_window_page_when_a_view_is_active(pages, tmp_path): - electron, _, switches = pages - - _, page = electron.start_electron_application(tmp_path / "bin" / "electron") - - assert page["page_id"] == "page=window" - assert switches == ["page=guest", "page=window"] - - -def test_keeps_the_active_page_when_it_is_the_window(pages, tmp_path): - electron, active, switches = pages - active[0] = "page=window" - - _, page = electron.start_electron_application(tmp_path / "bin" / "electron") - - assert page["page_id"] == "page=window" - assert switches == [] + assert (application, page["page_id"], page["video_path"]) == ("browser", "page=window", "window.webm") @pytest.fixture