[ZEPPELIN-6358] Remove anti-patterns of E2E and tidy test suite by jongyoul · Pull Request #5207 · apache/zeppelin

jongyoul · 2026-04-04T12:19:04Z

Summary

Cherry-pick of [ZEPPELIN-6358] Remove anti-patterns of E2E and tidy test suite #5180 for branch-0.12
Remove anti-patterns of E2E and tidy test suite

Test plan

CI pipeline (frontend.yml) E2E tests

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)

dididy · 2026-04-04T14:37:49Z

The failures are occurring in the React layer of the micro-frontend, and they appear to be flaky. I’ll investigate the root cause and share findings and potential fixes in the draft. (If needed, I’ll follow up with an additional PR)

Also, I noticed several audit issues in the zeppelin-react-related packages. I’ll create a separate PR to address them all at once.

jongyoul · 2026-04-05T00:56:59Z

@dididy You can freely modiry and/or create a your own PR. I just wanted to adopt it to branch-0.12. Could you please help to handle it? 🙏

dididy · 2026-04-10T15:34:20Z

@jongyoul

What's happening

The test should render React micro-frontend instead of Angular result component in published-paragraph.spec.ts fails on every retry (5/5). It's waiting for [data-testid="react-published-paragraph"] or .ant-alert, but neither ever shows up.

The ?react=true variant of the confirmation modal test is also affected.

Why

This PR cherry-picks the E2E tidy commit from master, but the tests reference the React micro-frontend feature added in ZEPPELIN-6371. That commit was never cherry-picked to branch-0.12, so zeppelin-react/ doesn't exist here. the tests have nothing to assert against.

https://github.com/jongyoul/zeppelin/tree/ZEPPELIN-6358-e2e-tidy-branch-0.12/zeppelin-web-angular/projects
-> Here there is no zeppelin-react, but master has it.

Options

Cherry-pick 4bde6b27 (ZEPPELIN-6371) into branch-0.12 too
Drop the React-specific tests from this PR (the feature isn't in 0.12) - dididy@a73137d

jongyoul · 2026-04-11T01:13:30Z

@dididy Thanks for the detailed analysis! I went with Option 2 and cherry-picked your commit a73137d to drop the React-specific tests, since ZEPPELIN-6371 is a large feature that shouldn't be pulled into branch-0.12 just to satisfy tests.

dididy · 2026-04-11T01:49:17Z

I checked the same workflow(frontend/run-playwright-e2e-tests) in my fork, and it completed successfully:

https://github.com/dididy/zeppelin/actions/runs/24250787247

It looks like this might be a transient issue with the CI environment. Could you please try re-running the job?

tbonelee · 2026-04-11T04:02:38Z

I triggered a re-run for failed jobs. So let's take a look.

Copilot

Pull request overview

This PR refactors the zeppelin-web-angular Playwright E2E suite to remove “always passing” assertions / silent skips, improve readiness checks and selectors, and reduce flakiness; it also adds a stable data-testid hook in the notebook action bar and tunes Playwright CI parallelism/retries.

Changes:

Replace non-failing assertions and conditional expect() patterns with assertions that can genuinely fail (and explicit test.skip() where appropriate).
Improve E2E robustness: better app “ready” waits, more locator-based waits instead of page.evaluate/querySelector, and add state cleanup (e.g., closing dropdowns).
CI-oriented Playwright config tweaks (retries/workers) and removal of duplicated/unused E2E files.

Reviewed changes

Copilot reviewed 35 out of 35 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
zeppelin-web-angular/src/app/pages/workspace/notebook/action-bar/action-bar.component.html	Adds `data-testid` for stable notebook title selection in tests
zeppelin-web-angular/playwright.config.js	Adjusts retries/workers based on `CI`
zeppelin-web-angular/e2e/utils.ts	Hardens “ready” waits and replaces DOM-eval waits with locator waits in a few places
zeppelin-web-angular/e2e/tests/workspace/workspace-main.spec.ts	Simplifies workspace checks; asserts meaningful rendered content
zeppelin-web-angular/e2e/tests/workspace/notebook-repos/notebook-repos-page-structure.spec.ts	Replaces non-failing count assertions with content-based checks
zeppelin-web-angular/e2e/tests/workspace/notebook-repos/notebook-repo-item-workflow.spec.ts	Makes edit/save workflow assertions more meaningful (but see review comment)
zeppelin-web-angular/e2e/tests/workspace/notebook-repos/notebook-repo-item-settings.spec.ts	Refines settings table assertions; uses locators/skip conditions
zeppelin-web-angular/e2e/tests/workspace/notebook-repos/notebook-repo-item-form-validation.spec.ts	Improves edit-mode validation checks using `locator.or()`
zeppelin-web-angular/e2e/tests/workspace/notebook-repos/notebook-repo-item-edit.spec.ts	Removes redundant edit-mode tests
zeppelin-web-angular/e2e/tests/workspace/notebook-repos/notebook-repo-item-display.spec.ts	Strengthens edit button assertions
zeppelin-web-angular/e2e/tests/theme/dark-mode.spec.ts	Skips WebKit due to crash; refactors persistence flow
zeppelin-web-angular/e2e/tests/share/note-import/note-import-modal.spec.ts	Uses direct locator assertions for tab state and button enablement
zeppelin-web-angular/e2e/tests/share/note-create/note-create-modal.spec.ts	Uses `data-testid` title hook and more reliable home readiness waits
zeppelin-web-angular/e2e/tests/share/node-list/node-list-functionality.spec.ts	Improves node list assertions; seeds notes when needed (but see review comment)
zeppelin-web-angular/e2e/tests/share/about-zeppelin/about-zeppelin-modal.spec.ts	Strengthens modal content assertions and verifies image load via `naturalWidth`
zeppelin-web-angular/e2e/tests/notebook/published/published-paragraph.spec.ts	Replaces brittle modal selectors and timing assumptions; better “ready” signals
zeppelin-web-angular/e2e/tests/home/home-page-notebook-actions.spec.ts	Removes weak assertions; adds crash-guard test for special character filtering
zeppelin-web-angular/e2e/tests/home/home-page-note-operations.spec.ts	Seeds a note for deterministic operations coverage (but see review comment)
zeppelin-web-angular/e2e/tests/home/home-page-layout.spec.ts	Removes low-signal test; adds readable heading assertions
zeppelin-web-angular/e2e/tests/home/home-page-external-links.spec.ts	Tightens docs URL matching to versioned docs pattern
zeppelin-web-angular/e2e/tests/home/home-page-enhanced-functionality.spec.ts	Deletes redundant/duplicative spec file
zeppelin-web-angular/e2e/tests/home/home-page-elements.spec.ts	Removes redundant tests; consolidates coverage and improves assertions
zeppelin-web-angular/e2e/tests/authentication/anonymous-login-redirect.spec.ts	Replaces boolean helpers with direct visible-content assertions
zeppelin-web-angular/e2e/tests/app.spec.ts	Replaces always-passing spinner count checks with post-ready expectations
zeppelin-web-angular/e2e/models/workspace-page.util.ts	Removes unused WorkspaceUtil helper
zeppelin-web-angular/e2e/models/workspace-page.ts	Removes unused WorkspacePage model
zeppelin-web-angular/e2e/models/notebook.util.ts	Replaces DOM-eval waits with URL + locator waits
zeppelin-web-angular/e2e/models/notebook-repos-page.ts	Removes unused methods; simplifies setting value retrieval
zeppelin-web-angular/e2e/models/notebook-repo-item.util.ts	Uses class assertions instead of `evaluate()` for edit mode
zeppelin-web-angular/e2e/models/note-import-modal.ts	Removes thin wrapper helpers; relies on locator assertions in tests
zeppelin-web-angular/e2e/models/note-create-modal.ts	Removes thin wrapper helper
zeppelin-web-angular/e2e/models/node-list-page.ts	Removes unused visibility helpers; restricts internal locator helper visibility
zeppelin-web-angular/e2e/models/home-page.ts	Refactors navigation + input behavior (but see review comment)
zeppelin-web-angular/e2e/models/base-page.ts	Updates label wait and input fill helper (but see review comment)
zeppelin-web-angular/e2e/models/about-zeppelin-modal.ts	Removes thin wrapper helper

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

zeppelin-web-angular/e2e/models/home-page.ts

+    await this.page.goto('/');
+    await this.waitForPageLoad();


zeppelin-web-angular/e2e/tests/home/home-page-note-operations.spec.ts

+    testNoteName = `_e2e_ops_test_${Date.now()}`;
+
    await page.goto('/#/');
    await waitForZeppelinReady(page);
    await performLoginIfRequired(page);
-    const noteListLocator = page.locator('zeppelin-node-list');
-    await expect(noteListLocator).toBeVisible({ timeout: 15000 });
-  });

-  test.describe('Given note operations are available', () => {
-    test('When note list loads Then should show note action buttons on hover', async ({ page }) => {
-      const notesExist = await page.locator('.node .file').count();
+    // Create a test note so all operation tests have a real target
+    await homePage.createNote(testNoteName);
+    await page.goto('/#/');


zeppelin-web-angular/e2e/tests/workspace/notebook-repos/notebook-repo-item-workflow.spec.ts

      if (isInputVisible) {
-        const originalValue = await repoItemPage.getSettingInputValue(settingName);
-        await repoItemPage.fillSettingInput(settingName, originalValue || 'test-value');
+        savedValue = (await repoItemPage.getSettingInputValue(settingName)) || 'test-value';


zeppelin-web-angular/e2e/models/base-page.ts

  async fillAndVerifyInput(
    locator: Locator,
    value: string,
    options?: { timeout?: number; clearFirst?: boolean }
  ): Promise<void> {
-    const { timeout = 10000, clearFirst = true } = options || {};
+    const { timeout = 10000 } = options || {};



zeppelin-web-angular/e2e/tests/share/node-list/node-list-functionality.spec.ts

+    if (notes.length === 0) {
+      // Seed a note so the test always runs — critical navigation path must not be skipped
+      await homePage.createNote(`_e2e_nav_${Date.now()}`);
+      await page.goto('/');
+      await waitForZeppelinReady(page);
+      notes = await nodeListPage.getAllVisibleNoteNames();


dididy · 2026-04-12T08:07:36Z

Here are some fixes I found while investigating CI flaky tests. Verified on my fork.
CI passes: https://github.com/dididy/zeppelin/actions/runs/24300567108

93b6f1e: fix flaky E2E: remove error-swallowing .catch() in waitForParagraphExecution + use toHaveText auto-retry for paragraph status assertion
1741c51: apply Copilot review: hash routing consistency, unused type cleanup, meaningful save verification
e928883: fix flaky clearNewName (use key events instead of clear()) + use waitForParagraphExecution for rapid run test

remove zeppelin-react tests of e2e

932a145

jongyoul marked this pull request as ready for review April 11, 2026 11:28

Copilot AI review requested due to automatic review settings April 11, 2026 11:28

Copilot started reviewing on behalf of jongyoul April 11, 2026 11:28 View session

Copilot AI reviewed Apr 11, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

[ZEPPELIN-6358] Remove anti-patterns of E2E and tidy test suite#5207
jongyoul wants to merge 2 commits intoapache:branch-0.12from
jongyoul:ZEPPELIN-6358-e2e-tidy-branch-0.12

jongyoul commented Apr 4, 2026

Uh oh!

dididy commented Apr 4, 2026

Uh oh!

jongyoul commented Apr 5, 2026

Uh oh!

dididy commented Apr 10, 2026

Uh oh!

jongyoul commented Apr 11, 2026

Uh oh!

dididy commented Apr 11, 2026

Uh oh!

tbonelee commented Apr 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

dididy commented Apr 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

jongyoul commented Apr 4, 2026

Summary

Test plan

Uh oh!

dididy commented Apr 4, 2026

Uh oh!

jongyoul commented Apr 5, 2026

Uh oh!

dididy commented Apr 10, 2026

What's happening

Why

Options

Uh oh!

jongyoul commented Apr 11, 2026

Uh oh!

dididy commented Apr 11, 2026

Uh oh!

tbonelee commented Apr 11, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

dididy commented Apr 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants