Skip to content

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

Merged
jongyoul merged 11 commits intoapache:masterfrom
dididy:tidy-e2e
Apr 4, 2026
Merged

[ZEPPELIN-6358] Remove anti-patterns of E2E and tidy test suite#5180
jongyoul merged 11 commits intoapache:masterfrom
dididy:tidy-e2e

Conversation

@dididy
Copy link
Copy Markdown
Contributor

@dididy dididy commented Mar 13, 2026

What is this PR for?

Applied the e2e-reviewer 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()

What type of PR is it?

Improvement
Refactoring

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

@jongyoul jongyoul merged commit 076676a into apache:master Apr 4, 2026
20 checks passed
@jongyoul
Copy link
Copy Markdown
Member

jongyoul commented Apr 4, 2026

Merged into master (076676a).

jongyoul pushed a commit that referenced this pull request Apr 4, 2026
### What is this PR for?
This is the final PR in the series derived from #5101.

Notebook features had zero E2E coverage. This adds 20 spec files (~3500 lines).

**Notebook core**
- `notebook-container` — structure, action bar presence, sidebar width constraints, paragraph grid layout, extension area
- `action-bar-functionality` — run all, code/output toggle, clear output, clone/export/reload, collaboration mode, revision controls, scheduler, settings group
- `notebook-keyboard-shortcuts` — full ShortcutsMap coverage (Monaco editor; serial because Monaco holds focus state between tests — isolating via `beforeEach` wasn't viable)
- `sidebar-functionality` — TOC panel, file tree panel, open/close state transitions
- `paragraph-functionality` — edit mode, run/cancel, dynamic forms, footer DOM presence

**Share features**
- `folder-rename` — hover context menu, rename modal, validation, delete confirmation, folder merge on name collision
- `note-rename` — inline title editing, enter/blur/escape flows, empty name rejection, special characters
- `note-toc` — panel open/close, empty state message, toggle button attributes, repeated toggle

#### Pulled in test failure fixes from #5180                             
- Cleaned up `about-zeppelin-modal` and `note-create-modal` specs and models
- Added missing aria attributes and `data-testid` selectors to `action-bar.component.html`
- Bumped `flatted` 3.3.3 → 3.4.1 (npm audit)


### What type of PR is it?
Improvement
Feature
Documentation

### 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


Closes #5181 from dididy/e2e/notebook-final.

Signed-off-by: Jongyoul Lee <jongyoul@gmail.com>
jongyoul pushed a commit that referenced this pull request Apr 4, 2026
### What is this PR for?
This is the final PR in the series derived from #5101.

Notebook features had zero E2E coverage. This adds 20 spec files (~3500 lines).

**Notebook core**
- `notebook-container` — structure, action bar presence, sidebar width constraints, paragraph grid layout, extension area
- `action-bar-functionality` — run all, code/output toggle, clear output, clone/export/reload, collaboration mode, revision controls, scheduler, settings group
- `notebook-keyboard-shortcuts` — full ShortcutsMap coverage (Monaco editor; serial because Monaco holds focus state between tests — isolating via `beforeEach` wasn't viable)
- `sidebar-functionality` — TOC panel, file tree panel, open/close state transitions
- `paragraph-functionality` — edit mode, run/cancel, dynamic forms, footer DOM presence

**Share features**
- `folder-rename` — hover context menu, rename modal, validation, delete confirmation, folder merge on name collision
- `note-rename` — inline title editing, enter/blur/escape flows, empty name rejection, special characters
- `note-toc` — panel open/close, empty state message, toggle button attributes, repeated toggle

#### Pulled in test failure fixes from #5180
- Cleaned up `about-zeppelin-modal` and `note-create-modal` specs and models
- Added missing aria attributes and `data-testid` selectors to `action-bar.component.html`
- Bumped `flatted` 3.3.3 → 3.4.1 (npm audit)

### What type of PR is it?
Improvement
Feature
Documentation

### 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

Closes #5181 from dididy/e2e/notebook-final.

Signed-off-by: Jongyoul Lee <jongyoul@gmail.com>
(cherry picked from commit 5e9847b)
Signed-off-by: Jongyoul Lee <jongyoul@gmail.com>
jongyoul pushed a commit to jongyoul/zeppelin that referenced this pull request Apr 4, 2026
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 apache#5180 from dididy/tidy-e2e.

Signed-off-by: Jongyoul Lee <jongyoul@gmail.com>
(cherry picked from commit 076676a)
jongyoul pushed a commit to jongyoul/zeppelin that referenced this pull request Apr 16, 2026
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 apache#5180 from dididy/tidy-e2e.

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