Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion zeppelin-web-angular/e2e/models/configuration-page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export class ConfigurationPage extends BasePage {
this.headerCells = this.table.locator('thead th');
// Both antd and ng-zorro render the "no data" state as a row, so exclude it
// to keep the counts about actual configuration entries.
this.rows = this.table.locator('tbody tr:not(.ant-table-placeholder)');
this.rows = this.table.locator('tbody tr:has(td:nth-child(2))');
}

async navigate(): Promise<void> {
Expand Down
15 changes: 7 additions & 8 deletions zeppelin-web-angular/e2e/models/notebook-repos-page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ export class NotebookReposPage extends BasePage {
}
}

export const getNotebookRepoSettingRows = (root: Locator): Locator => root.locator('tbody tr:has(td:nth-child(2))');

export class NotebookRepoItemPage extends BasePage {
readonly repositoryCard: Locator;
readonly repositoryName: Locator;
Expand All @@ -80,13 +82,12 @@ export class NotebookRepoItemPage extends BasePage {
constructor(page: Page, repoName: string) {
super(page);
this.repositoryCard = page.locator(`[data-testid="notebook-repo-item"][data-repo-name="${repoName}"]`);
this.repositoryName = this.repositoryCard.locator('.ant-card-head-title');
this.repositoryName = this.repositoryCard.getByText(repoName, { exact: true });
this.editButton = this.repositoryCard.locator('button:has-text("Edit")');
this.saveButton = this.repositoryCard.locator('button:has-text("Save")');
this.cancelButton = this.repositoryCard.locator('button:has-text("Cancel")');
// .ant-table is what both ng-zorro and antd render.
this.settingTable = this.repositoryCard.locator('.ant-table');
this.settingRows = this.repositoryCard.locator('tbody tr:not(.ant-table-placeholder)');
this.settingTable = this.repositoryCard.getByRole('table');
this.settingRows = getNotebookRepoSettingRows(this.repositoryCard);
}

async clickEdit(): Promise<void> {
Expand All @@ -109,15 +110,13 @@ export class NotebookRepoItemPage extends BasePage {

async fillSettingInput(settingName: string, value: string): Promise<void> {
const row = this.repositoryCard.locator('tbody tr').filter({ hasText: settingName });
// .ant-input, not [nz-input], since ng-zorro's nz-input directive renders that class too,
// and it excludes a DROPDOWN row's Select search input.
const input = row.locator('input.ant-input');
const input = row.getByRole('textbox');
await this.fillAndVerifyInput(input, value);
}

async getSettingInputValue(settingName: string): Promise<string> {
const row = this.repositoryCard.locator('tbody tr').filter({ hasText: settingName });
const input = row.locator('input.ant-input');
const input = row.getByRole('textbox');
return await input.inputValue();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export class PublishedParagraphPage extends BasePage {
this.reactWidget = page.locator('[data-testid="react-published-paragraph"]');
this.textOutput = page.locator('zeppelin-publish-paragraph pre');
// Without paragraph data the remote mounts an <Empty>, so tests that only assert "React took over" accept either.
this.reactWidgetOrEmptyState = this.reactWidget.or(page.locator('.ant-alert'));
this.reactWidgetOrEmptyState = this.reactWidget.or(page.getByTestId('react-empty-paragraph'));
}

async navigateToNotebook(noteId: string): Promise<void> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const MOUNTED_TABLE = `${MOUNT} ${TABLE}`;
// The entries arrive from ConfigurationService after the page settles, so wait
// for the first row before reading; evaluateAll does not retry on its own.
const readRows = async (page: Page, root: string): Promise<string[][]> => {
const rows = page.locator(`${root} tbody tr:not(.ant-table-placeholder)`);
const rows = page.locator(`${root} tbody tr:has(td:nth-child(2))`);
await expect(rows.first()).toBeVisible({ timeout: 15000 });
return rows.evaluateAll(all =>
all.map(row => Array.from(row.querySelectorAll('td')).map(cell => (cell.textContent ?? '').trim()))
Expand Down Expand Up @@ -99,7 +99,7 @@ test.describe('Configuration Page - React table behind a flag', () => {
await expect(page.locator(TABLE)).toBeVisible({ timeout: 15000 });
await expect(page.locator(MOUNT)).toHaveCount(0);
// JUSTIFIED: this spec uses raw selectors throughout so it can scope to the mount host; it builds no POM.
await expect(page.locator(`${TABLE} tbody tr:not(.ant-table-placeholder)`)).not.toHaveCount(0);
await expect(page.locator(`${TABLE} tbody tr:has(td:nth-child(2))`)).not.toHaveCount(0);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ test.describe('Notebook Repository Item - Display Mode', () => {

// JUSTIFIED: .first() picks the first configured repo; tests require at least one repo to be present
const firstCard = notebookReposPage.repositoryItems.first();
firstRepoName = (await firstCard.locator('.ant-card-head-title').textContent()) || '';
firstRepoName = (await firstCard.getAttribute('data-repo-name')) || '';
expect(firstRepoName, 'No repository found — ensure at least one repo is configured').not.toBe('');
repoItemPage = new NotebookRepoItemPage(page, firstRepoName);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ test.describe('Notebook Repository Item - Edit Mode', () => {

// JUSTIFIED: .first() picks the first configured repo; tests require at least one repo to be present
const firstCard = notebookReposPage.repositoryItems.first();
firstRepoName = (await firstCard.locator('.ant-card-head-title').textContent()) || '';
firstRepoName = (await firstCard.getAttribute('data-repo-name')) || '';
expect(firstRepoName, 'No repository found — ensure at least one repo is configured').not.toBe('');
repoItemPage = new NotebookRepoItemPage(page, firstRepoName);
repoItemUtil = new NotebookRepoItemUtil(page, firstRepoName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ test.describe('Notebook Repository Item - Form Validation', () => {

// JUSTIFIED: .first() picks the first configured repo; tests require at least one repo to be present
const firstCard = notebookReposPage.repositoryItems.first();
firstRepoName = (await firstCard.locator('.ant-card-head-title').textContent()) || '';
firstRepoName = (await firstCard.getAttribute('data-repo-name')) || '';
expect(firstRepoName, 'No repository found — ensure at least one repo is configured').not.toBe('');
repoItemPage = new NotebookRepoItemPage(page, firstRepoName);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ test.describe('Notebook Repository Item - Settings', () => {

// JUSTIFIED: .first() picks the first configured repo; tests require at least one repo to be present
const firstCard = notebookReposPage.repositoryItems.first();
firstRepoName = (await firstCard.locator('.ant-card-head-title').textContent()) || '';
firstRepoName = (await firstCard.getAttribute('data-repo-name')) || '';
expect(firstRepoName, 'No repository found — ensure at least one repo is configured').not.toBe('');
repoItemPage = new NotebookRepoItemPage(page, firstRepoName);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ import { addPageAnnotationBeforeEach, waitForZeppelinReady, PAGES } from '../../

// Run on both branches of the ZEPPELIN-6631 flag. verifyDisplayMode/verifyEditMode and
// fillSettingInput/getSettingInputValue are the only e2e paths that exercise the React card's
// markup (button visibility instead of the Angular-only `.edit` class, `.ant-input` instead of
// `[nz-input]`) - without this, those selectors are only ever proven against the Angular branch.
// markup (button visibility instead of the Angular-only `.edit` class). Without this,
// those selectors are only ever proven against the Angular branch.
for (const branch of NOTEBOOK_REPOS_BRANCHES) {
const { label } = branch;

Expand All @@ -40,7 +40,7 @@ for (const branch of NOTEBOOK_REPOS_BRANCHES) {

// JUSTIFIED: .first() picks the first configured repo; tests require at least one repo to be present
const firstCard = notebookReposPage.repositoryItems.first();
firstRepoName = (await firstCard.locator('.ant-card-head-title').textContent()) || '';
firstRepoName = (await firstCard.getAttribute('data-repo-name')) || '';
repoItemPage = new NotebookRepoItemPage(page, firstRepoName);
repoItemUtil = new NotebookRepoItemUtil(page, firstRepoName);
});
Expand All @@ -62,9 +62,8 @@ for (const branch of NOTEBOOK_REPOS_BRANCHES) {
const settingName = (await row.locator('td').first().textContent()) || '';

// JUSTIFIED: inline, not lifted to the Page Object - this locator only needs to
// distinguish an INPUT row from a DROPDOWN row within this row-scan loop. .ant-input, not
// [nz-input]: excludes a DROPDOWN row's Select search input.
const isInputVisible = await row.locator('input.ant-input').isVisible();
// distinguish an INPUT row from a DROPDOWN row within this row-scan loop.
const isInputVisible = await row.getByRole('textbox').isVisible();
if (isInputVisible) {
// Writes the value straight back rather than a distinct one: this repo config is shared
// across the whole suite, which runs this spec across both branches and every browser
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@ test.describe('Notebook Repository Page - Structure', () => {
test('should display all repository items with names', async () => {
await expect(notebookReposPage.repositoryItems).not.toHaveCount(0);
// JUSTIFIED: .first() samples the first repo card; all cards share the same title structure
const firstTitle = notebookReposPage.repositoryItems.first().locator('.ant-card-head-title');
await expect(firstTitle).not.toBeEmpty();
const firstCard = notebookReposPage.repositoryItems.first();
await expect(firstCard).toHaveAttribute('data-repo-name', /.+/);
const repoName = (await firstCard.getAttribute('data-repo-name'))!;
await expect(firstCard.getByText(repoName, { exact: true })).toBeVisible();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
*/

import { expect, test, Page } from '@playwright/test';
import { getNotebookRepoSettingRows } from '../../../models/notebook-repos-page';
import { addPageAnnotationBeforeEach, PAGES, waitForZeppelinReady } from '../../../utils';

// Both branches render REPO_ITEM; only the React branch has a mount host around
Expand All @@ -24,7 +25,7 @@ const openRepos = async (page: Page, query = ''): Promise<void> => {
await waitForZeppelinReady(page);
};

const settingRows = (page: Page, root: string) => page.locator(`${root} tbody tr:not(.ant-table-placeholder)`);
const settingRows = (page: Page, root: string) => getNotebookRepoSettingRows(page.locator(root));

test.describe('Notebook Repository - React list behind a flag', () => {
addPageAnnotationBeforeEach(PAGES.WORKSPACE.NOTEBOOK_REPOS);
Expand Down Expand Up @@ -85,15 +86,13 @@ test.describe('Notebook Repository - React list behind a flag', () => {
// JUSTIFIED: inline rather than NotebookRepoItemPage - this spec locates the card via
// MOUNT/REPO_ITEM constants rather than that Page Object, so reusing its selector alone
// without its `repositoryCard` root would be inconsistent with the rest of the file.
// .ant-input, not a bare 'input': a DROPDOWN row's Select also renders an
// <input role="combobox"> that this would otherwise match instead.
const input = card.locator('input.ant-input').first();
const input = card.getByRole('textbox').first();
await expect(input).toBeVisible();
await expect(input).toHaveValue(value);

await card.getByRole('button', { name: 'Cancel' }).click();
await expect(card.getByRole('button', { name: 'Edit' })).toBeVisible();
await expect(card.locator('input.ant-input')).toHaveCount(0);
await expect(card.getByRole('textbox')).toHaveCount(0);
});

test('when the remote fails to load, the Angular list renders', async ({ page }) => {
Expand Down
2 changes: 1 addition & 1 deletion zeppelin-web-angular/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@
"cross-env NODE_OPTIONS='--max-old-space-size=8192' eslint --fix",
"prettier --write"
],
"**/*.{js,mjs,json,css,html}": [
"**/*.{tsx,js,mjs,json,css,html}": [
"prettier --write"
]
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@ module.exports = tseslint.config(
...react.configs.recommended.rules,
...react.configs['jsx-runtime'].rules,
// == legacy `plugin:react-hooks/recommended`
...reactHooks.configs.recommended.rules,
'react-hooks/rules-of-hooks': 'error',
'react-hooks/exhaustive-deps': 'warn',

// == legacy custom `rules` (1:1 port from .eslintrc.json)
'@typescript-eslint/no-explicit-any': 'error',
Expand Down
Loading
Loading