diff --git a/zeppelin-web-angular/e2e/models/notebook-keyboard-page.ts b/zeppelin-web-angular/e2e/models/notebook-keyboard-page.ts index 44e5c3118c8..fb4a24a3a6f 100644 --- a/zeppelin-web-angular/e2e/models/notebook-keyboard-page.ts +++ b/zeppelin-web-angular/e2e/models/notebook-keyboard-page.ts @@ -10,7 +10,7 @@ * limitations under the License. */ -import test, { expect, Locator, Page } from '@playwright/test'; +import { expect, Locator, Page } from '@playwright/test'; import { navigateToNotebookWithFallback } from '../utils'; import { ShortcutsMap } from '../../src/app/key-binding/shortcuts-map'; import { ParagraphActions } from '../../src/app/key-binding/paragraph-actions'; @@ -123,6 +123,8 @@ export class NotebookKeyboardPage extends BasePage { if (!(await isSettled())) { await this.focusParagraphHost(paragraphIndex); await press(); + // Wait for this press's effect before retrying — an immediate recheck would double-press a slow toggle. The wait must comfortably exceed legitimate settle latency so only a genuinely dropped press is retried. + await expect.poll(isSettled, { timeout: 10000 }).toBe(true); } expect(await isSettled()).toBe(true); }).toPass({ timeout: 15000 }); @@ -369,27 +371,30 @@ export class NotebookKeyboardPage extends BasePage { return this.readEditorText(this.paragraphContainer.first()); } + // Gate for emptiness assertions — an empty Monaco model still renders one (empty) .view-line. + async waitForEditorRendered(paragraphIndex: number): Promise { + const paragraph = this.getParagraphByIndex(paragraphIndex); + await expect(paragraph.locator('.monaco-editor .view-line').first()).toBeAttached({ timeout: 10000 }); + } + // Reconstruct editor text from Monaco's absolutely-positioned `.view-line` divs sorted by top (DOM order need not match line order), via textContent; innerText is "" for off-layout lines in headless Chromium. + // Constraints: Monaco virtualizes lines (keep fixtures short); returns '' for both an empty and a not-yet-rendered editor. private async readEditorText(paragraph: Locator): Promise { const monaco = paragraph.locator('.monaco-editor').first(); - if ((await monaco.count()) > 0) { - const text = await monaco.evaluate((el: Element) => { - const lines = Array.from(el.querySelectorAll('.view-line')) as HTMLElement[]; - lines.sort((a, b) => parseInt(a.style.top || '0', 10) - parseInt(b.style.top || '0', 10)); - return lines.map(l => (l.textContent || '').replace(/\u00a0/g, ' ')).join('\n'); - }); - if (text.trim().length > 0) { - return text; - } + if ((await monaco.count()) === 0) { + return ''; } - const textarea = paragraph.locator('.monaco-editor textarea').first(); - if ((await textarea.count()) > 0) { - const value = await textarea.inputValue().catch(() => ''); - if (value) { - return value; - } - } - return ''; + // Reconstruct from view-lines only. The hidden textarea holds Monaco's IME buffer + // (a fragment, not the full model), so falling back to it returns partial text and + // makes reads non-deterministic. Callers poll, so '' while lines render is fine. + return monaco.evaluate((el: Element) => { + const lines = Array.from(el.querySelectorAll('.view-line')) as HTMLElement[]; + lines.sort((a, b) => parseInt(a.style.top || '0', 10) - parseInt(b.style.top || '0', 10)); + // Normalize all Unicode space separators (NBSP etc. that Monaco renders for + // whitespace) to a plain space, but keep line structure so exact-match assertions + // still verify it. + return lines.map(l => (l.textContent || '').replace(/\p{Zs}/gu, ' ')).join('\n'); + }); } async setCodeEditorContent(content: string, paragraphIndex: number = 0): Promise { @@ -398,55 +403,74 @@ export class NotebookKeyboardPage extends BasePage { return; } - await this.tryFocusCodeEditor(paragraphIndex); - if (this.page.isClosed()) { - console.warn('Cannot set code editor content: page closed after focusing'); - return; - } + // Seed via REST, not the editor: per-keystroke seeding fights the collaborative + // patch loop, merging a fresh note's "%python" prefix into the content on Firefox. + await this.seedParagraphViaRest(paragraphIndex, content); - const paragraph = this.getParagraphByIndex(paragraphIndex); - const editorInput = paragraph.locator('.monaco-editor .inputarea, .monaco-editor textarea').first(); + await this.tryFocusCodeEditor(paragraphIndex); - const browserName = test.info().project.name; - if (browserName !== 'firefox') { - await editorInput.waitFor({ state: 'visible', timeout: 30000 }); - await editorInput.click(); - await editorInput.clear(); + // The REST flush resets the cursor to (1,1); restore end-of-content so tests that + // seed then press End/Enter start from the right line. Keymap-independent. + if (content) { + await this.pressSelectAll(); + await this.page.keyboard.press('ArrowRight'); } + } - // Clear existing content with keyboard shortcuts for better reliability - await editorInput.focus(); + private noteIdFromUrl(): string { + const match = /\/notebook\/([^/?#]+)/.exec(this.page.url()); + if (!match) { + throw new Error(`No noteId in URL ${this.page.url()}`); + } + return match[1]; + } - if (browserName === 'firefox') { - // Clear by backspacing existing content length - const currentContent = await editorInput.inputValue(); - const contentLength = currentContent.length; + // Read a paragraph's persisted text from the server. Use this instead of the editor + // DOM when asserting content equality: Monaco reconstructs the same text with + // different whitespace codepoints across reads, which breaks byte-exact matches. + async getParagraphTextByIndex(paragraphIndex: number): Promise { + const noteId = this.noteIdFromUrl(); + const response = await this.page.request.get(`/api/notebook/${noteId}`, { failOnStatusCode: false }); + if (!response.ok()) { + throw new Error(`Fetch notebook REST request failed: ${response.status()} ${await response.text()}`); + } + const json = (await response.json()) as { body?: { paragraphs?: Array<{ text?: string }> } }; + return json.body?.paragraphs?.[paragraphIndex]?.text ?? ''; + } - // Position cursor at end and backspace all content - await this.page.keyboard.press('End'); - for (let i = 0; i < contentLength; i++) { - await this.page.keyboard.press('Backspace'); - } + private async seedParagraphViaRest(paragraphIndex: number, content: string): Promise { + const noteId = this.noteIdFromUrl(); + const paragraph = this.getParagraphByIndex(paragraphIndex); - // JUSTIFIED: Monaco textarea can be covered by editor overlays during fixture setup. - await editorInput.fill(content, { force: true }); - } else { - // Standard clearing for other browsers - await this.pressSelectAll(); - await this.page.keyboard.press('Delete'); - // JUSTIFIED: Monaco textarea can be overlaid after select+delete during fixture setup. - await editorInput.fill(content, { force: true }); + const paragraphId = await this.resolveParagraphId(noteId, paragraphIndex); + const response = await this.page.request.put(`/api/notebook/${noteId}/paragraph/${paragraphId}`, { + data: { text: content }, + failOnStatusCode: false + }); + if (!response.ok()) { + throw new Error(`Seed paragraph REST request failed: ${response.status()} ${await response.text()}`); } - // Wait for the full normalized editor content to avoid stale Monaco renders. + // The PUT broadcasts the paragraph; wait for the flush to render the exact content. const expected = content.replace(/\s+/g, ''); - if (expected.length === 0) { - await expect.poll(async () => (await this.readEditorText(paragraph)).trim(), { timeout: 10000 }).toBe(''); - } else { - await expect - .poll(async () => (await this.readEditorText(paragraph)).replace(/\s+/g, ''), { timeout: 10000 }) - .toContain(expected); - } + await expect + .poll(async () => (await this.readEditorText(paragraph)).replace(/\s+/g, ''), { timeout: 15000 }) + .toBe(expected); + } + + private async resolveParagraphId(noteId: string, paragraphIndex: number): Promise { + // Retry: a paragraph just inserted through the UI may not be persisted server-side yet. + let id: string | undefined; + await expect(async () => { + const response = await this.page.request.get(`/api/notebook/${noteId}`, { failOnStatusCode: false }); + if (!response.ok()) { + throw new Error(`Fetch notebook REST request failed: ${response.status()} ${await response.text()}`); + } + const json = (await response.json()) as { body?: { paragraphs?: Array<{ id?: string }> } }; + id = json.body?.paragraphs?.[paragraphIndex]?.id; + expect(id, `No paragraph at index ${paragraphIndex} in note ${noteId}`).toBeTruthy(); + }).toPass({ timeout: 10000, intervals: [200, 400, 800] }); + return id!; } // Helper methods for verifying shortcut effects diff --git a/zeppelin-web-angular/e2e/tests/notebook/keyboard/notebook-keyboard-shortcuts.spec.ts b/zeppelin-web-angular/e2e/tests/notebook/keyboard/notebook-keyboard-shortcuts.spec.ts index 2f43bb6f33f..6f3029fa51e 100644 --- a/zeppelin-web-angular/e2e/tests/notebook/keyboard/notebook-keyboard-shortcuts.spec.ts +++ b/zeppelin-web-angular/e2e/tests/notebook/keyboard/notebook-keyboard-shortcuts.spec.ts @@ -309,7 +309,9 @@ test.describe.serial('Comprehensive Keyboard Shortcuts (ShortcutsMap)', () => { const finalCount = await keyboardPage.getParagraphCount(); expect(finalCount).toBe(initialCount + 1); - // And: the new paragraph at index 0 holds no user content; empty or just an interpreter directive (poll so the async insert/render settles). + // And: the new paragraph at index 0 holds no user content; empty or just an interpreter directive. + // Render gate: an unrendered editor reads as '' and would vacuously match. + await keyboardPage.waitForEditorRendered(0); await expect.poll(() => keyboardPage.getCodeEditorContentByIndex(0).then(c => c.trim())).toMatch(/^(%\w+)?$/); // And the original content moved to index 1 (normalize whitespace; Monaco reflows). @@ -343,6 +345,8 @@ test.describe.serial('Comprehensive Keyboard Shortcuts (ShortcutsMap)', () => { expect(originalParagraphContent).toMatch(/Content\s+for\s+insert\s+below\s+test/); // And: a new paragraph exists at index 1 holding no user content. + // Render gate: an unrendered editor reads as '' and would vacuously match. + await keyboardPage.waitForEditorRendered(1); await expect.poll(() => keyboardPage.getCodeEditorContentByIndex(1).then(c => c.trim())).toMatch(/^(%\w+)?$/); }); }); @@ -354,7 +358,10 @@ test.describe.serial('Comprehensive Keyboard Shortcuts (ShortcutsMap)', () => { await keyboardPage.setCodeEditorContent('%md\n# Copy Test\nContent to be copied below'); const initialCount = await keyboardPage.getParagraphCount(); - const originalContent = await keyboardPage.getCodeEditorContentByIndex(0); + // Compare persisted server text, not the editor DOM: the clone's correctness is + // that the copied paragraph is stored identically, and reading from the server + // avoids Monaco's non-deterministic whitespace rendering. + const originalContent = await keyboardPage.getParagraphTextByIndex(0); // When: User presses Control+Shift+C await keyboardPage.pressInsertCopy(); @@ -362,8 +369,8 @@ test.describe.serial('Comprehensive Keyboard Shortcuts (ShortcutsMap)', () => { // Then: a copy is inserted below carrying the same text, and the original is unchanged await keyboardPage.waitForParagraphCountChange(initialCount + 1); expect(await keyboardPage.getParagraphCount()).toBe(initialCount + 1); - await expect.poll(() => keyboardPage.getCodeEditorContentByIndex(0)).toBe(originalContent); - await expect.poll(() => keyboardPage.getCodeEditorContentByIndex(1)).toBe(originalContent); + await expect.poll(() => keyboardPage.getParagraphTextByIndex(0)).toBe(originalContent); + await expect.poll(() => keyboardPage.getParagraphTextByIndex(1)).toBe(originalContent); }); }); @@ -389,9 +396,9 @@ test.describe.serial('Comprehensive Keyboard Shortcuts (ShortcutsMap)', () => { const paragraphCount = await keyboardPage.getParagraphCount(); expect(paragraphCount).toBe(2); - // Verify initial content before move - const initialFirst = await keyboardPage.getCodeEditorContentByIndex(0); - const initialSecond = await keyboardPage.getCodeEditorContentByIndex(1); + // Capture server-persisted text (avoids Monaco DOM whitespace non-determinism) + const initialFirst = await keyboardPage.getParagraphTextByIndex(0); + const initialSecond = await keyboardPage.getParagraphTextByIndex(1); // Focus on second paragraph for move operation await keyboardPage.tryFocusCodeEditor(1); @@ -403,9 +410,13 @@ test.describe.serial('Comprehensive Keyboard Shortcuts (ShortcutsMap)', () => { const finalParagraphCount = await keyboardPage.getParagraphCount(); expect(finalParagraphCount).toBe(2); - // And: Paragraph positions should be swapped (poll until the move lands in the DOM) - await expect.poll(() => keyboardPage.getCodeEditorContentByIndex(0)).toBe(initialSecond); - await expect.poll(() => keyboardPage.getCodeEditorContentByIndex(1)).toBe(initialFirst); + // And: positions are swapped (poll the server until the move persists) + await expect.poll(() => keyboardPage.getParagraphTextByIndex(0)).toBe(initialSecond); + await expect.poll(() => keyboardPage.getParagraphTextByIndex(1)).toBe(initialFirst); + // And the visible order reflects the swap. Containment on a distinctive marker + // proves the user-visible reorder without an exact Monaco whitespace match. + await expect(keyboardPage.getParagraphByIndex(0).locator('.view-lines')).toContainText('Second Paragraph'); + await expect(keyboardPage.getParagraphByIndex(1).locator('.view-lines')).toContainText('First Paragraph'); }); }); @@ -431,9 +442,9 @@ test.describe.serial('Comprehensive Keyboard Shortcuts (ShortcutsMap)', () => { const paragraphCount = await keyboardPage.getParagraphCount(); expect(paragraphCount).toBe(2); - // Verify initial content before move - const initialFirst = await keyboardPage.getCodeEditorContentByIndex(0); - const initialSecond = await keyboardPage.getCodeEditorContentByIndex(1); + // Capture server-persisted text (avoids Monaco DOM whitespace non-determinism) + const initialFirst = await keyboardPage.getParagraphTextByIndex(0); + const initialSecond = await keyboardPage.getParagraphTextByIndex(1); // Focus first paragraph for move operation await keyboardPage.tryFocusCodeEditor(0); @@ -445,9 +456,13 @@ test.describe.serial('Comprehensive Keyboard Shortcuts (ShortcutsMap)', () => { const finalParagraphCount = await keyboardPage.getParagraphCount(); expect(finalParagraphCount).toBe(2); - // And: Paragraph positions should be swapped (poll until the move lands in the DOM) - await expect.poll(() => keyboardPage.getCodeEditorContentByIndex(0)).toBe(initialSecond); - await expect.poll(() => keyboardPage.getCodeEditorContentByIndex(1)).toBe(initialFirst); + // And: positions are swapped (poll the server until the move persists) + await expect.poll(() => keyboardPage.getParagraphTextByIndex(0)).toBe(initialSecond); + await expect.poll(() => keyboardPage.getParagraphTextByIndex(1)).toBe(initialFirst); + // And the visible order reflects the swap. Containment on a distinctive marker + // proves the user-visible reorder without an exact Monaco whitespace match. + await expect(keyboardPage.getParagraphByIndex(0).locator('.view-lines')).toContainText('Second Paragraph'); + await expect(keyboardPage.getParagraphByIndex(1).locator('.view-lines')).toContainText('First Paragraph'); }); }); @@ -559,8 +574,11 @@ test.describe.serial('Comprehensive Keyboard Shortcuts (ShortcutsMap)', () => { const statusElBefore = keyboardPage.paragraphContainer.first().locator('.status'); await expect(statusElBefore).toHaveText(/FINISHED|ERROR|PENDING|RUNNING/); - // When: User presses Control+Alt+L (editor hidden after %md run; dispatch from the host) + // Gate: without visible output, isSettled starts true and the helper would skip the press entirely. const resultLocator = keyboardPage.getParagraphByIndex(0).locator('[data-testid="paragraph-result"]'); + await expect(resultLocator).toBeVisible(); + + // When: User presses Control+Alt+L (editor hidden after %md run; dispatch from the host) await keyboardPage.pressShortcutFromHostUntil( 0, () => keyboardPage.pressClearOutput(), @@ -932,7 +950,7 @@ test.describe.serial('Comprehensive Keyboard Shortcuts (ShortcutsMap)', () => { // Verify error result exists (invalid syntax produces a final ERROR or FINISHED with error output) // JUSTIFIED: single-paragraph test notebook; first() is deterministic const statusElError = keyboardPage.paragraphContainer.first().locator('.status'); - await expect(statusElError).toHaveText(/FINISHED|ERROR/, { timeout: 30000 }); + await expect(statusElError).toHaveText(/FINISHED|ERROR/, { timeout: 60000 }); // When: User continues with shortcuts (insert new paragraph) const initialCount = await keyboardPage.getParagraphCount(); @@ -945,11 +963,12 @@ test.describe.serial('Comprehensive Keyboard Shortcuts (ShortcutsMap)', () => { await keyboardPage.setCodeEditorContent('%md\n# Recovery Test\nShortcuts work after error', newParagraphIndex); await keyboardPage.pressRunParagraph(); - // Then: Shortcut execution still reaches a terminal state - await keyboardPage.waitForParagraphExecution(newParagraphIndex); + // Then: Shortcut execution still reaches a terminal state (real interpreter run; + // allow extra time as a cold interpreter under CI load can stay RUNNING past 30s) + await keyboardPage.waitForParagraphExecution(newParagraphIndex, 60000); // JUSTIFIED: newParagraphIndex is dynamically computed from getParagraphCount(); nth() is the only way to address this specific paragraph const statusElNew = keyboardPage.paragraphContainer.nth(newParagraphIndex).locator('.status'); - await expect(statusElNew).toHaveText(/FINISHED|ERROR/, { timeout: 30000 }); + await expect(statusElNew).toHaveText(/FINISHED|ERROR/, { timeout: 60000 }); }); test('should gracefully handle shortcuts when no paragraph is focused', async () => {