Skip to content

[SPARK-31756][WEBUI] Add real headless browser support for UI test #28627

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 10 commits into from
Closed

[SPARK-31756][WEBUI] Add real headless browser support for UI test #28627

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
424092a
Add RealBrowserUISeleniumSuite.scala
sarutak May 19, 2020
4fe3e80
Add ChromeUITest.java
sarutak May 19, 2020
760eb0a
Add ChromeUISeeniumSuite.scala
sarutak May 19, 2020
c2f6a6d
Remove JavaScript related tests from UISeleniumSuite.scala
sarutak May 19, 2020
e701dc9
Disable tests taged as 'ChromeUITest' when tests are run through run-…
sarutak May 19, 2020
6ccbc39
Fixed RealBrowserUISeleniumSuite.
sarutak May 20, 2020
7e49348
Merge branch 'real-headless-browser-support' into real-headless-brows…
sarutak May 24, 2020
6b47fe8
Disabled newly added suite by default.
sarutak May 24, 2020
70d07a3
Introduced test.default.exclude.tags to set excluded tests by default.
sarutak May 25, 2020
2e805ec
Merge branch 'master' of https://github.com/apache/spark into real-he…
sarutak May 28, 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
Prev Previous commit
Next Next commit
Disabled newly added suite by default.
  • Loading branch information
@sarutak
sarutak committed May 24, 2020
commit 6b47fe8cccca0460ec7c0db0e16c29535f1d01b8
2 changes: 1 addition & 1 deletion core/src/test/scala/org/apache/spark/ui/RealBrowserUISeleniumSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ abstract class RealBrowserUISeleniumSuite(val driverProp: String)
implicit var webDriver: WebDriver
private val driverPropPrefix = "spark.test."

override def beforeAll() {
override def beforeAll(): Unit = {
super.beforeAll()
assume(
sys.props(driverPropPrefix + driverProp) !== null,
Expand Down
2 changes: 1 addition & 1 deletion pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@
<leveldbjni.group>org.fusesource.leveldbjni</leveldbjni.group>

<test.java.home>${java.home}</test.java.home>
<test.exclude.tags></test.exclude.tags>
<test.exclude.tags>org.apache.spark.tags.ChromeUITest</test.exclude.tags>
Copy link
Member

@dongjoon-hyun dongjoon-hyun May 24, 2020

Choose a reason for hiding this comment

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

Could you add some comment before this line?

<test.include.tags></test.include.tags>

<!-- Package to use when relocating shaded classes. -->
Expand Down
9 changes: 6 additions & 3 deletions project/SparkBuild.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -967,6 +967,9 @@ object TestSettings {
"2.12"
}
*/

private val defaultExcludedTagsForScalaTest = Seq("org.apache.spark.tags.ChromeUITest")

lazy val settings = Seq (
// Fork new JVMs for tests and set Java options for those
fork := true,
Expand Down Expand Up @@ -1001,9 +1004,9 @@ object TestSettings {
javaOptions += "-Xmx3g",
// Exclude tags defined in a system property
testOptions in Test += Tests.Argument(TestFrameworks.ScalaTest,
sys.props.get("test.exclude.tags").map { tags =>
tags.split(",").flatMap { tag => Seq("-l", tag) }.toSeq
}.getOrElse(Nil): _*),
sys.props.get("test.exclude.tags").map(tag => tag.split(",").toSeq)
.getOrElse(defaultExcludedTagsForScalaTest).filter(!_.trim.isEmpty)
Copy link
Member

@dongjoon-hyun dongjoon-hyun May 24, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

This looks not robust enough.
If a developer set an irrelevant tag (ExtendedSQLTest or ExtendedHiveTest), ChromeUITest suddenly will cause a failure in the irrelevant Core module. Did I understand correctly?

Copy link
Member Author

@sarutak sarutak May 25, 2020

Choose a reason for hiding this comment

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

It's what I intended that if developers set test.excluded.tags explicitly, all the tags which are intended to be excluded should be added explicitly.
But as you concerned, this change seems to break the build compatibility.
I'll submit another idea.

.flatMap(tag => Seq("-l", tag)): _*),
testOptions in Test += Tests.Argument(TestFrameworks.JUnit,
sys.props.get("test.exclude.tags").map { tags =>
Seq("--exclude-categories=" + tags)
Expand Down