Skip to content

Conversation

@travagliad
Copy link
Contributor

Introduce a bootstrap hook for handling Grafana iframes in tests, fix issues related to the new UI, and implement general test improvements to enhance stability and performance.

@travagliad travagliad changed the base branch from experimenting-hooks-for-iframe to main December 23, 2025 19:38
@travagliad
Copy link
Contributor Author

Test Results:
FB Tests:
Can't be tested now, need to activate new UI automatically (all will fail for expecting iframe)
Nightly:
Nightly Test Results Link

Note 1:
Remaining issues are not related to these changes.
Note 2:
I fixed some test bugs too (last commit of this PR), if you find it inappropriate to be in this PR we may remove it and create it's own PR.

@travagliad
Copy link
Contributor Author

@copilot Create a description of how hooks.js enables iframe in test suite and provide a quick insight into how much of a shortcut this is instead of doing it in every locator.

@travagliad
Copy link
Contributor Author

🛠️ Implementation: Automated Grafana Iframe Handling (hooks.js)

Intent:
This PR introduces a global Bootstrap Hook (hooks.js) designed to abstract away the complexity of PMM's embedded new Iframe architecture. The intent is to make the #grafana-iframe completely transparent to test writers, allowing standard CodeceptJS syntax to function without manual context management.

Architecture & Insights:
Instead of requiring manual I.switchTo() calls in every test, this hook acts as a middleware layer between CodeceptJS and the Playwright helper.

  1. Context-Aware Proxying: We intercept navigation and interaction methods. If the Grafana iframe is present, the helper automatically swaps the standard Page context for a Playwright FrameLocator.
  2. Auto-Healing: The hook monitors navigation events (amOnPage, switchToNextTab). If the context is lost (e.g., switching tabs), it intelligently locates the active page and re-establishes the iframe connection before the next action executes.
  3. Polyfills for FrameLocator: Native Playwright FrameLocators lack certain methods (like keyboard.press). The hook injects these dependencies from the main page, allowing seamless use of keyboard/mouse actions inside the iframe.
  4. Selector Normalization: Centralized handling for various locator strategies (XPath, CSS, Objects) and support for the $ shorthand (e.g., $my-element[data-testid="my-element"]).

Impact (Before vs. After):

This implementation drastically reduces boilerplate. A standard interaction that previously required 5 lines of infrastructure code is now a single line of business logic.

// ❌ BEFORE: Manual Infrastructure Management
await I.switchTo();
await I.waitForVisible('#grafana-iframe', 60);
await I.switchTo('#grafana-iframe');
await I.waitForText('No queries available', 30);
await I.switchTo(); // Reset

// ✅ AFTER: Pure Business Logic
I.waitForText('No queries available', 30);

Key Benefits:

  • Zero Boilerplate: Estimated reduction of ~5,000+ lines of code across the suite.
  • Stability: Eliminates flaky tests caused by missing waitForLoad or incorrect context switching.
  • Maintainability: Iframe logic is centralized in one file; tests focus solely on behavior

Generated by AI

await locator.waitFor({ state: 'attached' });
await locator.type(filterName);
await page.waitForTimeout(200);
await new Promise((resolve) => { setTimeout(resolve, 200); });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because before the test runs on the hook we change page to be handled as a frameLocator, so waitForTimeout doesn't exist in it, it was the only occurrence I could find on our tests, so I just changed the waitForTimeout to a nodejs sleep instead.

I.fillField(this.fields.publicAddressInput, address);
I.click(this.fields.advancedButton);
I.verifyPopUpMessage(this.messages.successPopUpMessage);
I.refreshPage();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why refreshing of the page?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't remember, it was a fix for some failures on FB tests, I'll remove and retry.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When removed, docker configuration fails consistently

https://github.com/percona/pmm-qa/actions/runs/20758328823/job/59819078766

'PMM-T2050 - Verify PostgreSQL Instance Summary Dashboard @nightly @dashboards',
async ({ I, dashboardPage }) => {
const { service_name } = await inventoryAPI.getServiceDetailsByStartsWithName('pdpgsql_pmm_');
const { service_name } = await inventoryAPI.apiGetNodeInfoByServiceName(SERVICE_TYPE.POSTGRESQL, 'pdpgsql_pmm_', 'patroni');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With changes made to patroni the service chosen for checking the dashboard was being one of the patronis, and it had no data, so I changed it to exclude patroni and instead pickup the main service (I noticed the existence of another method that had this functionality so I just reused)

This change isn't related to new native, it was just a test bug that I fixed here, I can remove

@travagliad travagliad enabled auto-merge (squash) January 8, 2026 17:15
@travagliad travagliad disabled auto-merge January 8, 2026 17:16
@travagliad travagliad merged commit 2de02bf into main Jan 8, 2026
1 of 26 checks passed
@travagliad travagliad deleted the PMM-14365 branch January 8, 2026 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants