-
Notifications
You must be signed in to change notification settings - Fork 2.8k
[ZEPPELIN-6350] Fix Selenium test by Replacing Firefox with Edge #5089
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- name: Set up JDK 11 | ||
uses: actions/setup-java@v4 | ||
with: | ||
distribution: 'temurin' | ||
java-version: 11 | ||
- name: Cache local Maven repository | ||
uses: actions/cache@v4 | ||
with: | ||
path: | | ||
~/.m2/repository | ||
!~/.m2/repository/org/apache/zeppelin/ | ||
~/.spark-dist | ||
~/.cache | ||
key: ${{ runner.os }}-zeppelin-${{ hashFiles('**/pom.xml') }} | ||
restore-keys: | | ||
${{ runner.os }}-zeppelin- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reverted this part to align it with other jobs
- name: Tune Runner VM | ||
uses: ./.github/actions/tune-runner-vm | ||
- name: Set up Edge browser | ||
uses: browser-actions/setup-edge@v1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Used official browser actions to setup Edge driver.
return null; | ||
} | ||
}; | ||
Supplier<WebDriver> edgeDriverSupplier = () -> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please also update the comment at line 49
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added it, thanks!
.github/workflows/frontend.yml
Outdated
-pl zeppelin-integration \ | ||
-Pweb-classic -Pintegration -Pspark-scala-2.12 -Pspark-3.5 -Pweb-dist -Pusing-source-tree \ | ||
${MAVEN_ARGS} | ||
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 ${MAVEN_ARGS} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's wrong with the previous multi-line style?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was due to copy-pasting when reverting to the previous version, not intentional.
I've updated it back to the multi-line style as you suggested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
### What is this PR for? This PR stabilizes the `test-selenium...` job by switching the Selenium browser from Firefox to Microsoft Edge. In #4941, we worked around a Linux ChromeDriver issue (SeleniumHQ/selenium#15358) by moving from Chrome to Firefox, but that workaround also seems to be not working well. I use Edge driver instead, since it does not have flakiness like Firefox driver and is more aligned with Chrome since it is also based on Chromium. Limitation: The root cause of Firefox browser failures has not been fully analyzed. ### What type of PR is it? Bug Fix ### What is the Jira issue? [[ZEPPELIN-6350]](https://issues.apache.org/jira/browse/ZEPPELIN-6350) ### How should this be tested? - Check `test-selenium...` job in `frontend` ### Questions: * Does the license files need to update? No * Is there breaking changes for older versions? No * Does this needs documentation? No Closes #5089 from tbonelee/fix-selenium-test. Signed-off-by: Chan Lee <chanho0325@gmail.com> (cherry picked from commit 3d1ee7c) Signed-off-by: Chan Lee <chanho0325@gmail.com>
Thanks for the reviews. I've merged this into master and branch-0.12 |
What is this PR for?
This PR stabilizes the
test-selenium...
job by switching the Selenium browser from Firefox to Microsoft Edge.In #4941, we worked around a Linux ChromeDriver issue (SeleniumHQ/selenium#15358) by moving from Chrome to Firefox, but that workaround also seems to be not working well.
I use Edge driver instead, since it does not have flakiness like Firefox driver and is more aligned with Chrome since it is also based on Chromium.
Limitation: The root cause of Firefox browser failures has not been fully analyzed.
What type of PR is it?
Bug Fix
What is the Jira issue?
[ZEPPELIN-6350]
How should this be tested?
test-selenium...
job infrontend
Questions: