PMM-14544 new navigation tests by kiranvuyurru · Pull Request #855 · percona/pmm-qa

kiranvuyurru · 2026-01-05T18:11:46Z

verify the collapse and expand of the left menu
Traverse all menu and check no 404 response
non admin RBAC
Time range persists on various dashboards
old navigation menu not visible and iframe visibility test
Node and service persistence check

travagliad · 2026-01-06T10:45:04Z

e2e_tests/pages/navigation.page.ts

+    mouseHoverOnPmmLogo = async () => {
+        const pmmLogo = this.elements.sidebar.locator('rect');
+
+        const rectBox = await pmmLogo.boundingBox();
+        if (rectBox) {
+            await this.page.mouse.move(
+                rectBox.x + rectBox.width / 2,
+                rectBox.y + rectBox.height / 2
+            );
+        }
+    };


Have you tried just using pmmLogo.hover()?

You can even pass coordinates if really needed (but I don't think it's needed)
https://playwright.dev/docs/api/class-locator#locator-hover

I tried pmmlogo.hover(), it did not work.

travagliad · 2026-01-06T10:50:21Z

e2e_tests/tests/navigation.test.ts

+
+    await leftNavigation.elements.openLeftNavigationButton.click();
+    await expect(leftNavigation.elements.closeLeftNavigationButton).toBeVisible();
+    await expect(leftNavigation.elements.openLeftNavigationButton).not.toBeVisible();


prefer toBeHidden instead of not toBeVisible as it's more readable

@peterSirotnak Do you think we should enable it as a eslint plugin?

eslint-plugin-playwright/no-useless-not

travagliad · 2026-01-06T10:52:04Z

e2e_tests/tests/navigation.test.ts

+        await leftNavigation.elements.openLeftNavigationButton.click();
+    }
+
+    await leftNavigation.traverseAllMenuItems();


What is this test, testing?

We don't have any assertions/expects anywhere in the method or the test

Agree with @travagliad . There is no need to create a method which is not used anywhere else. Same applies to other methods in the leftNavigation class
@kiranvuyurru lets move the code into the test and wrap with test.step annotations for code readability

pls use test.step everywhere possible

travagliad · 2026-01-06T10:56:28Z

e2e_tests/fixtures/pmmTest.ts

+  leftNavigation: LeftNavigation;
 }>({
-  cliHelper: async ({}, use) => {
+  cliHelper: async ({ }, use) => {


We shouldn't be changing formatting of code on PRs otherwise we end up on a review hell anytime we push, if it's auto formatting try to change your rules so it doesn't happen or push without autoformatting on existing files

travagliad · 2026-01-06T10:59:24Z

e2e_tests/tests/navigation.test.ts

+    await leftNavigation.elements.home.click();
+    await leftNavigation.elements.timePickerOpenButton.click();
+    await leftNavigation.getTimeRangeOption(selectedTimeRange).click();
+    await page.waitForLoadState('domcontentloaded');


prefer web assertions instead of load states as this is flaky

https://playwright.dev/docs/best-practices#use-web-first-assertions

travagliad · 2026-01-06T11:03:27Z

I see many playwright methods like .click() on the spec file, while this works the reason we have page objects is to abstract the test files and move all playwright logic like this method to the page objects, so when someone non technical or even us, reads the test code we can understand what the test does without going into the details of the test. So I think this should be refactored to have every playwright logic inside page object files.

Also, not having expects in a test isn't ideal, the reason being that all test are supposed to be asserting something, or expecting for something, so for better understanding of a test we should have:

steps coming from the pages objects (So the methods should have a descriptive name) and expects in the end to understand what is being tested

If you need help refactoring it let me know

yurkovychv · 2026-01-06T16:45:09Z

e2e_tests/pages/navigation.page.ts

+    public get elements() {
+        return {
+            // parent menu items
+            home: this.page.locator('//*[@data-testid="navitem-home-page"]'),


please use this.page.getByTestId() everywhere

peterSirotnak · 2026-01-15T15:05:21Z

e2e_tests/pages/navigation.page.ts

+export default class LeftNavigation {
+
+  public readonly simpleMenuItems = ['home', 'qan', 'help'] as const;
+


No need for so many empty lines.

peterSirotnak · 2026-01-15T15:05:50Z

e2e_tests/pages/navigation.page.ts

+
+const grafanaIframe = '#grafana-iframe';
+
+export default class LeftNavigation {


Suggested change

export default class LeftNavigation {

export default class LeftNavigation extends BasePage {

peterSirotnak · 2026-01-15T15:06:11Z

e2e_tests/pages/navigation.page.ts

+
+export type MenuItem = Locator | (() => Locator) | { [key: string]: MenuItem };
+
+const grafanaIframe = '#grafana-iframe';


Please remove

peterSirotnak · 2026-01-15T15:08:50Z

e2e_tests/pages/navigation.page.ts

+    return this.page.frameLocator(grafanaIframe);
+  }
+
+  selectMenuItem = async (item: string): Promise<void> => {


Suggested change

selectMenuItem = async (item: string): Promise<void> => {

selectMenuItem = async (item: SomeInteface with defined object): Promise<void> => {

Do this so we have specific options.

peterSirotnak · 2026-01-15T15:10:29Z

e2e_tests/tests/navigation.test.ts

+    }
+  };
+
+  const traverseAllMenuItems = async () => {


Move this to page object.

peterSirotnak · 2026-01-15T15:10:59Z

e2e_tests/pages/navigation.page.ts

+    return newPage;
+  }
+
+  switchPage = (page: Page) => {


Move this to base page.

peterSirotnak · 2026-01-15T15:11:08Z

e2e_tests/pages/navigation.page.ts

+    }
+  }
+
+  newTab = async (): Promise<Page> => {


Move this to base page

peterSirotnak · 2026-01-15T15:11:30Z

e2e_tests/pages/navigation.page.ts

+    oldLeftMenu: () => this.page.getByTestId('data-testid navigation mega-menu'),
+  };
+
+  iframe(): FrameLocator {


Already in base page, remove.

yurkovychv · 2026-01-16T17:14:52Z

e2e_tests/pages/navigation.page.ts

+  constructor(page: Page) { super(page); }
+  public elements = {
+    // parent menu items
+    home: () => this.page.getByTestId('navitem-home-page'),


why all locators are functions?

yurkovychv · 2026-01-16T17:16:59Z

e2e_tests/tests/navigation.test.ts

+    await leftNavigation.selectMenuItem({ path: 'mysql' });
+    const selectedNode = await leftNavigation.selectNode(4, /client_container_\d+/);
+    await leftNavigation.grafanaIframe().getByText(selectedNode).waitFor({ state: 'visible', timeout: 10000 });
+    await expect(leftNavigation.grafanaIframe().getByText(selectedNode)).toBeVisible();


can we have this locator in the page object? to not have .grafanaIframe() everywhere?

peterSirotnak · 2026-01-26T14:54:12Z

e2e_tests/tests/navigation.test.ts

+    await expect(leftNavigation.buttons.oldLeftMenu).toBeHidden();
+  });
+  await pmmTest.step('Verify iframe hidden on Help page', async () => {
+    // await leftNavigation.selectMenuItem({ path: 'help' });


Remove commented text

peterSirotnak · 2026-01-26T14:56:14Z

e2e_tests/helpers/grafana.helper.ts

  }
+
+
+  // creates non admin user


Remove comments

e2e_tests/pages/navigation.page.ts

peterSirotnak · 2026-01-27T08:45:24Z

e2e_tests/pages/navigation.page.ts

+    this.buttons = {
+      // parent menu items
+      home: this.page.getByTestId('navitem-home-page'),
+      mysql: this.page.getByTestId('navitem-mysql'),


Suggested change

this.buttons = {

// parent menu items

home: this.page.getByTestId('navitem-home-page'),

mysql: this.page.getByTestId('navitem-mysql'),

this.buttons = {

home: this.page.getByTestId('navitem-home-page'),

mysql: {

locator: this.page.getByTestId('navitem-mysql'),

elements: {

overview: this.page.getByTestId('navitem-mysql-overview'),

summary: this.page.getByTestId('navitem-mysql-summary'),

highAvailability: this.page.getByTestId('navitem-mysql-high-availability'),

ha: {

groupReplication: this.page.getByTestId('navitem-mysql-group-replication-summary'),

replication: this.page.getByTestId('navitem-mysql-replication-summary'),

pxcGaleraCluster: this.page.getByTestId('navitem-pxc-cluster-summary'),

pxcGaleraNode: this.page.getByTestId('navitem-pxc-node-summary'),

pxcGaleraNodes: this.page.getByTestId('navitem-pxc-nodes-compare'),

commandHandler: this.page.getByTestId('navitem-mysql-command-handler-counters-compare'),

innodbDetails: this.page.getByTestId('navitem-mysql-innodb-details'),

innodbCompression: this.page.getByTestId('navitem-mysql-innodb-compression-details'),

performanceSchema: this.page.getByTestId('navitem-mysql-performance-schema-details'),

queryResponseTime: this.page.getByTestId('navitem-mysql-query-response-time-details'),

tableDetails: this.page.getByTestId('navitem-mysql-table-details'),

tokudbDetails: this.page.getByTestId('navitem-mysql-tokudb-details'),

otherDashboards: this.page.getByTestId('navitem-mysql-other-dashboards'),

}

}

Do this for each database type and all menus with submenus. Also remove comments.

peterSirotnak · 2026-01-27T08:48:43Z

e2e_tests/pages/navigation.page.ts

+  public readonly elements;
+
+  public readonly simpleMenuItems = ['home', 'qan', 'help'] as const;
+  public readonly menuWithChildren = [


Replace with method that finds menu with children maybe also with variable timerange: true also for simpleMenuItems dashboardsToVerifyTimeRange

e2e_tests/pages/navigation.page.ts

peterSirotnak · 2026-01-27T08:54:04Z

e2e_tests/pages/navigation.page.ts

+    });
+  };
+
+  public selectTimeRange = async (timeRange: string): Promise<void> => {


move this to base page

peterSirotnak · 2026-01-27T08:55:36Z

e2e_tests/pages/navigation.page.ts

+    return this.selectVariableValue(dashboardIndex);
+  };
+
+  public selectNode = async (dashboardIndex: number): Promise<string> => {


move selecting methods to base page

peterSirotnak · 2026-01-30T14:28:52Z

e2e_tests/tests/navigation.test.ts

+  async ({ page, leftNavigation }) => {
+    pmmTest.slow();
+    const ignore404 = (url: string) => url.includes('/settings') || url.includes('/admin_config');
+    const responseAfterClick = async (locator: Locator, name: string) => {


Why is this method in the test? Not in the page object?

this has assertions

e2e_tests/pages/navigation.page.ts

1. verify the collapse and expand of the left menu 2. Traverse all menu and check no 404 response 3. non admin RBAC 4. Time range persists on various dashboards 5. old navigation menu not visible and iframe visibility test

Refactored e2e test files and page objects for improved readability and consistency, including formatting, arrow function usage, and type improvements. Updated the IPageObject interface to support nested locators. No functional changes were made to test logic.

* create traverse test as a new file * Update navigation.page.ts * Update leftMenutravese.test.ts * Update navigation.page.ts

kiranvuyurru requested review from peterSirotnak and yurkovychv January 5, 2026 18:11

travagliad requested changes Jan 6, 2026

View reviewed changes

yurkovychv reviewed Jan 6, 2026

View reviewed changes

peterSirotnak requested changes Jan 15, 2026

View reviewed changes

kiranvuyurru force-pushed the PMM-14544-new-navigation branch 2 times, most recently from 8aa1241 to ed701a1 Compare January 16, 2026 16:12

yurkovychv requested changes Jan 16, 2026

View reviewed changes

kiranvuyurru force-pushed the PMM-14544-new-navigation branch from 78bf848 to 63ce711 Compare January 21, 2026 17:11

peterSirotnak reviewed Jan 26, 2026

View reviewed changes

e2e_tests/helpers/grafana.helper.ts Outdated

}

// creates non admin user

Copy link

Contributor

peterSirotnak Jan 26, 2026

Choose a reason for hiding this comment

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

Remove comments

peterSirotnak reviewed Jan 27, 2026

View reviewed changes

e2e_tests/pages/navigation.page.ts Outdated Show resolved Hide resolved

peterSirotnak requested changes Jan 27, 2026

View reviewed changes

peterSirotnak requested changes Jan 30, 2026

View reviewed changes

kiranvuyurru added 15 commits January 30, 2026 09:08

PMM-14365 new navigation tests

f8ca8bd

New left menu test

b49bf3c

1. verify the collapse and expand of the left menu 2. Traverse all menu and check no 404 response 3. non admin RBAC 4. Time range persists on various dashboards 5. old navigation menu not visible and iframe visibility test

refactored

0bab4e3

refactored

a9c3d83

Update grafana.helper.ts

050c10f

Update navigation.page.ts

4060b7b

added tags and fixed indentation

6455e38

changes and fixes

5eca751

changes and fixes

51c0566

Update navigation.page.ts

e17675e

changes and fixes

84f41d9

updated

ee1e9e7

Update navigation.page.ts

5eea4f8

Refactor navigation page and tests for new menu structure

28a4b3c

Rename 'Percona Advisors' to 'Advisors'

e868271

kiranvuyurru added 7 commits January 30, 2026 09:10

Change grafanaIframe to protected access modifier

09b339d

Refactor navigation page structure and variable selection

ff93d5d

Refactor navigation page objects and tests

ced9f7e

Update navigation.page.ts

e5b9c8a

Improve navigation menu traversal and test reliability

ce515dc

refactored

f11db83

kiranvuyurru force-pushed the PMM-14544-new-navigation branch from d94fbda to f11db83 Compare January 30, 2026 16:11

kiranvuyurru added 15 commits January 30, 2026 10:40

fixed flakiness

63666bf

Update navigation.test.ts

29e3177

Update navigation.test.ts

5e21c17

Update navigation.page.ts

acdcb83

Refactor nav traversal and update tests

0fdb7f8

Update navigation.page.ts

7c159aa

Update navigation.test.ts

1838428

Update navigation.page.ts

e12b0cf

Update navigation.page.ts

ee1e7f2

Update navigation.test.ts

5c5fe8a

refactor

a86e7a3

Update navigation.test.ts

5ad7b40

create traverse test as a new file (#872)

7934687

* create traverse test as a new file * Update navigation.page.ts * Update leftMenutravese.test.ts * Update navigation.page.ts

moved traverse test to navigation.test.ts

944709e

Update navigation.test.ts

a349f81

		export default class LeftNavigation {

		public readonly simpleMenuItems = ['home', 'qan', 'help'] as const;


		const grafanaIframe = '#grafana-iframe';

		export default class LeftNavigation {

	export default class LeftNavigation {
	export default class LeftNavigation extends BasePage {


		export type MenuItem = Locator \| (() => Locator) \| { [key: string]: MenuItem };

		const grafanaIframe = '#grafana-iframe';

	selectMenuItem = async (item: string): Promise<void> => {
	selectMenuItem = async (item: SomeInteface with defined object): Promise<void> => {

-    this.buttons = {
-      // parent menu items
-      home: this.page.getByTestId('navitem-home-page'),
-      mysql: this.page.getByTestId('navitem-mysql'),
+    this.buttons = {
+      home: this.page.getByTestId('navitem-home-page'),
+      mysql: {
+        locator: this.page.getByTestId('navitem-mysql'),
+        elements: {
+        overview: this.page.getByTestId('navitem-mysql-overview'),
+        summary: this.page.getByTestId('navitem-mysql-summary'),
+        highAvailability: this.page.getByTestId('navitem-mysql-high-availability'),
+        ha: {
+          groupReplication: this.page.getByTestId('navitem-mysql-group-replication-summary'),
+          replication: this.page.getByTestId('navitem-mysql-replication-summary'),
+          pxcGaleraCluster: this.page.getByTestId('navitem-pxc-cluster-summary'),
+          pxcGaleraNode: this.page.getByTestId('navitem-pxc-node-summary'),
+          pxcGaleraNodes: this.page.getByTestId('navitem-pxc-nodes-compare'),
+                  commandHandler: this.page.getByTestId('navitem-mysql-command-handler-counters-compare'),
+        innodbDetails: this.page.getByTestId('navitem-mysql-innodb-details'),
+        innodbCompression: this.page.getByTestId('navitem-mysql-innodb-compression-details'),
+        performanceSchema: this.page.getByTestId('navitem-mysql-performance-schema-details'),
+        queryResponseTime: this.page.getByTestId('navitem-mysql-query-response-time-details'),
+        tableDetails: this.page.getByTestId('navitem-mysql-table-details'),
+        tokudbDetails: this.page.getByTestId('navitem-mysql-tokudb-details'),
+        otherDashboards: this.page.getByTestId('navitem-mysql-other-dashboards'),
+        }
+      }

Conversation

kiranvuyurru commented Jan 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

travagliad commented Jan 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yurkovychv Jan 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kiranvuyurru commented Jan 5, 2026 •

edited

Loading

travagliad commented Jan 6, 2026 •

edited

Loading

yurkovychv Jan 6, 2026 •

edited

Loading