Conversation
…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.
|
Note) CI ran the job this PR targets, and the run reached the bad state rather than passing quietly.
I would not read this as proof. Master forces Being a flake, the job is worth watching over the next few runs. |
What is this PR for?
AuthenticationIT.testSimpleAuthenticationis the only test left that drives the login modal, and it fails intermittently intest-selenium-with-spark-module-for-spark-3-5. The click on the navbar user menu is intercepted by#loginModal, which is still displayed:authenticationUserdid not wait for the modal at all. Once the navbar dropdown appeared it removed the backdrop, forcedmodal('hide')and slept 500 ms. The classic UI closes the modal itself —login.controller.jscallsmodal('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:
#loginModalto become invisible inauthenticationUser, 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.logoutUserimmediately before opening the user menu, and retries that one click if it is still intercepted.testSimpleAuthentication, which namedtestCreateNewButton.Test-only. No production code changes.
Why
logoutUserneeds its own waitWaiting once in
authenticationUseris not sufficient, because the modal can be legitimately re-opened after it was closed:NotebookServer.onMessageanswers a WebSocket message whose ticket does not match the one on file by sendingOP.SESSION_LOGOUT(NotebookServer.java:362).login.controller.js:64reacts tosession_logoutby callingmodal('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:
The next test in the same run is the contrast: after
finance1logs in throughauthenticationUserViaRest, the log showsWebSocket ticket request completed: principal=finance1followed 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:
NotebookServerlogs the mismatch at DEBUG and the job runs at INFO, so the log does not show whether aSESSION_LOGOUTwas 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_SECis 30 s.logoutUserhas 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_SECis 10 s, generous for a modal the application closes on an HTTP response, andMODAL_CLEANUP_TIMEOUT_SECis 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.
logoutUserhas around 40 callers and all but one log in throughauthenticationUserViaRest, which refreshes the page and never touches the modal.invisibilityOfElementLocatedreturns 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 -Pintegration—BUILD SUCCESS, andtestCompilerecompiled the module's sources.checkstyle-fail-buildand 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.Not done:
safaridriver --enableplus 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.test-selenium-with-spark-module-for-spark-3-5several times and confirmtestSimpleAuthenticationno 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:
🤖 Generated with Claude Code