Skip to content

[SPARK-32957][INFRA] Add a GitHub Actions job to run WebUI tests with Chrome #29827

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.

Sign up for GitHub

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

Closed
sarutak wants to merge 3 commits into from
Closed

[SPARK-32957][INFRA] Add a GitHub Actions job to run WebUI tests with Chrome #29827

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1c9af36
Add a job to GitHub Actions workflow which runs WebUI tests with Chrome.
sarutak Sep 21, 2020
5206561
Fixed the prefix for "restore-keys".
sarutak Sep 22, 2020
cb6aef4
Removed the task to upload "test-reports".
sarutak Sep 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions .github/workflows/build_and_test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,44 @@ jobs:
cd docs
jekyll build

webui-tests-with-chrome:
name: WebUI tests with chrome
runs-on: ubuntu-latest
steps:
- name: Checkout Spark repository
uses: actions/checkout@v2
- name: Cache Maven local repository
uses: actions/cache@v2
with:
path: ~/.m2/repository
key: webui-tests-with-chrome-maven-${{ hashFiles('**/pom.xml') }}
restore-keys: |
webui-tests-with-chrome-maven-
- name: Install Java 11
uses: actions/setup-java@v1
with:
java-version: 11
- name: Install Chrome and ChromeDriver
run: |
sudo apt update
sudo apt install google-chrome-stable
sudo apt install chromium-chromedriver
- name: Run WebUI tests with Maven
run: |
Copy link
Member

@HyukjinKwon HyukjinKwon Sep 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think ideally we should just include ChromeUITesttag in core above without changing the guava version ...

Copy link
Member Author

@sarutak sarutak Sep 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I tried to do so first but I noticed those tests need the newer Guava so I made a separate job...

Copy link
Member Author

@sarutak sarutak Sep 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@HyukjinKwon I've tried including ChromeUITest tag in core and running GitHub Actions jobs on my repository but as I expected, tests failed due to the Guava version compatibility.

[info] org.apache.spark.deploy.history.ChromeUIHistoryServerSuite *** ABORTED *** (7 milliseconds)
[info]   java.lang.NoSuchMethodError: com.google.common.base.Preconditions.checkState(ZLjava/lang/String;Ljava/lang/Object;Ljava/lang/Object;Ljava/lang/Object;)V
[info]   at org.openqa.selenium.remote.service.DriverService.findExecutable(DriverService.java:134)
[info]   at org.openqa.selenium.chrome.ChromeDriverService.access$000(ChromeDriverService.java:35)
[info]   at org.openqa.selenium.chrome.ChromeDriverService$Builder.findDefaultExecutable(ChromeDriverService.java:159)
[info]   at org.openqa.selenium.remote.service.DriverService$Builder.build(DriverService.java:355)
[info]   at org.openqa.selenium.chrome.ChromeDriverService.createDefaultService(ChromeDriverService.java:94)
[info]   at org.openqa.selenium.chrome.ChromeDriver.<init>(ChromeDriver.java:157)
[info]   at org.apache.spark.deploy.history.ChromeUIHistoryServerSuite.beforeAll(ChromeUIHistoryServerSuite.scala:38)
[info]   at org.scalatest.BeforeAndAfterAll.liftedTree1$1(BeforeAndAfterAll.scala:212)
[info]   at org.scalatest.BeforeAndAfterAll.run(BeforeAndAfterAll.scala:210)
[info]   at org.scalatest.BeforeAndAfterAll.run$(BeforeAndAfterAll.scala:208)
[info]   at org.apache.spark.SparkFunSuite.run(SparkFunSuite.scala:61)
[info]   at org.scalatest.tools.Framework.org$scalatest$tools$Framework$$runSuite(Framework.scala:318)
[info]   at org.scalatest.tools.Framework$ScalaTestTask.execute(Framework.scala:513)
[info]   at sbt.ForkMain$Run$2.call(ForkMain.java:296)
[info]   at sbt.ForkMain$Run$2.call(ForkMain.java:286)
[info]   at java.util.concurrent.FutureTask.run(FutureTask.java:266)
[info]   at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
[info]   at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
[info]   at java.lang.Thread.run(Thread.java:748)
[error] Uncaught exception when running org.apache.spark.deploy.history.ChromeUIHistoryServerSuite: java.lang.NoSuchMethodError: com.google.common.base.Preconditions.checkState(ZLjava/lang/String;Ljava/lang/Object;Ljava/lang/Object;Ljava/lang/Object;)V
sbt.ForkMain$ForkError: java.lang.NoSuchMethodError: com.google.common.base.Preconditions.checkState(ZLjava/lang/String;Ljava/lang/Object;Ljava/lang/Object;Ljava/lang/Object;)V
	at org.openqa.selenium.remote.service.DriverService.findExecutable(DriverService.java:134)

So, we need to specify 25.0-jre only for tests tagged with ChromeUITest. Or, revert SPARK-31765 as I mentioned here.

export MAVEN_OPTS="-Xmx2g -XX:ReservedCodeCacheSize=1g -Dorg.slf4j.simpleLogger.defaultLogLevel=WARN"
export MAVEN_CLI_OPTS="--no-transfer-progress"
mkdir -p ~/.m2
./build/mvn -Dspark.test.webdriver.chrome.driver=/usr/bin/chromedriver \
-Dguava.version=25.0-jre -Djava.version=11 -Dtest.default.exclude.tags= -Dtest=none \
Copy link
Member

@dongjoon-hyun dongjoon-hyun Sep 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sarutak . Apache Spark 3.1's official Guava version is still <guava.version>14.0.1</guava.version>, isn't it?
Why do you piggy-back something important thing silently? :)

Copy link
Member Author

@sarutak sarutak Sep 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I know the official version of Guava is 14.0.1. Actually, I noticed the current ChromeDriver and RemoteWebDriver can't use guava 14.0.1 so I intended to use 25.0 only for ChromeUISeleniumSuite and ChromeUIHistoryServerSuite.
Do you have any better idea rather than use 25.0?

Copy link
Member

@dongjoon-hyun dongjoon-hyun Sep 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the test suite is broken with the default Apache Spark distribution, we need to revert the root cause which broke the test suite. IIRC, this test suite is tested with the default configuration initially.

Actually, I noticed the current ChromeDriver and RemoteWebDriver can't use guava 14.0.1

Do we have a JIRA issue for that test suite failure?

Copy link
Member Author

@sarutak sarutak Sep 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first time those tests are added, they works with guava 14.0.1.
I believe #28585 (SPARK-31765) requires upgraded RemoteWebDriver and it requires 25.0-jre.
That PR is for a small security issue.

Do we have a JIRA issue for that test suite failure?

Yes. But we might need to modify the title.
https://issues.apache.org/jira/browse/SPARK-31996

Copy link
Member

@dongjoon-hyun dongjoon-hyun Sep 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the pointer, @sarutak . BTW, there are many request for Guava update. I believe we can handle this in the same group.

Copy link
Member Author

@sarutak sarutak Sep 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, there are many request for Guava update.

Yeah, I know. So, should we revert that PR? It's a security fix but for the test code, or waiting for Guava updated?

Copy link
Member

@srowen srowen Sep 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about just not adding this test? :)
How important is it? does this cover new ground?

Copy link
Member Author

@sarutak sarutak Sep 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can be one option but I'm wondering if we can make much better coverage for UI tests somehow.
I often see that UI tests are done by "manual" especially when the change is related to JavaScript.
UISeleniumSuite, HistoryServerSuite and other similar suites are enough for the static part of the UI like but not enough for the dynamic part which dynamically changes DOM or CSS by JavaScript.
Those existing suites use HtmlUnitDriver but its JavaScript support is not enough.
So I think it's better if we can run test with real browsers in the CI.
ChromeDriver is similar to HtmlUnitDriver but it uses the real Chrome so we can test JavaScript related stuffs.
ChromeUISeleniumSute and ChromeUIHistoryServerSuite use ChromeDriver rather than HtmlUnitDriver and all tests in the suites are what need JavaScript support so I think they cover new ground.

-DwildcardSuites="org.apache.spark.ui.ChromeUISeleniumSuite,org.apache.spark.deploy.history.ChromeUIHistoryServerSuite" test
rm -rf ~/.m2/repository/org/apache/spark
- name: Upload unit tests log files
if: failure()
uses: actions/upload-artifact@v2
with:
name: unit-tests-log-webui
path: "**/target/unit-tests.log"

java11:
name: Java 11 build
runs-on: ubuntu-latest
Expand Down