[ZEPPELIN-6527] Fix flaky run-e2e-tests-in-zeppelin-web CI job (searchBlock Protractor timing)

[miinhho]

What is this PR for?

This PR removes the flaky classic zeppelin-web Protractor e2e job by moving its remaining active coverage to the existing Playwright e2e workflow.

The old run-e2e-tests-in-zeppelin-web job depended on Protractor, webdriver-manager, and a pinned ChromeDriver 2.35, and had been failing intermittently in the classic search/replace tests due to AngularJS/Ace synchronization timing.

Since that stack is end-of-life and cannot be reasonably modernized in place, this PR ports the 10 active checks from home.spec.js and searchBlock.spec.js to Playwright, pointed at the classic /classic UI.

The migrated tests keep the existing classic UI coverage for the home page, interpreter permission save flow, search shortcut, match counters, replace, and replace-all behavior. After that coverage is available in Playwright, the obsolete Protractor config, scripts, dependencies, and CI job are removed.

What type of PR is it?

Improvement

What is the Jira issue?

https://issues.apache.org/jira/browse/ZEPPELIN-6527

How should this be tested?

cd zeppelin-web-angular
npx playwright test e2e/tests/classic --project=classic --reporter=line

For the Maven/CI path:

./mvnw clean install -DskipTests -am -pl python,zeppelin-jupyter-interpreter,zeppelin-web,zeppelin-web-angular -Pweb-classic
xvfb-run --auto-servernum --server-args="-screen 0 1024x768x24" ./mvnw verify -pl zeppelin-web-angular -Pweb-e2e

Questions:

• Does the license files need to update? No
• Is there breaking changes for older versions? No
• Does this needs documentation? Edit zeppelin-web/README.md to reflect migrated classic web e2e test

[voidmatcha]

Nice work dropping Protractor and moving the classic UI checks to Playwright. The ported cases match the originals closely, and retiring the flaky run-e2e-tests-in-zeppelin-web job is a good cleanup.

A few things I'd tweak. I pushed them as separate commits on my fork so they're easy to cherry-pick or diff against.

1. Make the base URL overridable via PLAYWRIGHT_BASE_URL. Right now the config computes it into a local variable, so there's no way to point the run elsewhere. Collapsing it to one env var keeps local and CI consistent.
   voidmatcha@7059e995d

2. Run the classic suite through an e2e:classic script plus E2E_CLASSIC. npm run e2e -- --project=classic leans on fragile classic-only detection, so an explicit env var and npm script make the :8080 run unambiguous.
   voidmatcha@96ce23558

3. Document the classic conventions in e2e/AGENTS.md: that it's a frozen legacy surface, the locator exceptions the classic templates need, gating readiness on a classic signal instead of waitForZeppelinReady, the coverage exemption, how to run it, and the anonymous-only CI scoping.
   voidmatcha@3b894aaea

4. Skip the classic project in the auth CI leg. The original Protractor suite only ran anonymous, so scoping classic to the anonymous leg via E2E_MODE matches that.
   voidmatcha@0b5b83309

Full diff: voidmatcha/zeppelin@8597767...review/pr-5295-fixes

One note: the red CI here isn't from your PR. It's the new-UI keyboard-shortcut spec being flaky, and I'm tracking that one separately: https://issues.apache.org/jira/browse/ZEPPELIN-6536

[voidmatcha]

.

[miinhho]

Thanks for the detailed review and for splitting the suggested fixes into separate commits. 😄 🙇‍♂️

I applied the changes for the Playwright base URL override, the explicit classic e2e script, the classic test conventions docs, and the auth-matrix classic skip.

[voidmatcha]

Since collaborativeMode.spec.js was fully commented out and deleted rather than ported, could we leave a visible follow-up near the migrated classic specs?

For example, a skipped placeholder in zeppelin-web-angular/e2e/tests/classic/classic-collaborative-mode.spec.ts with a named TODO to port the old disabled collaborative editing scenario later.

The deleted file already pointed to https://issues.apache.org/jira/browse/ZEPPELIN-5674, so we could reference that there as well.

voidmatcha@12b4edb

Not a blocker, but it would make the migration more traceable.

[voidmatcha]

countSubstringOccurrence() uses new RegExp(substring, 'g'). Current terms are plain (t, te), but literal counting would be safer for future regex metacharacter cases.

Suggested change

	const countSubstringOccurrence = (text: string, substring: string): number =>
	(text.match(new RegExp(substring, 'g')) || []).length;
	const countSubstringOccurrence = (text: string, substring: string): number =>
	substring ? text.split(substring).length - 1 : 0;

[voidmatcha]

Could you rebase this onto master?

[miinhho]

I resolved the conflict via GitHub, which updated the branch with master and made it mergeable. I realized afterward that this produced a merge commit rather than a rebase.

Happy to redo it as a local rebase and force-push if you prefer the linear history.

[voidmatcha]

The removed run-e2e-tests-in-zeppelin-web job was the only CI path running the zeppelin-web Karma unit tests: -Pweb-e2e un-skips the npm run karma-test execution in zeppelin-web/pom.xml. The classic Playwright suite replaces the e2e coverage, but the 18 unit specs (e.g. the tabledata/advanced-transformation-util parsers) now run nowhere.

In case it helps, I sketched one possible way to restore them on my fork — feel free to use it as a reference:

• voidmatcha/zeppelin@3384d6c52 — web-unit-test profile + a one-line CI step in the anonymous Playwright leg (reuses the installed node_modules; verified green on a fork run)
• voidmatcha/zeppelin@57f721ecd — minor classic-spec convention alignment

[miinhho]

Thanks for catching this. I applied both suggested changes.

The zeppelin-web Karma unit coverage is now restored through a separate web-unit-test profile, and the frontend workflow runs it in the anonymous Playwright CI leg.

I also applied the classic spec convention cleanup by moving the interpreter permission case into classic-interpreter.spec.ts, switching the collaborative follow-up placeholder to test.fixme, and updating the classic locator guidance.

[voidmatcha]

LGTM. To check stability, I ran a 10x CI flake loop on a fork with this PR merged together with my keyboard-shortcut stabilization branch (#5304) (runs). The playwright legs, including the restored Karma step and the classic suite, passed in 9 of 10 runs (example job); the one failure was a transient WebKit crash in test setup (microsoft/playwright#34450), now handled in #5304.

One optional follow-up: the classic gating (E2E_CLASSIC env + argv sniffing in playwright.config.js) could move to a dedicated config, sketched in voidmatcha/zeppelin@ebcd133f3 and verified on a fork run (anonymous e2e job: main suite fully passed, then classic 11 passed via the new path). Concrete gains: a plain npm run e2e no longer fails without a -Pweb-classic server, the argv-based classic detection (which can misfire on arguments like --grep tests/classic) goes away, and the coverage reporter stops running for classic pages that sit outside the PAGES denominator. Feel free to adopt it into this PR; if not, I'll open it as a separate PR after this merges.

[miinhho]

Thanks for the stability check and for sketching the dedicated classic config path.

Since it’s optional and a bit more than a small cleanup, I’d prefer to leave it for a follow-up PR so this migration can stay focused.