[ZEPPELIN-6358] Remove anti-patterns of E2E and tidy test suite

Applied the [`e2e-reviewer`](https://github.com/dididy/e2e-skills) skill on the existing E2E suite. The skill does static analysis — it catches tests that can never actually fail, silent skips, swallowed errors in POM methods, that kind of thing.

Findings and fixes:

- `home-page-enhanced-functionality.spec.ts` was mostly duplicating `home-page-elements` and `home-page-note-operations` → deleted and merged
- `toBeGreaterThanOrEqual(0)` and `toBeAttached()` on static elements were always passing → replaced with assertions that can fail
- `if (isVisible) { expect() }` patterns silently skip when something breaks → removed or converted to `test.skip`
- Several POM methods had `.catch(() => {})` with no comment → removed; kept the intentional ones and marked with `// JUSTIFIED:`
- `document.querySelector` in `page.evaluate()` → swapped for Playwright locator API
- Added `aria-label` / `data-testid` to action bar HTML; a few tests were breaking on DOM structure changes
- Renamed a handful of tests whose names didn't match what they actually tested; dropped the ones that only called `toBeVisible()`

Improvement
Refactoring

ZEPPELIN-6358

* Does the license files need to update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No

Closes #5180 from dididy/tidy-e2e.

Signed-off-by: Jongyoul Lee <jongyoul@gmail.com>
(cherry picked from commit 076676a)

Author: dididy

zeppelin-web-angular/e2e/models/about-zeppelin-modal.ts

@@ -43,10 +43,6 @@ export class AboutZeppelinModal extends BasePage {
     return (await this.versionText.textContent()) || '';
   }
 
-  async isLogoVisible(): Promise<boolean> {
-    return this.logo.isVisible();
-  }
-
   async getGetInvolvedHref(): Promise<string | null> {
     return this.getInvolvedLink.getAttribute('href');
   }

zeppelin-web-angular/e2e/models/base-page.ts

@@ -24,8 +24,6 @@ export class BasePage {
   readonly zeppelinHeader: Locator;
 
   readonly modalTitle: Locator;
-  readonly modalBody: Locator;
-  readonly modalContent: Locator;
 
   readonly okButton: Locator;
   readonly cancelButton: Locator;
@@ -41,8 +39,6 @@ export class BasePage {
     this.zeppelinHeader = page.locator('zeppelin-header');
 
     this.modalTitle = page.locator('.ant-modal-confirm-title, .ant-modal-title');
-    this.modalBody = page.locator('.ant-modal-confirm-content, .ant-modal-body');
-    this.modalContent = page.locator('.ant-modal-body');
 
     this.okButton = page.locator('button:has-text("OK")');
     this.cancelButton = page.locator('button:has-text("Cancel")');
@@ -71,11 +67,6 @@ export class BasePage {
     await this.navigateToRoute('/');
   }
 
-  getCurrentPath(): string {
-    const url = new URL(this.page.url());
-    return url.hash || url.pathname;
-  }
-
   async waitForUrlNotContaining(fragment: string): Promise<void> {
     await this.page.waitForURL(url => !url.toString().includes(fragment));
   }
@@ -85,14 +76,8 @@ export class BasePage {
   }
 
   async waitForFormLabels(labelTexts: string[], timeout = 10000): Promise<void> {
-    await this.page.waitForFunction(
-      texts => {
-        const labels = Array.from(document.querySelectorAll('nz-form-label'));
-        return texts.some(text => labels.some(l => l.textContent?.includes(text)));
-      },
-      labelTexts,
-      { timeout }
-    );
+    const locators = labelTexts.map(text => this.page.locator('nz-form-label', { hasText: text }));
+    await Promise.race(locators.map(l => l.waitFor({ state: 'attached', timeout })));
   }
 
   async waitForElementAttribute(
@@ -109,25 +94,20 @@ export class BasePage {
     }
   }
 
-  async waitForRouterOutletChild(timeout = 10000): Promise<void> {
-    await expect(this.page.locator('zeppelin-workspace router-outlet + *')).toHaveCount(1, { timeout });
-  }
-
   async fillAndVerifyInput(
     locator: Locator,
     value: string,
     options?: { timeout?: number; clearFirst?: boolean }
   ): Promise<void> {
-    const { timeout = 10000, clearFirst = true } = options || {};
+    const { timeout = 10000 } = options || {};
 
     await expect(locator).toBeVisible({ timeout });
     await expect(locator).toBeEnabled({ timeout: 5000 });
 
-    if (clearFirst) {
-      await locator.clear();
-    }
-
+    // Click first so Angular's form control is focused and its initial setValue cycle
+    // has completed before we overwrite it.  Then fill() atomically sets the value.
+    await locator.click();
     await locator.fill(value);
-    await expect(locator).toHaveValue(value);
+    await expect(locator).toHaveValue(value, { timeout: 10000 });
   }
 }

zeppelin-web-angular/e2e/models/home-page.ts

@@ -17,8 +17,6 @@ export class HomePage extends BasePage {
   readonly notebookSection: Locator;
   readonly helpSection: Locator;
   readonly communitySection: Locator;
-  readonly zeppelinLogo: Locator;
-  readonly anonymousUserIndicator: Locator;
   readonly welcomeSection: Locator;
   readonly moreInfoGrid: Locator;
   readonly notebookColumn: Locator;
@@ -28,9 +26,6 @@ export class HomePage extends BasePage {
   readonly notebookHeading: Locator;
   readonly helpHeading: Locator;
   readonly communityHeading: Locator;
-  readonly createNoteModal: Locator;
-  readonly createNoteButton: Locator;
-  readonly notebookNameInput: Locator;
   readonly externalLinks: {
     documentation: Locator;
     mailingList: Locator;
@@ -41,13 +36,12 @@ export class HomePage extends BasePage {
     createNewNoteLink: Locator;
     importNoteLink: Locator;
     filterInput: Locator;
-    tree: Locator;
-    noteActions: {
-      renameNote: Locator;
-      clearOutput: Locator;
-      moveToTrash: Locator;
-    };
   };
+  readonly anonymousUserIndicator: Locator;
+  private readonly zeppelinLogo: Locator;
+  private readonly createNoteModal: Locator;
+  private readonly createNoteButton: Locator;
+  private readonly notebookNameInput: Locator;
 
   constructor(page: Page) {
     super(page);
@@ -58,8 +52,8 @@ export class HomePage extends BasePage {
     this.anonymousUserIndicator = page.locator('text=anonymous');
     this.welcomeSection = page.locator('.welcome');
     this.moreInfoGrid = page.locator('.more-info');
-    this.notebookColumn = page.locator('[nz-col]').first();
-    this.helpCommunityColumn = page.locator('[nz-col]').last();
+    this.notebookColumn = page.locator('[nz-col]').first(); // first() — left column contains the Notebook section
+    this.helpCommunityColumn = page.locator('[nz-col]').last(); // last() — right column contains Help and Community sections
     this.welcomeDescription = page.locator('.welcome').getByText('Zeppelin is web-based notebook');
     this.refreshNoteButton = page.locator('a.refresh-note');
     this.notebookHeading = this.notebookColumn.locator('h3');
@@ -79,30 +73,21 @@ export class HomePage extends BasePage {
     this.nodeList = {
       createNewNoteLink: page.locator('zeppelin-node-list a').filter({ hasText: 'Create new Note' }),
       importNoteLink: page.locator('zeppelin-node-list a').filter({ hasText: 'Import Note' }),
-      filterInput: page.locator('zeppelin-node-list input[placeholder*="Filter"]'),
-      tree: page.locator('zeppelin-node-list nz-tree'),
-      noteActions: {
-        renameNote: page.locator('.file .operation a[nztooltiptitle*="Rename note"]'),
-        clearOutput: page.locator('.file .operation a[nztooltiptitle*="Clear output"]'),
-        moveToTrash: page.locator('.file .operation a[nztooltiptitle*="Move note to Trash"]')
-      }
+      filterInput: page.locator('zeppelin-node-list input[placeholder*="Filter"]')
     };
   }
 
+  async navigateToHome(): Promise<void> {
+    await this.page.goto('/');
+    await this.waitForPageLoad();
+  }
+
   async navigateToLogin(): Promise<void> {
     await this.navigateToRoute('/login');
     // Wait for potential redirect to complete by checking URL change
     await this.waitForUrlNotContaining('#/login');
   }
 
-  async isHomeContentDisplayed(): Promise<boolean> {
-    return this.welcomeTitle.isVisible();
-  }
-
-  async isAnonymousUser(): Promise<boolean> {
-    return this.anonymousUserIndicator.isVisible();
-  }
-
   async clickZeppelinLogo(): Promise<void> {
     await this.zeppelinLogo.click({ timeout: 15000 });
   }
@@ -112,19 +97,11 @@ export class HomePage extends BasePage {
     return text || '';
   }
 
-  async getWelcomeDescriptionText(): Promise<string> {
-    const text = await this.welcomeDescription.textContent();
-    return text || '';
-  }
-
   async clickRefreshNotes(): Promise<void> {
+    await this.refreshNoteButton.waitFor({ state: 'visible', timeout: 10000 });
     await this.refreshNoteButton.click({ timeout: 15000 });
   }
 
-  async isNotebookListVisible(): Promise<boolean> {
-    return this.zeppelinNodeList.isVisible();
-  }
-
   async clickCreateNewNote(): Promise<void> {
     await this.nodeList.createNewNoteLink.click({ timeout: 15000 });
     await this.createNoteModal.waitFor({ state: 'visible' });
@@ -136,30 +113,37 @@ export class HomePage extends BasePage {
     // Wait for the modal form to be fully rendered with proper labels
     await this.page.waitForSelector('nz-form-label', { timeout: 10000 });
 
-    await this.waitForFormLabels(['Note Name', 'Clone Note']);
+    await this.waitForFormLabels(['Note Name']);
 
     // Fill and verify the notebook name input
     await this.fillAndVerifyInput(this.notebookNameInput, notebookName);
 
     // Click the 'Create' button in the modal
     await expect(this.createNoteButton).toBeEnabled({ timeout: 5000 });
     await this.createNoteButton.click({ timeout: 15000 });
-    await this.waitForPageLoad();
+    // Wait for navigation to the notebook page — confirms the note was created server-side.
+    // waitForPageLoad() (domcontentloaded) fires instantly on SPA routing and does not guarantee this.
+    await this.page.waitForURL(/\/notebook\//, { timeout: 45000 });
   }
 
   async clickImportNote(): Promise<void> {
     await this.nodeList.importNoteLink.click({ timeout: 15000 });
   }
 
   async filterNotes(searchTerm: string): Promise<void> {
-    await this.nodeList.filterInput.fill(searchTerm, { timeout: 15000 });
+    await this.page.waitForLoadState('domcontentloaded', { timeout: 10000 });
+    await this.nodeList.filterInput.waitFor({ state: 'visible', timeout: 5000 });
+    // pressSequentially fires real key events so Angular's ngModel detects the change (fill() does not).
+    // Triple-click to select all, then type to replace or Backspace to clear.
+    await this.nodeList.filterInput.click({ clickCount: 3 });
+    if (searchTerm) {
+      await this.nodeList.filterInput.pressSequentially(searchTerm);
+    } else {
+      await this.nodeList.filterInput.press('Backspace');
+    }
   }
 
   async waitForRefreshToComplete(): Promise<void> {
     await this.waitForElementAttribute('a.refresh-note i[nz-icon]', 'nzSpin', false);
   }
-
-  async getDocumentationLinkHref(): Promise<string | null> {
-    return this.externalLinks.documentation.getAttribute('href');
-  }
 }

zeppelin-web-angular/e2e/models/node-list-page.ts

@@ -41,11 +41,7 @@ export class NodeListPage extends BasePage {
     await this.createNewNoteButton.click();
   }
 
-  getFolderByName(folderName: string): Locator {
-    return this.page.locator('nz-tree-node').filter({ hasText: folderName }).first();
-  }
-
-  getNoteByName(noteName: string): Locator {
+  private getNoteByName(noteName: string): Locator {
     return this.page.locator('nz-tree-node').filter({ hasText: noteName }).first();
   }
 
@@ -56,14 +52,6 @@ export class NodeListPage extends BasePage {
     await noteLink.click();
   }
 
-  async isFilterInputVisible(): Promise<boolean> {
-    return this.filterInput.isVisible();
-  }
-
-  async isTrashFolderVisible(): Promise<boolean> {
-    return this.trashFolder.isVisible();
-  }
-
   async getAllVisibleNoteNames(): Promise<string[]> {
     const noteElements = await this.notes.all();
     const names: string[] = [];

zeppelin-web-angular/e2e/models/note-create-modal.ts

@@ -47,8 +47,4 @@ export class NoteCreateModal extends BasePage {
   async clickCreate(): Promise<void> {
     await this.createButton.click();
   }
-
-  async isFolderInfoVisible(): Promise<boolean> {
-    return this.folderInfoAlert.isVisible();
-  }
 }

zeppelin-web-angular/e2e/models/note-import-modal.ts

@@ -59,16 +59,6 @@ export class NoteImportModal extends BasePage {
     await this.urlTab.click();
   }
 
-  async isJsonFileTabSelected(): Promise<boolean> {
-    const ariaSelected = await this.jsonFileTab.getAttribute('aria-selected');
-    return ariaSelected === 'true';
-  }
-
-  async isUrlTabSelected(): Promise<boolean> {
-    const ariaSelected = await this.urlTab.getAttribute('aria-selected');
-    return ariaSelected === 'true';
-  }
-
   async setImportUrl(url: string): Promise<void> {
     await this.urlInput.fill(url);
   }
@@ -77,19 +67,7 @@ export class NoteImportModal extends BasePage {
     await this.importNoteButton.click();
   }
 
-  async isImportNoteButtonDisabled(): Promise<boolean> {
-    return this.importNoteButton.isDisabled();
-  }
-
   async getFileSizeLimit(): Promise<string> {
     return (await this.fileSizeLimit.textContent()) || '';
   }
-
-  async isErrorAlertVisible(): Promise<boolean> {
-    return this.errorAlert.isVisible();
-  }
-
-  async getErrorMessage(): Promise<string> {
-    return (await this.errorAlert.textContent()) || '';
-  }
 }

zeppelin-web-angular/e2e/models/notebook-repo-item.util.ts

@@ -24,14 +24,12 @@ export class NotebookRepoItemUtil extends BasePage {
 
   async verifyDisplayMode(): Promise<void> {
     await expect(this.repoItemPage.editButton).toBeVisible();
-    const isEditMode = await this.repoItemPage.isEditMode();
-    expect(isEditMode).toBe(false);
+    await expect(this.repoItemPage.repositoryCard).not.toHaveClass(/\bedit\b/);
   }
 
   async verifyEditMode(): Promise<void> {
     await expect(this.repoItemPage.saveButton).toBeVisible();
     await expect(this.repoItemPage.cancelButton).toBeVisible();
-    const isEditMode = await this.repoItemPage.isEditMode();
-    expect(isEditMode).toBe(true);
+    await expect(this.repoItemPage.repositoryCard).toHaveClass(/\bedit\b/);
   }
 }

zeppelin-web-angular/e2e/models/notebook-repos-page.ts

@@ -34,10 +34,6 @@ export class NotebookReposPage extends BasePage {
       this.page.waitForSelector('zeppelin-notebook-repo-item', { state: 'visible' })
     ]);
   }
-
-  async getRepositoryItemCount(): Promise<number> {
-    return await this.repositoryItems.count();
-  }
 }
 
 export class NotebookRepoItemPage extends BasePage {
@@ -72,50 +68,22 @@ export class NotebookRepoItemPage extends BasePage {
     await this.cancelButton.click({ timeout: 15000 });
   }
 
-  async isEditMode(): Promise<boolean> {
-    return await this.repositoryCard.evaluate(el => el.classList.contains('edit'));
-  }
-
-  async isSaveButtonEnabled(): Promise<boolean> {
-    return await this.saveButton.isEnabled();
-  }
-
-  async getSettingValue(settingName: string): Promise<string> {
-    const row = this.repositoryCard.locator('tbody tr').filter({ hasText: settingName });
-    const valueCell = row.locator('td').nth(1);
-    return (await valueCell.textContent()) || '';
-  }
-
   async fillSettingInput(settingName: string, value: string): Promise<void> {
     const row = this.repositoryCard.locator('tbody tr').filter({ hasText: settingName });
     const input = row.locator('input[nz-input]');
     await input.clear();
     await input.fill(value);
   }
 
-  async selectSettingDropdown(settingName: string, optionValue: string): Promise<void> {
-    const row = this.repositoryCard.locator('tbody tr').filter({ hasText: settingName });
-    const select = row.locator('nz-select');
-    await select.click({ timeout: 15000 });
-    await this.page.locator(`nz-option[nzvalue="${optionValue}"]`).click({ timeout: 15000 });
-  }
-
   async getSettingInputValue(settingName: string): Promise<string> {
     const row = this.repositoryCard.locator('tbody tr').filter({ hasText: settingName });
     const input = row.locator('input[nz-input]');
     return await input.inputValue();
   }
 
-  async isInputVisible(settingName: string): Promise<boolean> {
-    const row = this.repositoryCard.locator('tbody tr').filter({ hasText: settingName });
-    const input = row.locator('input[nz-input]');
-    return await input.isVisible();
-  }
-
-  async isDropdownVisible(settingName: string): Promise<boolean> {
+  async getSettingValue(settingName: string): Promise<string> {
     const row = this.repositoryCard.locator('tbody tr').filter({ hasText: settingName });
-    const select = row.locator('nz-select');
-    return await select.isVisible();
+    return (await row.locator('td').nth(1).textContent()) || '';
   }
 
   async getSettingCount(): Promise<number> {