E2E/Playwright: Clickable link in plugin tooltip#36070
E2E/Playwright: Clickable link in plugin tooltip#36070saturninoabril wants to merge 1 commit intomasterfrom
Conversation
mm-cloud-bot
added
the
release-note-none
📝 WalkthroughWalkthroughThe PR updates the demo plugin artifact version from 0.11.0 to 0.11.1 across multiple e2e test files and adds new Playwright end-to-end tests for plugin link tooltip functionality, including interaction behavior and localization verification. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@e2e-tests/playwright/specs/functional/plugins/demo_plugin/webapp/plugin_link_tooltip.spec.ts`:
- Around line 106-112: The test currently asserts popup.url() immediately after
capturing the popup (popupPromise) which can be "about:blank" and flaky; after
awaiting the popup (variable popup) from channelsPage.page.waitForEvent('popup')
you should wait for navigation completion by calling await popup.waitForURL()
(or use await expect(popup).toHaveURL(...)) before asserting the destination URL
(the titleLink click remains the trigger). Update the code around popupPromise,
titleLink.click(), and the popup assertion to first await popup.waitForURL() and
then assert the URL contains 'example.com'.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b4a1b6e3-3641-4cb5-884b-173fec1fd16d
📒 Files selected for processing (5)
e2e-tests/cypress/tests/utils/plugins.jse2e-tests/playwright/specs/functional/plugins/demo_plugin/helpers.tse2e-tests/playwright/specs/functional/plugins/demo_plugin/webapp/plugin_link_tooltip.spec.tse2e-tests/playwright/specs/functional/plugins/demo_plugin_installation.spec.tswebapp/channels/src/components/plugin_link_tooltip/index.test.tsx
| const popupPromise = channelsPage.page.waitForEvent('popup'); | ||
| await titleLink.click(); | ||
| const popup = await popupPromise; | ||
|
|
||
| // * Verify a new tab opens with the expected URL | ||
| expect(popup.url()).toContain('example.com'); | ||
| await popup.close(); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Playwright, can popup.url() still be "about:blank" immediately after page.waitForEvent('popup')? What is the recommended way to assert popup destination URL reliably?
💡 Result:
Yes, popup.url can be "about:blank" immediately after page.waitForEvent('popup'), because the popup event fires as soon as the new page is created (often starting at about:blank), before any navigation to the destination URL completes. This is commonly seen with window.open or target="_blank" links that navigate after opening, and is reported across browsers, especially in headless mode for certain cases like PDFs. The recommended way to reliably assert the popup destination URL is: 1. Await the popup promise from page.waitForEvent('popup'). 2. Call await popup.waitForLoadState('domcontentloaded') or await popup.waitForURL(expectedUrl) on the popup page to wait for navigation and loading to complete. 3. Then assert with expect(popup).toHaveURL(expectedUrl). Official Playwright documentation explicitly shows this pattern: const popupPromise = page.waitForEvent('popup'); await page.getByRole('button').click; // triggers popup const popup = await popupPromise; await popup.waitForLoadState('domcontentloaded'); // Wait until ready expect(popup).toHaveURL('/login'); // or whatever expected Alternative: Use popup.waitForURL directly after getting the popup, or Promise.all([page.waitForEvent('popup'), page.click(selector)]) for coordination. Avoid asserting URL immediately after waitForEvent('popup') without waiting for load/navigation, as it will often be "about:blank" and cause flaky tests.
Citations:
- 1: https://stackoverflow.com/questions/78494671/playwright-unable-to-capture-url-address-of-a-new-tab-in-headless-mode
- 2: https://playwright.dev/docs/next/api/class-page
- 3: https://stackoverflow.com/questions/69575124/get-popup-url-after-a-new-request-from-javascript-on-playwright
- 4: https://playwright.dev/docs/next/pages
- 5: https://playwright.dev/docs/pages
Wait for popup navigation before asserting destination URL.
popup.url() can return "about:blank" immediately after waitForEvent('popup'), causing flaky tests. Use await popup.waitForURL() to wait for navigation completion, then assert with expect(popup).toHaveURL().
Proposed reliability fix
const popupPromise = channelsPage.page.waitForEvent('popup');
await titleLink.click();
const popup = await popupPromise;
- // * Verify a new tab opens with the expected URL
- expect(popup.url()).toContain('example.com');
+ // * Verify a new tab opens with the expected URL
+ await popup.waitForURL(/example\.com/);
+ await expect(popup).toHaveURL(/example\.com/);
await popup.close();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@e2e-tests/playwright/specs/functional/plugins/demo_plugin/webapp/plugin_link_tooltip.spec.ts`
around lines 106 - 112, The test currently asserts popup.url() immediately after
capturing the popup (popupPromise) which can be "about:blank" and flaky; after
awaiting the popup (variable popup) from channelsPage.page.waitForEvent('popup')
you should wait for navigation completion by calling await popup.waitForURL()
(or use await expect(popup).toHaveURL(...)) before asserting the destination URL
(the titleLink click remains the trigger). Update the code around popupPromise,
titleLink.click(), and the popup assertion to first await popup.waitForURL() and
then assert the URL contains 'example.com'.
2 participants
Summary
PluginLinkTooltip) to ensure tooltip interactivity and localization work correctly.LinkTooltipcomponent.Note: Requires demo plugin: mattermost/mattermost-plugin-demo#209
pointer-eventspattern)Ticket Link
None, ref: #36056 (comment)
Release Note
Change Impact: 🟢 Low
Regression Risk: Changes are entirely confined to E2E test infrastructure and test files with no modifications to production code. Demo plugin version updates only affect test artifact dependencies, and new test assertions verify existing functionality without changing production behavior.
QA Recommendation: Manual QA is not required. Changes are test-focused with full automated coverage. Standard E2E test execution will validate the new test cases and version updates.