[ZEPPELIN-1836] Make testAngularRunParagraph wait for rendered Angular output by jongyoul · Pull Request #5250 · apache/zeppelin

jongyoul · 2026-05-17T04:36:55Z

What is this PR for?

testAngularRunParagraph has been failing on every frontend.yml test-selenium-with-spark-module-for-spark-3-5 run on master since the last green build at 6353224 on 2026-04-23. The failure is always at ZeppelinIT.java:348:

org.openqa.selenium.TimeoutException: Expected condition failed: waiting for visibility of element located by By.xpath: (//div[@ng-controller="ParagraphCtrl"])[1]//div[@id="angularRunParagraph"] (tried for 30 second(s) with 500 milliseconds interval)

Tracing the server log confirms the angular paragraph re-run completes successfully on the server side (paragraph FINISHED, result broadcast) and the stalenessOf(oldAngularDiv) check before visibilityWait does succeed — so the old angularRunParagraph div is detached as expected. The new div, however, is not detected as visible within 30s.

The race lives on the frontend side of the re-render: AngularJS' renderAngular() does elem.html(generated) and then $compile(elem.contents())(paragraphScope). Between those two calls the result div can be momentarily detached or empty, which is exactly the window visibilityWait polls into. Once it gets a stale reference, the wait does not refetch.

This builds on the stalenessOf + JavaScript-click fix from ZEPPELIN-6409 (#5209). That fix proved the old output was being torn down; this PR makes the wait for the new output equally tolerant of mid-$compile DOM churn.

What type of PR is it?

Bug Fix

Todos

Replace single visibilityWait with a content-aware polling wait that re-finds the element and ignores StaleElementReferenceException
Mirror the content-based pattern already used after the first run (waitForText(\"Run second paragraph\", ...))

What is the Jira issue?

https://issues.apache.org/jira/browse/ZEPPELIN-1836 (long-standing flaky test ticket, open since 2016)

How should this be tested?

CI frontend.yml → test-selenium-with-spark-module-for-spark-3-5 should run ZeppelinIT.testAngularRunParagraph to completion without timing out at line 348.

Local repro (slow, requires -Pweb-classic and Spark 3.5):

./mvnw verify -DfailIfNoTests=false -pl zeppelin-integration \
  -Pweb-classic -Pintegration -Pspark-scala-2.12 -Pspark-3.5 -Pweb-dist -Pusing-source-tree \
  -Dit.test=ZeppelinIT#testAngularRunParagraph

Screenshots (if appropriate)

N/A

Questions:

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

…r output After the rerun, visibilityWait alone races against AngularJS $compile — the result div can briefly detach or render empty before settling, so the wait times out at 30s on master frontend.yml runs. Replace the single visibilityWait with a polling lambda that re-finds the element each iteration, tolerates StaleElementReferenceException, and only returns once the div is displayed *and* shows the expected text. This mirrors the content-based pattern already used after the first run. Builds on the stalenessOf + JS click fix from ZEPPELIN-6409. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…lar paragraph The 4ms FINISHED on the rerun + missing replacement div pointed at the paragraph executing with stale (or empty) text: ACE's setValue does not fire the 'input' event that paragraph.controller.js binds aceChanged to, so paragraph.text never commits. If the angular paragraph's editor was closed after the first run, $scope.editor is falsy and getEditorValue() falls back to paragraph.text — sending the old body to the interpreter, which echoes back an empty/stale ANGULAR result. That explains the stalenessOf success followed by no new div. Force the commit by writing the new text directly onto the paragraph scope, syncing the ACE buffer when present, and running $apply. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…on race The previous scope-commit attempt still saw the rerun finish in ~3ms with no new div in 30s. That matches a stale/empty paragraph.text reaching the backend, not just a render race: * setTextOfParagraph(1, ...) clicks `icon-size-fullscreen`. After the first %angular run the paragraph has editorHide=true, so this toggle re-mounts the ACE editor; ace.edit(id).setValue() races with that re-mount. * ACE setValue() does not fire the editor's 'input' event, so the binding $scope.editor.on('input', aceChanged) never runs and paragraph.text is never committed. * runParagraphFromButton calls getEditorValue(), which returns paragraph.text when $scope.editor is falsy (between teardown and the new binding from the toggle). The backend then receives the previous body — or an empty one — and the AngularInterpreter echoes back identical/empty ANGULAR data. elem.html(empty) detaches the old div (stalenessOf passes) while leaving no replacement, which is exactly the observed timeline. Drive the rerun through the controller's own runParagraph(text, ...) so the new text is delivered no matter what the editor is doing. Tighten the new-render wait to also require the ng-click attribute, so a stale-text echo cannot satisfy the condition. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

jongyoul · 2026-07-22T08:31:27Z

@voidmatcha @tbonelee Does it make sense?

Copilot

Pull request overview

Note

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

This PR re-enables and hardens the flaky ZeppelinIT.testAngularRunParagraph Selenium integration test by avoiding ACE editor races and making Angular output waits tolerant of DOM churn during $compile.

Changes:

Re-enable testAngularRunParagraph and update the paragraph rerun flow to set Angular paragraph text via controller scope + JS execution.
Replace a single visibility wait with a polling wait that re-finds the element and ignores StaleElementReferenceException.
Make authenticationUser more resilient by handling modal timing and using an Angular model readiness wait before input.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.

File	Description
zeppelin-integration/src/test/java/org/apache/zeppelin/integration/ZeppelinIT.java	Re-enables the Angular run test and makes the rerun + re-render wait robust against AngularJS DOM churn.
zeppelin-integration/src/test/java/org/apache/zeppelin/AbstractZeppelinIT.java	Improves login modal handling and inputs by waiting for Angular `ngModel` readiness and doing a single-task JS update/click.

Files not reviewed (1)

zeppelin-web-angular/projects/zeppelin-react/package-lock.json: Generated file

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

+    WebElement loginModal = manager.getWebDriver().findElement(By.id("loginModal"));
+    if (!loginModal.isDisplayed()) {


+      ((JavascriptExecutor) manager.getWebDriver()).executeScript(
+              "var els = document.querySelectorAll('div[ng-controller=\"ParagraphCtrl\"]');"
+                      + "var s = angular.element(els[0]).scope();"
+                      + "s.paragraph.text = arguments[0];"
+                      + "if (s.editor) { s.editor.setValue(arguments[0], 1); s.editor.clearSelection(); }"
+                      + "s.runParagraph(arguments[0], true, false);",
+              newAngularText);


+                return el.isDisplayed()
+                        && "Run second paragraph".equals(el.getText())
+                        && ngClick != null && ngClick.contains("z.runParagraph")
+                        ? el : null;


tbonelee

Diagnosis and fix both look right. There's probably a cleaner path, but for test code against the legacy classic-web UI, landing it at "passes reliably" is fine by me.

Minor: the fast-uri lock bump looks accidental, worth double-checking.

voidmatcha

Approve 👍

The testAngularRunParagraph fix is the real win. Re-enabling the test and waiting on the actual rendered output properly closes the render race.

Scope note: this also hardens the shared authenticationUser(), but the target test runs in ZeppelinIT (auth off, no login). That method is only used by the shiro-enabled AuthenticationIT / PersonalizeActionsIT / InterpreterModeActionsIT, whose login flakiness #5335 fixes at the root via REST, dropping this method from the call path. Both are good; cleaner to let #5335 own login. After merge, I'll leave a note on #5335 to track the overlap.

tbonelee · 2026-07-23T01:53:40Z

Merged to master

### What is this PR for? `test-selenium-with-spark-module-for-spark-3-5` fails on ~88% of master runs. Every failure lands in `AbstractZeppelinIT.authenticationUser`: the Bootstrap 3 login modal races its fade animation and the WebSocket `session_logout` auto-open, so the login click gets intercepted, the field is not interactable, or the modal never reaches its shown state. Across eight failing master runs the exception rotates between those three, which is why hardening any single step never held on its own. Since that helper is the shared login precondition for `AuthenticationIT`, `PersonalizeActionsIT` and `InterpreterModeActionsIT`, one racy modal destabilizes the whole job. Test-only, no production changes. - Add `authenticationUserViaRest`, which logs in with a synchronous XHR `POST /classic/api/login` issued from inside the browser's own JS context, so the browser session itself becomes authenticated. Reloading afterwards lets `app.js`'s pre-bootstrap `GET /security/ticket` render the logged-in state. No modal opens, so none of its races apply. Used wherever login is only a precondition (35 call sites across the three ITs). - Keep `authenticationUser` as hardened by ZEPPELIN-1836, and keep `AuthenticationIT.testSimpleAuthentication` on it, so the classic login modal keeps its automated coverage. - Add `extractNoteIdFromCurrentUrl`, which strips any `?query`/`#fragment` after the last path segment. After a reload the SPA appends `?ref=...` to note URLs, which otherwise leaks into the extracted note id and breaks the `<at>href` lookups. - Correct `PersonalizeActionsIT.testDynamicFormAction` (see below). An earlier approach injected a JSESSIONID obtained from a separate `java.net.http` client via `addCookie`; that session never transferred to the browser — the server logged a successful login while the browser stayed anonymous. Issuing the request inside the page authenticates the real session, which the server logs confirm (`LoginRestApi` returns the expected principal and `SecurityRestApi` then reports it for the browser). Rebased onto current `master` so it sits on top of #5250. ### Why `testDynamicFormAction` changes The test asserted that a non-owner's dynamic-form edit is ignored (`Status: Before`). The server does the opposite: `NotebookService.runParagraph` applies the caller's params to that user's own paragraph copy, `Note.run` executes that copy, and `GUI.textbox` prefers the params value over the default. Nothing filters a non-owner's params. The same wrong expectation was already corrected for `testGraphAction` in 238e032 (ZEPPELIN-4355), which also added the owner-side check that proves isolation; the dynamic-form case was missed at the time and still has no such step. The assertion has not been revisited since 2017, and its message (`"The output of graph mode is changed"`) is a leftover from the graph test. It passed until now only because a late WebSocket broadcast replaces `paragraph.settings` and reverts the typed form value before the run. The test now waits for the input to actually hold the new value, so that race fails loudly instead of passing silently. Adding the owner-side isolation step would fail on a real production bug — in personalized mode the caller's params are written to the shared master paragraph before the personalized branch is taken, and then persisted. Filed separately as ZEPPELIN-6556; not addressed here. ### Scope and related issues - **ZEPPELIN-6525, login modal** — fixed, though not the way the ticket proposed. Driving the modal defensively (retrying open/fill/submit, tolerating `ElementClickInterceptedException`) was tried first and made things worse: one error became six, and ignoring the interception threw away the diagnostic the raw exception carried. Taking the modal out of the precondition path addresses the class of failure rather than one instance of it. - **ZEPPELIN-6525, `ParagraphActionsIT` dynamic-form sleeps** — out of scope here. `ParagraphActionsIT` reports `Tests run: 21, Failures: 0, Errors: 0` on master and on this branch, so it is not what keeps the job red. Those fixed sleeps remain a latent race and deserve a follow-up. - **ZEPPELIN-6526, REST session instead of per-test UI logins** — implemented, with one deviation: the proposed `addCookie` injection does not work, for the reason above, so the request is issued from inside the page instead. The `?ref=` risk the ticket flagged did materialise and is handled by `extractNoteIdFromCurrentUrl`. - **ZEPPELIN-6556** — production isolation bug found while investigating this; filed, not fixed here. ### What type of PR is it? Bug Fix (test infrastructure) ### What is the ticket? https://issues.apache.org/jira/browse/ZEPPELIN-6525 ### How should this be tested? Re-run `test-selenium-with-spark-module-for-spark-3-5`. Locally: ``` ZEPPELIN_SELENIUM_BROWSER=chrome ./mvnw -o failsafe:integration-test failsafe:verify \ -pl zeppelin-integration -Dit.test=AuthenticationIT -DfailIfNoTests=false \ -Pintegration -Pusing-source-tree -Pweb-classic -Pspark-scala-2.12 -Pspark-3.5 ``` `AuthenticationIT` (3/3, including the modal-based `testSimpleAuthentication`) and `PersonalizeActionsIT#testDynamicFormAction` pass locally. ### Questions - Does this need documentation? No. Closes #5335 from HwangRock/ZEPPELIN-6525. Signed-off-by: Jongyoul Lee <jongyoul@gmail.com>

jongyoul force-pushed the ZEPPELIN-selenium-angular-paragraph branch from 60c3d75 to 92b58d2 Compare May 18, 2026 13:51

jongyoul force-pushed the ZEPPELIN-selenium-angular-paragraph branch from 92b58d2 to 50f4c82 Compare July 13, 2026 03:24

jongyoul and others added 4 commits July 22, 2026 14:25

[ZEPPELIN-1836] Re-enable angular paragraph integration test

17b30a1

jongyoul force-pushed the ZEPPELIN-selenium-angular-paragraph branch from aea7c5e to 17b30a1 Compare July 22, 2026 05:27

jongyoul added 2 commits July 22, 2026 14:32

[HOTFIX] Update fast-uri for npm audit

d0db646

[HOTFIX] Stabilize Selenium authentication login

f561b7f

jongyoul force-pushed the ZEPPELIN-selenium-angular-paragraph branch from 7e22091 to f561b7f Compare July 22, 2026 06:49

jongyoul marked this pull request as ready for review July 22, 2026 08:30

Copilot AI review requested due to automatic review settings July 22, 2026 08:30

Copilot AI reviewed Jul 22, 2026

View reviewed changes

Copilot started reviewing on behalf of jongyoul July 22, 2026 08:55

View session

tbonelee approved these changes Jul 22, 2026

View reviewed changes

voidmatcha approved these changes Jul 23, 2026

View reviewed changes

tbonelee merged commit a619089 into apache:master Jul 23, 2026
22 checks passed

voidmatcha mentioned this pull request Jul 23, 2026

[ZEPPELIN-6525] Authenticate via REST for Selenium login preconditions #5335

Merged

Navigation Menu

Repository navigation

[ZEPPELIN-1836] Make testAngularRunParagraph wait for rendered Angular output - #5250

[ZEPPELIN-1836] Make testAngularRunParagraph wait for rendered Angular output#5250
tbonelee merged 6 commits into
apache:masterfrom
jongyoul:ZEPPELIN-selenium-angular-paragraph

jongyoul commented May 17, 2026

Uh oh!

jongyoul commented Jul 22, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

tbonelee left a comment

Uh oh!

voidmatcha left a comment

Uh oh!

Uh oh!

tbonelee commented Jul 23, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		WebElement loginModal = manager.getWebDriver().findElement(By.id("loginModal"));
		if (!loginModal.isDisplayed()) {

Conversation

jongyoul commented May 17, 2026

What is this PR for?

What type of PR is it?

Todos

What is the Jira issue?

How should this be tested?

Screenshots (if appropriate)

Questions:

Uh oh!

jongyoul commented Jul 22, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

tbonelee left a comment

Choose a reason for hiding this comment

Uh oh!

voidmatcha left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tbonelee commented Jul 23, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants