[ZEPPELIN-6661] Cover notebook editor save timing

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

@@ -22,6 +22,7 @@ export class NotebookKeyboardPage extends BasePage {
   readonly codeEditor: Locator;
   readonly paragraphContainer: Locator;
   readonly firstParagraph: Locator;
+  readonly firstEditorInput: Locator;
   readonly runButton: Locator;
   readonly paragraphResult: Locator;
   readonly newParagraphButton: Locator;
@@ -45,6 +46,7 @@ export class NotebookKeyboardPage extends BasePage {
     this.codeEditor = page.locator('.monaco-editor .monaco-mouse-cursor-text');
     this.paragraphContainer = page.locator('zeppelin-notebook-paragraph');
     this.firstParagraph = this.paragraphContainer.first();
+    this.firstEditorInput = this.firstParagraph.locator('.monaco-editor textarea.inputarea');
     this.runButton = page.locator('button[title="Run this paragraph"], button:has-text("Run")');
     this.paragraphResult = page.locator(PARAGRAPH_RESULT_SELECTOR);
     this.newParagraphButton = page.locator('button:has-text("Add Paragraph"), .new-paragraph-button');

zeppelin-web-angular/e2e/models/notebook-save-timing.util.ts

@@ -0,0 +1,189 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { expect, Page, WebSocketRoute } from '@playwright/test';
+
+const IDLE_SAVE_TIMEOUT_MS = 30000;
+const PROXY_TIMEOUT_MS = 15000;
+const COMMIT_STABILITY_MS = 2000;
+const ZEPPELIN_WS_URL_PATTERN = /\/ws(\?|$)/;
+
+interface NotebookSocketMessage {
+  op?: string;
+  msgId?: string;
+  data?: {
+    id?: string;
+    noteId?: string;
+    paragraph?: string | Record<string, unknown>;
+    status?: boolean;
+  };
+}
+
+interface CommitParagraphMessage extends NotebookSocketMessage {
+  op: 'COMMIT_PARAGRAPH';
+  msgId: string;
+  data: {
+    id: string;
+    noteId: string;
+    paragraph: string;
+  };
+}
+
+export class CommitParagraphSocketProbe {
+  private shouldHoldFirstCommitResponse = false;
+  private shouldQueueServerMessages = false;
+  private heldResponseMsgId: string | null = null;
+  private collaborativeModeSignal: string | null = null;
+  private readonly commits: CommitParagraphMessage[] = [];
+  private readonly forwardedResponseMsgIds: string[] = [];
+  private readonly heldResponses = new Map<string, { socket: WebSocketRoute; message: string | Buffer }>();
+  private readonly queuedResponses: Array<{ socket: WebSocketRoute; message: string | Buffer }> = [];
+
+  handleClientMessage(server: WebSocketRoute, message: string | Buffer): void {
+    const parsed = parseSocketMessage(message);
+    if (parsed?.op === 'PATCH_PARAGRAPH') {
+      this.collaborativeModeSignal ??= 'client sent PATCH_PARAGRAPH';
+    }
+    if (isCommitParagraphMessage(parsed)) {
+      this.commits.push(parsed);
+      if (this.shouldHoldFirstCommitResponse && this.heldResponseMsgId === null) {
+        this.heldResponseMsgId = parsed.msgId;
+      }
+    }
+    server.send(message);
+  }
+
+  handleServerMessage(socket: WebSocketRoute, message: string | Buffer): void {
+    const parsed = parseSocketMessage(message);
+    if (parsed?.op === 'COLLABORATIVE_MODE_STATUS' && parsed.data?.status === true) {
+      this.collaborativeModeSignal ??= 'server sent COLLABORATIVE_MODE_STATUS with status true';
+    }
+    if (parsed?.op === 'PARAGRAPH' && parsed.msgId) {
+      if (parsed.msgId === this.heldResponseMsgId && !this.heldResponses.has(parsed.msgId)) {
+        this.heldResponses.set(parsed.msgId, { socket, message });
+        this.shouldQueueServerMessages = true;
+        return;
+      }
+    }
+    // Preserve server order while the delayed response is delivered and observed in isolation.
+    if (this.shouldQueueServerMessages) {
+      this.queuedResponses.push({ socket, message });
+      return;
+    }
+    if (parsed?.op === 'PARAGRAPH' && parsed.msgId) {
+      this.forwardedResponseMsgIds.push(parsed.msgId);
+    }
+    socket.send(message);
+  }
+
+  holdFirstCommitParagraphResponse(): void {
+    this.shouldHoldFirstCommitResponse = true;
+  }
+
+  async waitForCommitCount(expectedCount: number): Promise<CommitParagraphMessage[]> {
+    await expect
+      .poll(() => this.collaborativeModeSignal !== null || this.commits.length >= expectedCount, {
+        timeout: IDLE_SAVE_TIMEOUT_MS
+      })
+      .toBe(true);
+    if (this.collaborativeModeSignal !== null) {
+      throw new Error(
+        `Notebook entered collaborative mode (${this.collaborativeModeSignal}); edits are sent as PATCH_PARAGRAPH, so no COMMIT_PARAGRAPH will arrive`
+      );
+    }
+    return this.commits.slice();
+  }
+
+  async waitForHeldResponse(msgId: string): Promise<void> {
+    await expect.poll(() => this.heldResponses.has(msgId), { timeout: PROXY_TIMEOUT_MS }).toBe(true);
+  }
+
+  async waitForForwardedResponse(msgId: string): Promise<void> {
+    await expect
+      .poll(() => this.forwardedResponseCount(msgId), { timeout: PROXY_TIMEOUT_MS })
+      .toBeGreaterThanOrEqual(1);
+  }
+
+  async expectCommitCountToStay(expectedCount: number): Promise<void> {
+    const stableUntil = Date.now() + COMMIT_STABILITY_MS;
+    await expect
+      .poll(() => (this.commits.length !== expectedCount || Date.now() >= stableUntil ? this.commits.length : null), {
+        timeout: PROXY_TIMEOUT_MS
+      })
+      .toBe(expectedCount);
+  }
+
+  commitCount(): number {
+    return this.commits.length;
+  }
+
+  forwardedResponseCount(msgId: string): number {
+    return this.forwardedResponseMsgIds.filter(forwardedMsgId => forwardedMsgId === msgId).length;
+  }
+
+  releaseHeldResponseWithParagraphTitle(msgId: string, title: string): void {
+    const held = this.heldResponses.get(msgId);
+    if (!held) {
+      throw new Error(`No held PARAGRAPH response for msgId ${msgId}`);
+    }
+    const parsed = parseSocketMessage(held.message);
+    if (parsed?.op !== 'PARAGRAPH' || typeof parsed.data?.paragraph !== 'object') {
+      throw new Error(`Held response for msgId ${msgId} is not a PARAGRAPH snapshot`);
+    }
+    // The held response only echoes the committed text, so a marker title lets the test observe when it is applied.
+    const paragraph = parsed.data.paragraph;
+    const config = typeof paragraph.config === 'object' && paragraph.config !== null ? paragraph.config : {};
+    paragraph.title = title;
+    paragraph.config = { ...config, title: true };
+
+    this.heldResponses.delete(msgId);
+    this.heldResponseMsgId = null;
+    this.shouldHoldFirstCommitResponse = false;
+    this.forwardedResponseMsgIds.push(msgId);
+    held.socket.send(JSON.stringify(parsed));
+  }
+
+  releaseQueuedResponses(): void {
+    this.shouldQueueServerMessages = false;
+    for (const queued of this.queuedResponses.splice(0)) {
+      this.handleServerMessage(queued.socket, queued.message);
+    }
+  }
+}
+
+export const installCommitParagraphProbe = async (page: Page): Promise<CommitParagraphSocketProbe> => {
+  const probe = new CommitParagraphSocketProbe();
+  await page.routeWebSocket(ZEPPELIN_WS_URL_PATTERN, socket => {
+    const server = socket.connectToServer();
+    socket.onMessage(message => probe.handleClientMessage(server, message));
+    server.onMessage(message => probe.handleServerMessage(socket, message));
+  });
+  return probe;
+};
+
+const parseSocketMessage = (message: string | Buffer): NotebookSocketMessage | null => {
+  try {
+    return JSON.parse(message.toString()) as NotebookSocketMessage;
+  } catch {
+    return null;
+  }
+};
+
+const isCommitParagraphMessage = (message: NotebookSocketMessage | null): message is CommitParagraphMessage => {
+  return (
+    message?.op === 'COMMIT_PARAGRAPH' &&
+    typeof message.msgId === 'string' &&
+    typeof message.data?.id === 'string' &&
+    typeof message.data.noteId === 'string' &&
+    typeof message.data.paragraph === 'string'
+  );
+};

zeppelin-web-angular/e2e/scenarios/notebook-parity.json

@@ -514,9 +514,14 @@
         "runner": "not-applicable"
       },
       "coverage": {
-        "status": "gap",
-        "tests": [],
-        "issues": ["ZEPPELIN-6661"],
+        "status": "covered",
+        "tests": [
+          {
+            "path": "zeppelin-web-angular/e2e/tests/notebook/persistence/notebook-save-timing.spec.ts",
+            "tag": "@NB-PARITY-050"
+          }
+        ],
+        "issues": [],
         "uncoveredOutcomes": []
       },
       "implementationEvidence": [
@@ -529,6 +534,10 @@
         {
           "path": "zeppelin-web-angular/e2e/models/notebook-keyboard-page.ts",
           "symbol": "NotebookKeyboardPage"
+        },
+        {
+          "path": "zeppelin-web-angular/e2e/models/notebook-save-timing.util.ts",
+          "symbol": "CommitParagraphSocketProbe"
         }
       ]
     },
@@ -565,9 +574,14 @@
         "runner": "not-applicable"
       },
       "coverage": {
-        "status": "gap",
-        "tests": [],
-        "issues": ["ZEPPELIN-6661"],
+        "status": "covered",
+        "tests": [
+          {
+            "path": "zeppelin-web-angular/e2e/tests/notebook/persistence/notebook-save-timing.spec.ts",
+            "tag": "@NB-PARITY-051"
+          }
+        ],
+        "issues": [],
         "uncoveredOutcomes": []
       },
       "implementationEvidence": [
@@ -580,7 +594,12 @@
           "symbol": "NotebookComponent"
         }
       ],
-      "verificationEvidence": []
+      "verificationEvidence": [
+        {
+          "path": "zeppelin-web-angular/e2e/models/notebook-save-timing.util.ts",
+          "symbol": "CommitParagraphSocketProbe"
+        }
+      ]
     },
     {
       "id": "NB-PARITY-060",

zeppelin-web-angular/e2e/scenarios/notebook-parity.md

@@ -33,8 +33,8 @@ Coverage note: `covered` mechanically means this registry points to a matching e
 | NB-PARITY-011 | editor | The second Escape after inline completion dismissal blurs the editor | covered | not-applicable | zeppelin-web-angular/e2e/tests/notebook/inline-completion.spec.ts<br>@NB-PARITY-011 |  |
 | NB-PARITY-021 | result | Text and table result displays preserve output semantics after paragraph execution | partial | owner: allow<br>writer: allow<br>reader: deny<br>runner: allow | zeppelin-web-angular/e2e/tests/notebook/paragraph/paragraph-functionality.spec.ts<br>@NB-PARITY-021 | ZEPPELIN-6514, ZEPPELIN-6516 |
 | NB-PARITY-022 | result | Streaming interpreter output accumulates while a paragraph is running | covered | owner: allow<br>writer: allow<br>reader: deny<br>runner: allow | zeppelin-web-angular/e2e/tests/notebook/paragraph/paragraph-functionality.spec.ts<br>@NB-PARITY-022 |  |
-| NB-PARITY-050 | persistence | Notebook editor persists the latest text after typing stops | gap | owner: allow<br>writer: allow<br>reader: deny<br>runner: not-applicable |  | ZEPPELIN-6661 |
-| NB-PARITY-051 | persistence | Notebook editor does not lose an edit made while a prior save is in flight | gap | owner: allow<br>writer: allow<br>reader: deny<br>runner: not-applicable |  | ZEPPELIN-6661 |
+| NB-PARITY-050 | persistence | Notebook editor persists the latest text after typing stops | covered | owner: allow<br>writer: allow<br>reader: deny<br>runner: not-applicable | zeppelin-web-angular/e2e/tests/notebook/persistence/notebook-save-timing.spec.ts<br>@NB-PARITY-050 |  |
+| NB-PARITY-051 | persistence | Notebook editor does not lose an edit made while a prior save is in flight | covered | owner: allow<br>writer: allow<br>reader: deny<br>runner: not-applicable | zeppelin-web-angular/e2e/tests/notebook/persistence/notebook-save-timing.spec.ts<br>@NB-PARITY-051 |  |
 | NB-PARITY-060 | theme | Notebook honors host theme selection | gap | not-applicable |  | ZEPPELIN-6640 |
 
 ## Scenario Details
@@ -151,26 +151,26 @@ Coverage note: `covered` mechanically means this registry points to a matching e
 ### NB-PARITY-050 Notebook editor persists the latest text after typing stops
 
 - Area: persistence
-- Coverage: gap
+- Coverage: covered
 - Interpreter: not-applicable
 - Role verification: owner: unverified; writer: unverified; reader: unverified; runner: not-applicable
 - Preconditions: A disposable notebook with one editable paragraph is open. The user can edit the paragraph.
 - Action: Replace the paragraph text and stop typing long enough for the notebook save path to acknowledge the edit.
 - Observable outcomes: NB-PARITY-050-OUTCOME-001: The persisted paragraph text equals the latest typed text. NB-PARITY-050-OUTCOME-002: The save assertion is based on observable persistence or wire evidence, not an internal timer.
 - Implementation evidence: zeppelin-web-angular/src/app/pages/workspace/notebook/paragraph/code-editor/code-editor.component.ts (NotebookParagraphCodeEditorComponent)
-- Verification evidence: zeppelin-web-angular/e2e/models/notebook-keyboard-page.ts (NotebookKeyboardPage)
+- Verification evidence: zeppelin-web-angular/e2e/models/notebook-keyboard-page.ts (NotebookKeyboardPage); zeppelin-web-angular/e2e/models/notebook-save-timing.util.ts (CommitParagraphSocketProbe)
 
 ### NB-PARITY-051 Notebook editor does not lose an edit made while a prior save is in flight
 
 - Area: persistence
-- Coverage: gap
+- Coverage: covered
 - Interpreter: not-applicable
 - Role verification: owner: unverified; writer: unverified; reader: unverified; runner: not-applicable
 - Preconditions: A disposable notebook with one editable paragraph is open. The first paragraph save request can be observed before it completes.
 - Action: Edit the paragraph, keep the first save in flight, then make a second edit.
 - Observable outcomes: NB-PARITY-051-OUTCOME-001: The first in-flight save does not overwrite or drop the second edit. NB-PARITY-051-OUTCOME-002: A later observable save or reconciliation persists the second edit.
 - Implementation evidence: zeppelin-web-angular/src/app/pages/workspace/notebook/paragraph/code-editor/code-editor.component.ts (NotebookParagraphCodeEditorComponent); zeppelin-web-angular/src/app/pages/workspace/notebook/notebook.component.ts (NotebookComponent)
-- Verification evidence: not-applicable
+- Verification evidence: zeppelin-web-angular/e2e/models/notebook-save-timing.util.ts (CommitParagraphSocketProbe)
 
 ### NB-PARITY-060 Notebook honors host theme selection