Skip to content

Conversation

@dididy
Copy link
Contributor

@dididy dididy commented Oct 11, 2025

What is this PR for?

Addition and improvement of notebook-related E2E tests for New UI


/#/notebook/:noteId – View or edit a specific notebook
/#/notebook/:noteId/revision/:revisionId – View a specific revision of a notebook
/#/notebook/:noteId/paragraph/:paragraphId – Notebook paragraph presentation mode

PAGES.WORKSPACE.NOTEBOOK
→ src/app/pages/workspace/notebook/notebook.component

PAGES.WORKSPACE.NOTEBOOK_ACTION_BAR
→ src/app/pages/workspace/notebook/action-bar/action-bar.component

PAGES.WORKSPACE.NOTEBOOK_PARAGRAPH
→ src/app/pages/workspace/notebook/paragraph/paragraph.component

PAGES.WORKSPACE.NOTEBOOK_SIDEBAR
→ src/app/pages/workspace/notebook/sidebar/sidebar.component

PAGES.WORKSPACE.PUBLISHED_PARAGRAPH
→ src/app/pages/workspace/published/paragraph/paragraph.component

What type of PR is it?

Improvement

Todos

What is the Jira issue?

ZEPPELIN-6358

How should this be tested?

Screenshots (if appropriate)

Questions:

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

@dididy dididy force-pushed the e2e/notebook branch 8 times, most recently from d6228ac to b07b933 Compare October 19, 2025 05:34
@tbonelee
Copy link
Contributor

Could you rebase this onto master branch?

Copy link
Contributor

@tbonelee tbonelee left a comment

Choose a reason for hiding this comment

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

Could you take a quick pass on two areas?

  1. There are places where try/catch accepts both error and non-error paths. Could we make these more explicit, either intentionally silencing with a clear rationale or failing, so we reduce false positives?
  2. For browser-specific branches (e.g., notebook-keyboard-page.ts → executePlatformShortcut), could we extract that logic so the main flow reads sequentially and the differences are isolated?

@dididy dididy force-pushed the e2e/notebook branch 4 times, most recently from d058c16 to 9bd2f40 Compare October 27, 2025 10:13
@dididy
Copy link
Contributor Author

dididy commented Oct 27, 2025

You can see that request item 1 has been addressed in e92d5ae and item 2 in eea335a. Thank you for the thorough review.

@dididy dididy force-pushed the e2e/notebook branch 13 times, most recently from 1dc6ffa to f4f51b1 Compare October 30, 2025 15:57
@dididy dididy force-pushed the e2e/notebook branch 8 times, most recently from 50eed38 to 07b58b4 Compare December 21, 2025 13:08
dididy added a commit to dididy/zeppelin that referenced this pull request Dec 21, 2025
dididy added a commit to dididy/zeppelin that referenced this pull request Dec 21, 2025
dididy added a commit to dididy/zeppelin that referenced this pull request Dec 21, 2025
dididy added a commit to dididy/zeppelin that referenced this pull request Dec 21, 2025
@dididy dididy force-pushed the e2e/notebook branch 3 times, most recently from f069171 to 1d263ec Compare December 22, 2025 03:12
dididy added a commit to dididy/zeppelin that referenced this pull request Dec 22, 2025
@dididy dididy force-pushed the e2e/notebook branch 2 times, most recently from ba47403 to 3f5582c Compare December 22, 2025 11:41
@dididy
Copy link
Contributor Author

dididy commented Dec 22, 2025

It took some time to incorporate the review feedback, do incremental refactoring, and clean up the tests that were failing in CI.
I’ve opened #5128 with the common environment and utility changes. Once that PR is merged, I’ll follow up with the PRs below:

  • A PR reflecting changes to existing test POMs, utils, and specs
  • A PR for keyboard shortcut–related tests
  • A PR for the remaining notebook-related tests
  • A PR for tests related to shared modules (folder-rename, note-rename, note-toc)

Once all of those PRs are merged, we can close the current PR accordingly.

@dididy dididy force-pushed the e2e/notebook branch 2 times, most recently from 375098e to 4993ecd Compare December 23, 2025 00:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants