Skip to content

[ZEPPELIN-6711] Wait for the login modal to close before driving the navbar - #5497

Open
kimyenac wants to merge 1 commit into
apache:masterfrom
kimyenac:ZEPPELIN-6711
Open

kimyenac wants to merge 1 commit into
apache:masterfrom
kimyenac:ZEPPELIN-6711

Conversation

@kimyenac

Copy link
Copy Markdown
Contributor

What is this PR for?

AuthenticationIT.testSimpleAuthentication is the only test left that drives the login modal, and it fails intermittently in test-selenium-with-spark-module-for-spark-3-5. The click on the navbar user menu is intercepted by #loginModal, which is still displayed:

org.openqa.selenium.ElementClickInterceptedException: element click intercepted:
  Element <li style="margin-left: 10px">...</li> is not clickable at point (1858, 21).
  Other element would receive the click:
  <div id="loginModal" class="modal fade ng-scope in" role="dialog" style="display: block;">
	at org.apache.zeppelin.AbstractZeppelinIT.logoutUser(AbstractZeppelinIT.java:173)
	at org.apache.zeppelin.integration.AuthenticationIT.testSimpleAuthentication(AuthenticationIT.java:101)

authenticationUser did not wait for the modal at all. Once the navbar dropdown appeared it removed the backdrop, forced modal('hide') and slept 500 ms. The classic UI closes the modal itself — login.controller.js calls modal('toggle') when the login request succeeds — so that cleanup raced the application instead of waiting for it, and it left no trace when the modal was in fact still up.

This PR:

  • Waits for #loginModal to become invisible in authenticationUser, and keeps the forced cleanup, unchanged, as the fallback for when that wait times out. The fallback, and a modal still displayed after it, are now logged, so a future failure says which case it hit instead of only surfacing as an intercepted click.
  • Waits for the modal in logoutUser immediately before opening the user menu, and retries that one click if it is still intercepted.
  • Fixes the error message in testSimpleAuthentication, which named testCreateNewButton.

Test-only. No production code changes.

Why logoutUser needs its own wait

Waiting once in authenticationUser is not sufficient, because the modal can be legitimately re-opened after it was closed:

  • NotebookServer.onMessage answers a WebSocket message whose ticket does not match the one on file by sending OP.SESSION_LOGOUT (NotebookServer.java:362).
  • login.controller.js:64 reacts to session_logout by calling modal('show') inside a $timeout(..., 1000), guarded by $rootScope.userName !== '' — true exactly after a successful login.

The failing run's log is consistent with this. The WebSocket opened before the login stays open across it and no new ticket is requested:

04:49:28,276  Open connection to /0:0:0:0:0:0:0:1:43492
04:49:29,197  Login request completed: principal=admin, success=true
04:49:31,352  ERROR AbstractZeppelinIT - Exception in AuthenticationIT ...
04:49:31,479  Closed connection to /0:0:0:0:0:0:0:1:43492 (1001)

The next test in the same run is the contrast: after finance1 logs in through authenticationUserViaRest, the log shows WebSocket ticket request completed: principal=finance1 followed by a new connection. Only the modal login path keeps a connection established under the previous ticket.

To be clear about the limit of that evidence: NotebookServer logs the mismatch at DEBUG and the job runs at INFO, so the log does not show whether a SESSION_LOGOUT was actually emitted. The mechanism is established from the code and the timing fits, but the emission itself is unverified. I have left the same note on the Jira issue.

Questions a reviewer may have

Why two new timeout constants instead of MAX_BROWSER_TIMEOUT_SEC?
MAX_BROWSER_TIMEOUT_SEC is 30 s. logoutUser has around 40 callers, so a 30 s wait on a path that is only reached when something is already wrong makes a bad run much slower. MODAL_CLOSE_TIMEOUT_SEC is 10 s, generous for a modal the application closes on an HTTP response, and MODAL_CLEANUP_TIMEOUT_SEC is 2 s, enough for a 300 ms Bootstrap fade — there is no reason to wait 10 s more just to log a warning. Happy to fold them into the existing constant if you would rather have fewer knobs.

Does this slow down the other tests?
No. logoutUser has around 40 callers and all but one log in through authenticationUserViaRest, which refreshes the page and never touches the modal. invisibilityOfElementLocated returns immediately when the element is hidden or absent, which is the normal case. The wait only costs time when the modal is genuinely up — where the old code failed outright.

Why is the retry only around the first click?
An intercepted click never reached the user menu, so the dropdown is still closed and opening it again is safe. Retrying the whole open-menu-then-logout sequence would not be: if the second click were intercepted, the dropdown would already be open, and clicking the user menu again would close it and leave the logout link unclickable. The sleeps and the logout click are therefore left exactly as they were, and only the first click is wrapped.

Why keep the forced modal('hide') at all?
It is the existing behaviour and it is the only lever left if the application does not close the modal. It is now a fallback rather than the first move, and it is byte-identical to what is on master — the only behavioural change in that path is that we wait first.

Should the WebSocket reconnect be fixed instead?
Arguably the classic UI should re-request the WebSocket ticket after a modal login, the way the REST path effectively does. That is a production change and the issue puts it out of scope, so I have not touched it; it seems worth a separate issue if you agree.

What type of PR is it?

Bug Fix

Todos

None.

What is the Jira issue?

How should this be tested?

Done:

  • ./mvnw install -DskipTests -am -pl zeppelin-integration -Pweb-classic -PintegrationBUILD SUCCESS, and testCompile recompiled the module's sources.
  • Checked that the changed lines add no Checkstyle violations. The module is not wired into checkstyle-fail-build and carries pre-existing violations, so I compared the reported lines against the lines this PR touches rather than the module total: zero on changed lines.
  • Read the failing job's log and the code paths quoted above rather than assuming the cause.

Not done:

  • I have not run the Selenium suite. This machine has no Chrome, Firefox or Edge — only Safari, which would need safaridriver --enable plus a manual Develop-menu step, and the suite runs headed. The flake is also specific to the CI browser, so a Safari run would not be evidence. CI is the real check here.
  • The issue's verification step is to re-run test-selenium-with-spark-module-for-spark-3-5 several times and confirm testSimpleAuthentication no longer fails this way. Being a flake, one green run does not prove much; it is worth watching the job over the next few runs. For reference, between 2026-09-13 and 2026-09-17 the issue reports 13 runs failing this way, and I hit it again on 2026-09-22.

Screenshots (if appropriate)

N/A

Questions:

  • Does the license files need to update? No. No files added or removed; both touched files keep their ASF headers.
  • Is there breaking changes for older versions? No. Test-only.
  • Does this needs documentation? No.

🤖 Generated with Claude Code

…navbar

AuthenticationIT.testSimpleAuthentication is the only test left that drives
the login modal, and it fails intermittently in
test-selenium-with-spark-module-for-spark-3-5: the click on the navbar user
menu is intercepted by #loginModal, which is still displayed.

authenticationUser did not wait for the modal at all. It removed the backdrop
and forced modal('hide') right after the navbar dropdown appeared, then slept
500 ms. The classic UI closes the modal itself - login.controller.js calls
modal('toggle') when the login request succeeds - so that forced cleanup races
the application instead of waiting for it, and it left no trace when the modal
was in fact still up.

Wait for the modal to become invisible first, and keep the forced cleanup
unchanged as a fallback for when that wait times out. The fallback, and a modal
still displayed after it, are now logged, so a future failure says which case it
hit instead of only surfacing as an intercepted click.

The modal can also come back after it was closed, so waiting once in
authenticationUser is not enough on its own: NotebookServer sends SESSION_LOGOUT
when a WebSocket message carries a stale ticket, and login.controller.js answers
that by re-opening the modal one second later, guarded by userName != '' - true
exactly after a login. logoutUser therefore waits for the modal immediately
before it opens the user menu, and retries that one click if it is still
intercepted; an intercepted click never reached the menu, so the dropdown is
still closed and opening it again is safe.

logoutUser has around 40 callers, all but one of which log in through
authenticationUserViaRest and never touch the modal. Its sleeps and its logout
click are left exactly as they were so their timing does not change; this only
adds the guard. The wait returns immediately when the modal is absent or hidden,
which is the normal case.

Also fix the error message in testSimpleAuthentication, which named
testCreateNewButton.
@kimyenac

kimyenac commented Sep 22, 2026

Copy link
Copy Markdown
Contributor Author

Note) CI ran the job this PR targets, and the run reached the bad state rather than passing quietly.

test-selenium-with-spark-module-for-spark-3-5 passed
(run 35708748063),
AuthenticationIT at Tests run: 3, Failures: 0, Errors: 0. The new warning fired:

09:21:19,313  INFO  LoginRestApi:245 - Login request completed: principal=admin, success=true
09:21:29,761  WARN  AbstractZeppelinIT:121 - Login modal still displayed after 10s, taking it down from the page
09:21:31,455  INFO  LoginRestApi:278 - {"status":"UNAUTHORIZED","message":"","body":{"clearAuthorizationHeader":"true"}}

#loginModal was still displayed ten seconds after a successful login. the state behind the ElementClickInterceptedException here. The wait timed out, the fallback cleared it, and the logout went through. The other two new log lines did not appear, so the forced cleanup worked and the retry was not needed.

I would not read this as proof. Master forces modal('hide') ten seconds earlier, while the application is still acting on the login, and whether that happens to work is the race itself, so I cannot say master would have failed this particular run. Worth noting though: a modal still up after ten seconds is not an unfinished 300 ms fade, which fits a re-opened modal (the SESSION_LOGOUT path on the Jira issue) more than one stuck mid-toggle.

Being a flake, the job is worth watching over the next few runs.

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.

1 participant