You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@zeppelin.apache.org by "HwangRock (via GitHub)" <gi...@apache.org> on 2026/07/21 13:29:40 UTC
[PR] [ZEPPELIN-6525] Harden Selenium login modal against animation click-interception flakiness [zeppelin]
HwangRock opened a new pull request, #5335:
URL: https://github.com/apache/zeppelin/pull/5335
### What is this PR for?
The `test-selenium-with-spark-module-for-spark-3-5` CI job fails on nearly every master run. The failures all originate in `AbstractZeppelinIT.authenticationUser`: the login modal is driven while its Bootstrap fade-in animation is still running, so the navbar logo intercepts the submit click (`ElementClickInterceptedException`) or the username field is not yet visible (`TimeoutException`). Since `authenticationUser` is the shared login precondition for `AuthenticationIT` and `PersonalizeActionsIT`, this single race destabilizes the whole job.
This PR rewrites `authenticationUser` defensively (test-only, no production or CI changes):
- Neutralize CSS transitions/animations up front so the modal never sits at intermediate coordinates while it fades in — this removes the root cause of the click interception, mirroring how GitLab disables animations for its QA runs.
- Wait for the Bootstrap fade-complete state (`.in`) before touching inputs, and use `elementToBeClickable` for the username/password fields.
- Verify each field value after `sendKeys`, retrying if focus was momentarily lost while the modal settled.
- Submit through a `FluentWait` that ignores `ElementClickInterceptedException` and `StaleElementReferenceException` (the canonical Selenium retry pattern), guarding against re-submitting an already-successful login.
- Replace the brittle exact class-string success locator (`nav-btn dropdown-toggle ng-scope`) with an order-agnostic partial match, extracted to a shared `loggedInUserMenuLocator()` reused by `AuthenticationIT`.
The `JavascriptExecutor` forced-click anti-pattern is intentionally avoided — the fix keeps the real user click path intact and only removes the animation race around it.
### 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 the `test-selenium-with-spark-module-for-spark-3-5` job. Because the failure is flaky, it should stay green across several consecutive runs. Locally:
```
xvfb-run --auto-servernum --server-args="-screen 0 1600x1024x16" \
./mvnw verify -DfailIfNoTests=false -pl zeppelin-integration \
-Pweb-classic -Pintegration -Pspark-scala-2.12 -Pspark-3.5 -Pweb-dist -Pusing-source-tree \
-Dtest=AuthenticationIT,PersonalizeActionsIT
```
### Questions
- Does this need documentation? No.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ZEPPELIN-6525] Authenticate via REST for Selenium login preconditions [zeppelin]
Posted by "HwangRock (via GitHub)" <gi...@apache.org>.
HwangRock closed pull request #5335: [ZEPPELIN-6525] Authenticate via REST for Selenium login preconditions
URL: https://github.com/apache/zeppelin/pull/5335
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ZEPPELIN-6525] Authenticate via REST for Selenium login preconditions [zeppelin]
Posted by "HwangRock (via GitHub)" <gi...@apache.org>.
HwangRock commented on PR #5335:
URL: https://github.com/apache/zeppelin/pull/5335#issuecomment-5056873826
Thanks for the detailed resolution — made it really easy to follow!
Applied it: `authenticationUser()` untouched, used only by `testSimpleAuthentication`, the other 30 call sites on `authenticationUserViaRest()`. And good catch on the Playwright claim — you were right, it only covers the new UI. Fixed the description. CI is green, selenium included 🎉
One thing caught my eye while I was in there, and you'd know it better than anyone.
`run-playwright-e2e-tests (auth)` is 53m and drives the whole hour-long `frontend` workflow. Of the five browser projects in `playwright.config.js`, three are Chromium — the bundled build plus the `chrome` and `msedge` channels. ZEPPELIN-6314 sets the goal as "Chromium / Firefox / WebKit", and nothing in `e2e/` touches either channel; every browser branch is `webkit` or `firefox`. Dropping the two would leave engine coverage untouched and cut that job to ~27m — half the critical path from a config-only change!
WebKit stays, so ZEPPELIN-6450 is unaffected. JupyterLab runs Chromium only and marimo has WebKit commented out, so three engines is already generous.
Were the channels deliberate? Happy to open a ticket and PR if not 🙂
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ZEPPELIN-6525] Authenticate via REST for Selenium login preconditions [zeppelin]
Posted by "jongyoul (via GitHub)" <gi...@apache.org>.
jongyoul commented on PR #5335:
URL: https://github.com/apache/zeppelin/pull/5335#issuecomment-5065558379
Merged into master (a31975a0).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ZEPPELIN-6525] Stabilize Selenium login by authenticating via REST [zeppelin]
Posted by "voidmatcha (via GitHub)" <gi...@apache.org>.
voidmatcha commented on PR #5335:
URL: https://github.com/apache/zeppelin/pull/5335#issuecomment-5054080246
Thanks @HwangRock 🙏 Now that #5250 is merged, `master` conflicts with this PR in `AbstractZeppelinIT.authenticationUser()` — #5250 hardened that method, while this PR replaces it with `authenticationUserViaRest()`. Could you rebase onto latest `master` and resolve it like this:
- **Keep the REST login** — it's the right root-cause fix for the precondition call sites, so take this PR's side there.
- **Don't delete `authenticationUser()`.** Keep it and have only `AuthenticationIT.testSimpleAuthentication` still use it, since #5250's hardened version is now the only automated coverage of the classic login modal (the Playwright suite only exercises the new UI login).
So the end state: `authenticationUser()` stays, used only by `testSimpleAuthentication`; everything else uses REST. That clears the conflict and keeps both fixes. If you'd rather drop classic-modal coverage entirely, let me know.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ZEPPELIN-6525] Authenticate via REST for Selenium login preconditions [zeppelin]
Posted by "jongyoul (via GitHub)" <gi...@apache.org>.
jongyoul merged PR #5335:
URL: https://github.com/apache/zeppelin/pull/5335
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org