diff --git a/.github/workflows/create_test_report.yml b/.github/workflows/create_test_report.yml index 9410de4f62f8b..4ddaaa8bdfb05 100644 --- a/.github/workflows/create_test_report.yml +++ b/.github/workflows/create_test_report.yml @@ -4,6 +4,9 @@ on: workflows: ["tests 1", "tests 2", "tests others", "MCP"] types: - completed + +permissions: {} + jobs: merge-reports: permissions: @@ -13,6 +16,10 @@ jobs: contents: read # This is required for actions/checkout to succeed if: ${{ github.event.workflow_run.event == 'pull_request' || github.event.workflow_run.event == 'push' }} runs-on: ubuntu-latest + outputs: + pr_number: ${{ steps.pr.outputs.number }} + has_failures: ${{ steps.failures.outputs.has_failures }} + triage_allowed: ${{ steps.pr.outputs.triage_allowed }} env: MARKDOWN_OUTPUT_FILE: ${{ github.workspace }}/report.md steps: @@ -36,6 +43,15 @@ jobs: NODE_OPTIONS: --max-old-space-size=8192 WORKFLOW_RUN_CONCLUSION: ${{ github.event.workflow_run.conclusion }} + - name: Detect failures + id: failures + run: | + if [ -f "$MARKDOWN_OUTPUT_FILE" ] && grep -qE '\*\*[0-9]+ failed\*\*' "$MARKDOWN_OUTPUT_FILE"; then + echo "has_failures=true" >> "$GITHUB_OUTPUT" + else + echo "has_failures=false" >> "$GITHUB_OUTPUT" + fi + - name: Upload HTML report id: upload-report uses: actions/upload-artifact@v7 @@ -47,7 +63,7 @@ jobs: # The triggering workflow ran on a pull_request event, but workflow_run.pull_requests is # empty for PRs from forks, so resolve the number from the head ref instead. gh's head # filter wants "owner:branch" for forks but a bare branch for same-repo PRs. - - name: Resolve PR number + - name: Resolve PR number and triage allow-list if: ${{ github.event.workflow_run.event == 'pull_request' }} id: pr env: @@ -57,6 +73,14 @@ jobs: NUMBER=$(gh pr view --repo "${{ github.repository }}" "$HEAD_REF" --json number --jq '.number' 2>/dev/null || true) echo "number=$NUMBER" >> "$GITHUB_OUTPUT" + AUTHOR=$(gh pr view --repo "${{ github.repository }}" "$HEAD_REF" --json author --jq '.author.login' 2>/dev/null || true) + ALLOWED="github-actions[bot] pavelfeldman yury-s dgozman Skn0tt dcrousso" + TRIAGE_ALLOWED=false + for a in $ALLOWED; do + if [ "$a" = "$AUTHOR" ]; then TRIAGE_ALLOWED=true; break; fi + done + echo "triage_allowed=$TRIAGE_ALLOWED" >> "$GITHUB_OUTPUT" + - name: Post report comment to PR if: ${{ steps.pr.outputs.number }} uses: actions/github-script@v8 @@ -88,3 +112,98 @@ jobs: target_url: '${{ steps.upload-report.outputs.artifact-url }}', context: 'HTML Report (${{ github.event.workflow_run.name }})', }); + + triage: + needs: merge-reports + if: ${{ needs.merge-reports.outputs.has_failures == 'true' && needs.merge-reports.outputs.pr_number && needs.merge-reports.outputs.triage_allowed == 'true' }} + runs-on: ubuntu-latest + timeout-minutes: 20 + permissions: + copilot-requests: write + outputs: + has_draft: ${{ steps.triage.outputs.has_draft }} + pr_number: ${{ needs.merge-reports.outputs.pr_number }} + env: + PR_NUMBER: ${{ needs.merge-reports.outputs.pr_number }} + GH_TOKEN: ${{ github.token }} + steps: + - name: Checkout + uses: actions/checkout@v6 + + - name: Set up Node.js + uses: actions/setup-node@v6 + with: + node-version: "24" + + - name: Install Copilot CLI + run: npm install -g @github/copilot + + - name: Triage failures with Copilot CLI + id: triage + env: + COPILOT_GITHUB_TOKEN: ${{ github.token }} + run: | + mkdir -p output + PROMPT=$(cat <> "$GITHUB_OUTPUT" + else + echo "has_draft=false" >> "$GITHUB_OUTPUT" + fi + + - name: Add session transcript to job summary + if: ${{ always() }} + run: | + { + echo "## CI triage session transcript (PR #$PR_NUMBER)" + echo '' + cat "output/copilot-session.md" 2>/dev/null || echo "(no transcript)" + } >> "$GITHUB_STEP_SUMMARY" + + - name: Upload output + if: ${{ always() }} + uses: actions/upload-artifact@v4 + with: + name: ci-triage-${{ needs.merge-reports.outputs.pr_number }} + path: output/triage.md + if-no-files-found: warn + + post: + needs: triage + if: needs.triage.outputs.has_draft == 'true' + runs-on: ubuntu-latest + permissions: + issues: write + env: + PR_NUMBER: ${{ needs.triage.outputs.pr_number }} + GH_TOKEN: ${{ github.token }} + WORKFLOW_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + steps: + - name: Download triage output + uses: actions/download-artifact@v4 + with: + name: ci-triage-${{ needs.triage.outputs.pr_number }} + path: output + + - name: Post triage comment + run: | + printf '\n\nTriaged by the Playwright bot - [agent run](%s)\n\n' "$WORKFLOW_URL" >> output/triage.md + gh issue comment "$PR_NUMBER" --repo "${{ github.repository }}" --body-file output/triage.md + diff --git a/.github/workflows/pr-ci-triage.md b/.github/workflows/pr-ci-triage.md new file mode 100644 index 0000000000000..7ab6b783cb50b --- /dev/null +++ b/.github/workflows/pr-ci-triage.md @@ -0,0 +1,50 @@ +# PR CI Failure Verdict + +Come up with a verdict on a PR's failing CI tests: **are they likely caused by this PR, or +pre-existing flakes / infra noise?** +We want a verdict for the failures. If you can trivially come up with a fix, you can also propose it. +A merged report is posted on the PR by the `github-actions[bot]` comment, look at the most recent one. + +No failing tests (only flaky/interrupted) → nothing to triage. Group the same test failing across browsers as one story. + +**Hard rule for calling something a flake:** you must find the *same test* failing or flaking somewhere the PR can't be responsible for. Query the aggregated CI results via the `playwright-test-results` skill — look for the same `(project_name, file, test_title)` failing on other SHAs/PRs, or flipping verdict across runs. Divergence ("different tests, different browsers/OSes") and a plausible signature (timeout, element-not-found) are **not** enough on their own — flakes are often surprising and unrelated to the test's subject. No such evidence found → the failure is **uncertain**, not a flake. + +Some things to look into per failure (or group): + +- **Does the diff reach it?** The file under test, a feature it exercises, a shared helper it imports, or the product code path it asserts on. Network test vs. a docs-only PR → unrelated; click test vs. a PR rewriting input dispatch → suspicious. +- **Has this test flaked before?** Go look (see the hard rule above): use the `playwright-test-results` skill to check the same test's history across runs. Run the skill's `update` step first — the shared snapshot lags a few hours, so the freshest runs (including this PR's own) may not be in it yet. This is the only thing that proves a flake. +- **Flake/infra signature?** Timeouts, `Target closed`, browser launch/download errors, network hiccups, or the same test also flaking (passed on retry) in this report. A supporting hint, never proof on its own. +- **Browser/platform divergence.** One browser only, in code the PR didn't touch for that engine → leans flake; explainable on every browser → leans caused-by-PR. A weak hint only. + +## Verdict format + +Lead with a single traffic-light headline so the signal is readable at a glance, then put the per-group detail in a collapsible `
`: + +- 🔴 **Red** — at least one failure is caused by this PR. +- 🟡 **Yellow** — uncertain; you couldn't prove it either way. +- 🟢 **Green** — no real failures, or all confirmed pre-existing flakes / infra. The PR is clear. + +The overall colour is the worst of the per-group calls (any red → red, else any yellow → yellow, else green). + +Inside the collapsible, make an overall assessment of the PR's impact on CI failures, then segment by each failure/group as **caused by this PR** (which change, why), **pre-existing flake / infra** (cite where else the same test failed/flaked), or **uncertain** (what you'd need to be sure). + +Example (a red verdict — one real failure plus flakes): + +```markdown +## 🔴 One failure looks caused by this PR + +`page-request-gc.spec.ts:36` fails on Firefox because the fix here is Chromium-only. + +
+Details + +**Caused by this PR** + +- `[firefox-page] › page/page-request-gc.spec.ts:36 › should collect element retained by locator hit-target interceptor after detach` — the test the PR adds. The retention fix lives in `crPage.ts::requestGC`, which is Chromium-specific, so the element is still retained under Firefox and `weakRef.deref()` isn't `undefined`. The test has no browser gate, so it runs and fails on Firefox. Gate it to Chromium, e.g. `test.skip(browserName === 'firefox')`. + +**Pre-existing flake / infra** + +- `[chromium] › mcp/annotate.spec.ts:230 › should capture annotations via show --annotate` (+ `:496`) — pre-existing flake. Across the test-results DB this test flips verdict: **failed 7 of 54 runs (13%), passed the other 47**, on SHAs unrelated to this PR. This PR only touches the hit-target interceptor and Chromium `requestGC`, which the MCP annotate flow doesn't exercise. + +
+``` diff --git a/.github/workflows/triage.yml b/.github/workflows/triage.yml index 4208aba7b290f..62fdbf209555d 100644 --- a/.github/workflows/triage.yml +++ b/.github/workflows/triage.yml @@ -41,14 +41,12 @@ jobs: id: triage env: COPILOT_GITHUB_TOKEN: ${{ github.token }} - WORKFLOW_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} run: | mkdir -p output PROMPT=$(cat <\n' >> output/triage.md + printf '\n\nTriaged by the Playwright bot - [agent run](%s)\n\n' "$WORKFLOW_URL" >> output/triage.md gh issue comment "$ISSUE" --repo "${{ github.repository }}" --body-file output/triage.md diff --git a/docs/src/api/class-browsercontext.md b/docs/src/api/class-browsercontext.md index e4f2379cd9621..a5679cf5a96a3 100644 --- a/docs/src/api/class-browsercontext.md +++ b/docs/src/api/class-browsercontext.md @@ -1719,6 +1719,24 @@ Will throw an error if the page is closed before the [`event: BrowserContext.con * langs: python - returns: <[EventContextManager]<[ConsoleMessage]>> +**Usage** + +```python async +async with context.expect_console_message() as message_info: + await page.get_by_role("button").click() + +message = await message_info.value +print(message.text) +``` + +```python sync +with context.expect_console_message() as message_info: + page.get_by_role("button").click() + +message = message_info.value +print(message.text) +``` + ### param: BrowserContext.waitForConsoleMessage.action = %%-csharp-wait-for-event-action-%% * since: v1.34 @@ -1818,6 +1836,24 @@ Will throw an error if the context closes before new [Page] is created. * langs: python - returns: <[EventContextManager]<[Page]>> +**Usage** + +```python async +async with context.expect_page() as page_info: + await page.get_by_text("Open new tab").click() + +new_page = await page_info.value +print(await new_page.title()) +``` + +```python sync +with context.expect_page() as page_info: + page.get_by_text("Open new tab").click() + +new_page = page_info.value +print(new_page.title()) +``` + ### param: BrowserContext.waitForPage.action = %%-csharp-wait-for-event-action-%% * since: v1.12 diff --git a/docs/src/api/class-page.md b/docs/src/api/class-page.md index 7cf23b76637ff..913fc56185e9f 100644 --- a/docs/src/api/class-page.md +++ b/docs/src/api/class-page.md @@ -4616,6 +4616,24 @@ Will throw an error if the page is closed before the [`event: Page.console`] eve * langs: python - returns: <[EventContextManager]<[ConsoleMessage]>> +**Usage** + +```python async +async with page.expect_console_message() as message_info: + await page.get_by_role("button").click() + +message = await message_info.value +print(message.text) +``` + +```python sync +with page.expect_console_message() as message_info: + page.get_by_role("button").click() + +message = message_info.value +print(message.text) +``` + ### param: Page.waitForConsoleMessage.action = %%-csharp-wait-for-event-action-%% * since: v1.12 @@ -4752,6 +4770,24 @@ Will throw an error if the page is closed before the file chooser is opened. * langs: python - returns: <[EventContextManager]<[FileChooser]>> +**Usage** + +```python async +async with page.expect_file_chooser() as fc_info: + await page.get_by_text("Upload").click() + +file_chooser = await fc_info.value +await file_chooser.set_files("myfile.pdf") +``` + +```python sync +with page.expect_file_chooser() as fc_info: + page.get_by_text("Upload").click() + +file_chooser = fc_info.value +file_chooser.set_files("myfile.pdf") +``` + ### param: Page.waitForFileChooser.action = %%-csharp-wait-for-event-action-%% * since: v1.12 @@ -5111,6 +5147,24 @@ Will throw an error if the page is closed before the popup event is fired. * langs: python - returns: <[EventContextManager]<[Page]>> +**Usage** + +```python async +async with page.expect_popup() as popup_info: + await page.get_by_text("Open popup").click() + +popup = await popup_info.value +print(await popup.title()) +``` + +```python sync +with page.expect_popup() as popup_info: + page.get_by_text("Open popup").click() + +popup = popup_info.value +print(popup.title()) +``` + ### param: Page.waitForPopup.action = %%-csharp-wait-for-event-action-%% * since: v1.12 @@ -5253,6 +5307,24 @@ Will throw an error if the page is closed before the [`event: Page.requestFinish * langs: python - returns: <[EventContextManager]<[Request]>> +**Usage** + +```python async +async with page.expect_request_finished() as request_info: + await page.get_by_text("Trigger request").click() + +request = await request_info.value +print(request.url) +``` + +```python sync +with page.expect_request_finished() as request_info: + page.get_by_text("Trigger request").click() + +request = request_info.value +print(request.url) +``` + ### param: Page.waitForRequestFinished.action = %%-csharp-wait-for-event-action-%% * since: v1.12 @@ -5657,6 +5729,24 @@ Will throw an error if the page is closed before the WebSocket event is fired. * langs: python - returns: <[EventContextManager]<[WebSocket]>> +**Usage** + +```python async +async with page.expect_websocket() as ws_info: + await page.get_by_text("Connect").click() + +ws = await ws_info.value +print(ws.url) +``` + +```python sync +with page.expect_websocket() as ws_info: + page.get_by_text("Connect").click() + +ws = ws_info.value +print(ws.url) +``` + ### param: Page.waitForWebSocket.action = %%-csharp-wait-for-event-action-%% * since: v1.12 @@ -5689,6 +5779,24 @@ Will throw an error if the page is closed before the worker event is fired. * langs: python - returns: <[EventContextManager]<[Worker]>> +**Usage** + +```python async +async with page.expect_worker() as worker_info: + await page.get_by_text("Start worker").click() + +worker = await worker_info.value +print(worker.url) +``` + +```python sync +with page.expect_worker() as worker_info: + page.get_by_text("Start worker").click() + +worker = worker_info.value +print(worker.url) +``` + ### param: Page.waitForWorker.action = %%-csharp-wait-for-event-action-%% * since: v1.12 diff --git a/docs/src/api/class-websocket.md b/docs/src/api/class-websocket.md index 354de8f874447..d242d6e67fd13 100644 --- a/docs/src/api/class-websocket.md +++ b/docs/src/api/class-websocket.md @@ -77,6 +77,24 @@ value. Will throw an error if the webSocket is closed before the event is fired. * langs: python - returns: <[EventContextManager]> +**Usage** + +```python async +async with ws.expect_event("framereceived") as event_info: + await page.get_by_text("Send message").click() + +payload = await event_info.value +print(payload) +``` + +```python sync +with ws.expect_event("framereceived") as event_info: + page.get_by_text("Send message").click() + +payload = event_info.value +print(payload) +``` + ### param: WebSocket.waitForEvent.event * since: v1.8 - `event` <[string]> diff --git a/packages/isomorphic/codegen/actions.d.ts b/packages/isomorphic/codegen/actions.d.ts index 3f9c18cc5fcdb..e8092bdf0fa44 100644 --- a/packages/isomorphic/codegen/actions.d.ts +++ b/packages/isomorphic/codegen/actions.d.ts @@ -161,12 +161,10 @@ export type Signal = NavigationSignal | PopupSignal | DownloadSignal | DialogSig export type FrameDescription = { pageGuid: string; pageAlias: string; - framePath: string[]; }; export type ActionInContext = { frame: FrameDescription; - description?: string; action: Action; startTime: number; endTime?: number; diff --git a/packages/isomorphic/codegen/csharp.ts b/packages/isomorphic/codegen/csharp.ts index af9bb4ff2f08d..e62265936533b 100644 --- a/packages/isomorphic/codegen/csharp.ts +++ b/packages/isomorphic/codegen/csharp.ts @@ -72,8 +72,7 @@ export class CSharpLanguageGenerator implements LanguageGenerator { return formatter.format(); } - const locators = actionInContext.frame.framePath.map(selector => `.${this._asLocator(selector)}.ContentFrame`); - const subject = `${pageAlias}${locators.join('')}`; + const subject = pageAlias; const signals = toSignalMap(action); if (signals.dialog) { diff --git a/packages/isomorphic/codegen/java.ts b/packages/isomorphic/codegen/java.ts index cbaa0cfebcda7..99398a213788c 100644 --- a/packages/isomorphic/codegen/java.ts +++ b/packages/isomorphic/codegen/java.ts @@ -63,8 +63,7 @@ export class JavaLanguageGenerator implements LanguageGenerator { return formatter.format(); } - const locators = actionInContext.frame.framePath.map(selector => `.${this._asLocator(selector, false)}.contentFrame()`); - const subject = `${pageAlias}${locators.join('')}`; + const subject = pageAlias; const signals = toSignalMap(action); if (signals.dialog) { @@ -74,7 +73,7 @@ export class JavaLanguageGenerator implements LanguageGenerator { });`); } - let code = this._generateActionCall(subject, actionInContext, !!actionInContext.frame.framePath.length); + let code = this._generateActionCall(subject, actionInContext); if (signals.popup) { code = `Page ${signals.popup.popupAlias} = ${pageAlias}.waitForPopup(() -> { @@ -93,7 +92,7 @@ export class JavaLanguageGenerator implements LanguageGenerator { return formatter.format(); } - private _generateActionCall(subject: string, actionInContext: actions.ActionInContext, inFrameLocator: boolean): string { + private _generateActionCall(subject: string, actionInContext: actions.ActionInContext): string { const action = actionInContext.action; switch (action.name) { case 'openPage': @@ -106,46 +105,46 @@ export class JavaLanguageGenerator implements LanguageGenerator { method = 'dblclick'; const options = toClickOptionsForSourceCode(action); const optionsText = formatClickOptions(options); - return `${subject}.${this._asLocator(action.selector, inFrameLocator)}.${method}(${optionsText});`; + return `${subject}.${this._asLocator(action.selector)}.${method}(${optionsText});`; } case 'hover': { const optionsText = action.position ? `new Locator.HoverOptions().setPosition(${action.position.x}, ${action.position.y})` : ''; - return `${subject}.${this._asLocator(action.selector, inFrameLocator)}.hover(${optionsText});`; + return `${subject}.${this._asLocator(action.selector)}.hover(${optionsText});`; } case 'check': - return `${subject}.${this._asLocator(action.selector, inFrameLocator)}.check();`; + return `${subject}.${this._asLocator(action.selector)}.check();`; case 'uncheck': - return `${subject}.${this._asLocator(action.selector, inFrameLocator)}.uncheck();`; + return `${subject}.${this._asLocator(action.selector)}.uncheck();`; case 'fill': - return `${subject}.${this._asLocator(action.selector, inFrameLocator)}.fill(${quote(action.text)});`; + return `${subject}.${this._asLocator(action.selector)}.fill(${quote(action.text)});`; case 'setInputFiles': - return `${subject}.${this._asLocator(action.selector, inFrameLocator)}.setInputFiles(${formatPath(action.files.length === 1 ? action.files[0] : action.files)});`; + return `${subject}.${this._asLocator(action.selector)}.setInputFiles(${formatPath(action.files.length === 1 ? action.files[0] : action.files)});`; case 'press': { const modifiers = toKeyboardModifiers(action.modifiers); const shortcut = [...modifiers, action.key].join('+'); - return `${subject}.${this._asLocator(action.selector, inFrameLocator)}.press(${quote(shortcut)});`; + return `${subject}.${this._asLocator(action.selector)}.press(${quote(shortcut)});`; } case 'navigate': return `${subject}.navigate(${quote(action.url)});`; case 'select': - return `${subject}.${this._asLocator(action.selector, inFrameLocator)}.selectOption(${formatSelectOption(action.options.length === 1 ? action.options[0] : action.options)});`; + return `${subject}.${this._asLocator(action.selector)}.selectOption(${formatSelectOption(action.options.length === 1 ? action.options[0] : action.options)});`; case 'assertText': - return `assertThat(${subject}.${this._asLocator(action.selector, inFrameLocator)}).${action.substring ? 'containsText' : 'hasText'}(${quote(action.text)});`; + return `assertThat(${subject}.${this._asLocator(action.selector)}).${action.substring ? 'containsText' : 'hasText'}(${quote(action.text)});`; case 'assertChecked': - return `assertThat(${subject}.${this._asLocator(action.selector, inFrameLocator)})${action.checked ? '' : '.not()'}.isChecked();`; + return `assertThat(${subject}.${this._asLocator(action.selector)})${action.checked ? '' : '.not()'}.isChecked();`; case 'assertVisible': - return `assertThat(${subject}.${this._asLocator(action.selector, inFrameLocator)}).isVisible();`; + return `assertThat(${subject}.${this._asLocator(action.selector)}).isVisible();`; case 'assertValue': { const assertion = action.value ? `hasValue(${quote(action.value)})` : `isEmpty()`; - return `assertThat(${subject}.${this._asLocator(action.selector, inFrameLocator)}).${assertion};`; + return `assertThat(${subject}.${this._asLocator(action.selector)}).${assertion};`; } case 'assertSnapshot': - return `assertThat(${subject}.${this._asLocator(action.selector, inFrameLocator)}).matchesAriaSnapshot(${quote(action.ariaSnapshot)});`; + return `assertThat(${subject}.${this._asLocator(action.selector)}).matchesAriaSnapshot(${quote(action.ariaSnapshot)});`; } } - private _asLocator(selector: string, inFrameLocator: boolean) { - return asLocator('java', selector, inFrameLocator); + private _asLocator(selector: string) { + return asLocator('java', selector); } generateHeader(options: LanguageGeneratorOptions): string { diff --git a/packages/isomorphic/codegen/javascript.ts b/packages/isomorphic/codegen/javascript.ts index 8972143a6729a..a0dc4e7ca3f66 100644 --- a/packages/isomorphic/codegen/javascript.ts +++ b/packages/isomorphic/codegen/javascript.ts @@ -51,8 +51,7 @@ export class JavaScriptLanguageGenerator implements LanguageGenerator { return formatter.format(); } - const locators = actionInContext.frame.framePath.map(selector => `.${this._asLocator(selector)}.contentFrame()`); - const subject = `${pageAlias}${locators.join('')}`; + const subject = pageAlias; const signals = toSignalMap(action); if (signals.dialog) { @@ -67,7 +66,7 @@ export class JavaScriptLanguageGenerator implements LanguageGenerator { if (signals.download) formatter.add(`const download${signals.download.downloadAlias}Promise = ${pageAlias}.waitForEvent('download');`); - formatter.add(wrapWithStep(actionInContext.description, this._generateActionCall(subject, actionInContext))); + formatter.add(this._generateActionCall(subject, actionInContext)); if (signals.popup) formatter.add(`const ${signals.popup.popupAlias} = await ${signals.popup.popupAlias}Promise;`); @@ -256,12 +255,6 @@ function quote(text: string) { return escapeWithQuotes(text, '\''); } -function wrapWithStep(description: string | undefined, body: string) { - return description ? `await test.step(\`${description}\`, async () => { -${body} -});` : body; -} - export function quoteMultiline(text: string, indent = ' ') { const escape = (text: string) => text.replace(/\\/g, '\\\\') .replace(/`/g, '\\`') diff --git a/packages/isomorphic/codegen/python.ts b/packages/isomorphic/codegen/python.ts index 833bdbda08f6a..a16a09556119f 100644 --- a/packages/isomorphic/codegen/python.ts +++ b/packages/isomorphic/codegen/python.ts @@ -58,8 +58,7 @@ export class PythonLanguageGenerator implements LanguageGenerator { return formatter.format(); } - const locators = actionInContext.frame.framePath.map(selector => `.${this._asLocator(selector)}.content_frame`); - const subject = `${pageAlias}${locators.join('')}`; + const subject = pageAlias; const signals = toSignalMap(action); if (signals.dialog) diff --git a/packages/isomorphic/locatorGenerators.ts b/packages/isomorphic/locatorGenerators.ts index 00d1e23f0facd..744b7a69cc380 100644 --- a/packages/isomorphic/locatorGenerators.ts +++ b/packages/isomorphic/locatorGenerators.ts @@ -734,8 +734,13 @@ export class JsonlLocatorFactory implements LocatorFactory { chainLocators(locators: string[]): string { const objects = locators.map(l => JSON.parse(l)); - for (let i = 0; i < objects.length - 1; ++i) - objects[i].next = objects[i + 1]; + for (let i = 0; i < objects.length - 1; ++i) { + // A locator may already be a chain, e.g. `contentFrame()` produces one. Append to its tail. + let tail = objects[i]; + while (tail.next) + tail = tail.next; + tail.next = objects[i + 1]; + } return JSON.stringify(objects[0]); } } diff --git a/packages/playwright-core/browsers.json b/packages/playwright-core/browsers.json index c5e49309c3e4b..409c49270e801 100644 --- a/packages/playwright-core/browsers.json +++ b/packages/playwright-core/browsers.json @@ -45,7 +45,7 @@ }, { "name": "webkit", - "revision": "2327", + "revision": "2328", "installByDefault": true, "revisionOverrides": { "mac14": "2251", diff --git a/packages/playwright-core/src/server/frames.ts b/packages/playwright-core/src/server/frames.ts index 3184bce5f9ee6..44de2e8249066 100644 --- a/packages/playwright-core/src/server/frames.ts +++ b/packages/playwright-core/src/server/frames.ts @@ -37,7 +37,7 @@ import { SdkObject } from './instrumentation'; import * as js from './javascript'; import * as network from './network'; import { Page, ariaSnapshotForFrame } from './page'; -import { isAbortError, nullProgress, ProgressController } from './progress'; +import { isAbortError, nullProgress, ProgressController, raceUncancellableOperationWithCleanup } from './progress'; import * as types from './types'; import { isSessionClosedError } from './protocolError'; @@ -1632,7 +1632,7 @@ export class Frame extends SdkObject { return this.retryWithProgressAndTimeouts(progress, [100], async () => { const context = world === 'main' ? await progress.race(this.mainContext()) : await progress.race(this.utilityContext()); const injectedScript = await progress.race(context.injectedScript()); - const handle = await progress.race(injectedScript.evaluateHandle((injected, { expression, isFunction, polling, arg }) => { + const handle = await raceUncancellableOperationWithCleanup(progress, () => injectedScript.evaluateHandle((injected, { expression, isFunction, polling, arg }) => { let evaledExpression: any; const predicate = (): R => { // NOTE: make sure to use `globalThis.eval` instead of `self.eval` due to a bug with sandbox isolation @@ -1679,7 +1679,10 @@ export class Frame extends SdkObject { next(); return { result, abort: () => aborted = true }; - }, { expression, isFunction, polling: options.pollingInterval, arg })); + }, { expression, isFunction, polling: options.pollingInterval, arg }), async handle => { + await handle.evaluate(h => h.abort()).catch(() => {}); + handle.dispose(); + }); try { return await progress.race(handle.evaluateHandle(h => h.result)); } catch (error) { diff --git a/packages/playwright-core/src/server/recorder.ts b/packages/playwright-core/src/server/recorder.ts index c91775849a9be..a6537f64a34eb 100644 --- a/packages/playwright-core/src/server/recorder.ts +++ b/packages/playwright-core/src/server/recorder.ts @@ -552,15 +552,6 @@ export class Recorder extends EventEmitter implements Instrume return { pageGuid: page.guid, pageAlias: this._pageAliases.get(page)!, - framePath: [], - }; - } - - private async _describeFrame(progress: Progress, frame: Frame): Promise { - return { - pageGuid: frame._page.guid, - pageAlias: this._pageAliases.get(frame._page)!, - framePath: await generateFrameSelector(progress, frame), }; } @@ -568,19 +559,24 @@ export class Recorder extends EventEmitter implements Instrume return this._params.testIdAttributeName || this._context.selectors().testIdAttributeName() || 'data-testid'; } - private async _createActionInContext(progress: Progress, frame: Frame, action: actions.Action): Promise { - const frameDescription = await this._describeFrame(progress, frame); + private async _appendContextToAction(progress: Progress, frame: Frame, action: actions.Action): Promise { + const framePath = await generateFrameSelector(progress, frame); + if (framePath.length) { + if ('selector' in action) + action.selector = buildFullSelector(framePath, action.selector); + if (action.preconditionSelector) + action.preconditionSelector = buildFullSelector(framePath, action.preconditionSelector); + } const actionInContext: actions.ActionInContext = { - frame: frameDescription, + frame: this._describeMainFrame(frame._page), action, - description: undefined, startTime: monotonicTime(), }; return actionInContext; } private async _performAction(progress: Progress, frame: Frame, action: actions.PerformOnRecordAction) { - const actionInContext = await this._createActionInContext(progress, frame, action); + const actionInContext = await this._appendContextToAction(progress, frame, action); this._signalProcessor.addAction(actionInContext); try { if (actionInContext.action.name !== 'openPage' && actionInContext.action.name !== 'closePage') @@ -591,7 +587,7 @@ export class Recorder extends EventEmitter implements Instrume } private async _recordAction(progress: Progress, frame: Frame, action: actions.Action) { - const actionInContext = await this._createActionInContext(progress, frame, action); + const actionInContext = await this._appendContextToAction(progress, frame, action); this._signalProcessor.addAction(actionInContext); } diff --git a/packages/playwright-core/src/server/recorder/recorderRunner.ts b/packages/playwright-core/src/server/recorder/recorderRunner.ts index dc0dc804a9713..aef46f7b3b344 100644 --- a/packages/playwright-core/src/server/recorder/recorderRunner.ts +++ b/packages/playwright-core/src/server/recorder/recorderRunner.ts @@ -15,7 +15,7 @@ */ import { toKeyboardModifiers } from '@isomorphic/codegen/language'; -import { buildFullSelector, mainFrameForAction } from './recorderUtils'; +import { mainFrameForAction } from './recorderUtils'; import { Progress } from '../progress'; import type { Page } from '../page'; @@ -44,7 +44,7 @@ async function performActionImpl(progress: Progress, mainFrame: Frame, actionInC return; } - const selector = buildFullSelector(actionInContext.frame.framePath, action.selector); + const selector = action.selector; if (action.name === 'click') { const options = toClickOptions(action); diff --git a/packages/playwright-core/src/server/recorder/recorderSignalProcessor.ts b/packages/playwright-core/src/server/recorder/recorderSignalProcessor.ts index 634b65b206388..7a0f714a84cdb 100644 --- a/packages/playwright-core/src/server/recorder/recorderSignalProcessor.ts +++ b/packages/playwright-core/src/server/recorder/recorderSignalProcessor.ts @@ -16,8 +16,6 @@ import { monotonicTime } from '@isomorphic/time'; import { isUnderTest } from '@utils/debug'; -import { generateFrameSelector } from './recorderUtils'; -import { nullProgress } from '../progress'; import type { Signal } from '@isomorphic/codegen/actions'; import type { Frame } from '../frames'; @@ -60,7 +58,6 @@ export class RecorderSignalProcessor { frame: { pageGuid: frame._page.guid, pageAlias, - framePath: [], }, action: { name: 'navigate', @@ -74,17 +71,13 @@ export class RecorderSignalProcessor { return; } - generateFrameSelector(nullProgress, frame).then(framePath => { - const signalInContext: actions.SignalInContext = { - frame: { - pageGuid: frame._page.guid, - pageAlias, - framePath, - }, - signal, - timestamp, - }; - this._delegate.addSignal(signalInContext); + this._delegate.addSignal({ + frame: { + pageGuid: frame._page.guid, + pageAlias, + }, + signal, + timestamp, }); } } diff --git a/packages/playwright-core/src/server/recorder/recorderUtils.ts b/packages/playwright-core/src/server/recorder/recorderUtils.ts index e874a5bcdc66e..99f4959662215 100644 --- a/packages/playwright-core/src/server/recorder/recorderUtils.ts +++ b/packages/playwright-core/src/server/recorder/recorderUtils.ts @@ -64,7 +64,7 @@ export function mainFrameForAction(pageAliases: Map, actionInConte } function isSameAction(a: actions.ActionInContext, b: actions.ActionInContext): boolean { - return a.action.name === b.action.name && a.frame.pageAlias === b.frame.pageAlias && a.frame.framePath.join('|') === b.frame.framePath.join('|'); + return a.action.name === b.action.name && a.frame.pageAlias === b.frame.pageAlias; } function isSameSelector(action: actions.ActionInContext, lastAction: actions.ActionInContext): boolean { diff --git a/tests/library/inspector/cli-codegen-3.spec.ts b/tests/library/inspector/cli-codegen-3.spec.ts index ac48a6aeebaef..342a895417cba 100644 --- a/tests/library/inspector/cli-codegen-3.spec.ts +++ b/tests/library/inspector/cli-codegen-3.spec.ts @@ -61,7 +61,6 @@ await page.GetByRole(AriaRole.Button, new() { Name = "Submit" }).First.ClickAsyn locator: { body: 'button', kind: 'role', options: { exact: false, attrs: [], name: 'Submit' }, next: { body: '', kind: 'first', options: {} } }, modifiers: 0, signals: [], - framePath: [], pageAlias: 'page', pageGuid: expect.any(String), }); @@ -131,13 +130,18 @@ await page.Locator("#frame1").ContentFrame.GetByText("Hello1").ClickAsync();`); const clickAction = sources.get('JSON')!.actions.map(l => JSON.parse(l)).find(a => a.name === 'click'); expect.soft(clickAction).toEqual({ name: 'click', - selector: 'internal:text="Hello1"i', + selector: '#frame1 >> internal:control=enter-frame >> internal:text="Hello1"i', button: 'left', clickCount: 1, - locator: { body: 'Hello1', kind: 'text', options: { exact: false } }, + locator: { + body: '#frame1', kind: 'default', options: {}, + next: { + body: '', kind: 'frame', options: {}, + next: { body: 'Hello1', kind: 'text', options: { exact: false } }, + }, + }, modifiers: 0, signals: [], - framePath: ['#frame1'], pageAlias: 'page', pageGuid: expect.any(String), }); @@ -170,13 +174,24 @@ await page.Locator("#frame1").ContentFrame.Locator("iframe").ContentFrame.GetByT const clickAction = sources.get('JSON')!.actions.map(l => JSON.parse(l)).find(a => a.name === 'click'); expect.soft(clickAction).toEqual({ name: 'click', - selector: 'internal:text="Hello2"i', + selector: '#frame1 >> internal:control=enter-frame >> iframe >> internal:control=enter-frame >> internal:text="Hello2"i', button: 'left', clickCount: 1, - locator: { body: 'Hello2', kind: 'text', options: { exact: false } }, + locator: { + body: '#frame1', kind: 'default', options: {}, + next: { + body: '', kind: 'frame', options: {}, + next: { + body: 'iframe', kind: 'default', options: {}, + next: { + body: '', kind: 'frame', options: {}, + next: { body: 'Hello2', kind: 'text', options: { exact: false } }, + }, + }, + }, + }, modifiers: 0, signals: [], - framePath: ['#frame1', 'iframe'], pageAlias: 'page', pageGuid: expect.any(String), }); @@ -209,13 +224,33 @@ await page.Locator("#frame1").ContentFrame.Locator("iframe").ContentFrame.Locato const clickAction = sources.get('JSON')!.actions.map(l => JSON.parse(l)).find(a => a.name === 'click'); expect.soft(clickAction).toEqual({ name: 'click', - selector: 'internal:text="HelloNameAnonymous"i', + selector: '#frame1 >> internal:control=enter-frame >> iframe >> internal:control=enter-frame >> iframe >> nth=2 >> internal:control=enter-frame >> internal:text="HelloNameAnonymous"i', button: 'left', clickCount: 1, - locator: { body: 'HelloNameAnonymous', kind: 'text', options: { exact: false } }, + locator: { + body: '#frame1', kind: 'default', options: {}, + next: { + body: '', kind: 'frame', options: {}, + next: { + body: 'iframe', kind: 'default', options: {}, + next: { + body: '', kind: 'frame', options: {}, + next: { + body: 'iframe', kind: 'default', options: {}, + next: { + body: '2', kind: 'nth', options: {}, + next: { + body: '', kind: 'frame', options: {}, + next: { body: 'HelloNameAnonymous', kind: 'text', options: { exact: false } }, + }, + }, + }, + }, + }, + }, + }, modifiers: 0, signals: [], - framePath: ['#frame1', 'iframe', 'iframe >> nth=2'], pageAlias: 'page', pageGuid: expect.any(String), });