Skip to content

Conversation

@sarutak
Copy link
Member

@sarutak sarutak commented May 19, 2020

What changes were proposed in this pull request?

This PR mainly adds two things.

  1. Real headless browser support for UI test
  2. A test suite using headless Chrome as one instance of those browsers.

Also, for environment where Chrome and Chrome driver is not installed, ChromeUITest tag is added to filter out the test suite.

Why are the changes needed?

In the current master, there are two problems for UI test.

  1. Lots of tests especially JavaScript related ones are done manually.
    Appearance is better to be confirmed by our eyes but logic should be tested by test cases ideally.

  2. Compared to the real web browsers, HtmlUnit doesn't seem to support JavaScript enough.
    I added a JavaScript related test before for SPARK-31534 using HtmlUnit which is simple library based headless browser for test.
    The test I added works somehow but some JavaScript related error is shown in unit-tests.log.

======= EXCEPTION START ========
Exception class=[net.sourceforge.htmlunit.corejs.javascript.JavaScriptException]
com.gargoylesoftware.htmlunit.ScriptException: Error: TOOLTIP: Option "sanitizeFn" provided type "window" but expected type "(null|function)". (http://192.168.1.209:60724/static/jquery-3.4.1.min.js#2)
        at com.gargoylesoftware.htmlunit.javascript.JavaScriptEngine$HtmlUnitContextAction.run(JavaScriptEngine.java:904)
        at net.sourceforge.htmlunit.corejs.javascript.Context.call(Context.java:628)
        at net.sourceforge.htmlunit.corejs.javascript.ContextFactory.call(ContextFactory.java:515)
        at com.gargoylesoftware.htmlunit.javascript.JavaScriptEngine.callFunction(JavaScriptEngine.java:835)
        at com.gargoylesoftware.htmlunit.javascript.JavaScriptEngine.callFunction(JavaScriptEngine.java:807)
        at com.gargoylesoftware.htmlunit.InteractivePage.executeJavaScriptFunctionIfPossible(InteractivePage.java:216)
        at com.gargoylesoftware.htmlunit.javascript.background.JavaScriptFunctionJob.runJavaScript(JavaScriptFunctionJob.java:52)
        at com.gargoylesoftware.htmlunit.javascript.background.JavaScriptExecutionJob.run(JavaScriptExecutionJob.java:102)
        at com.gargoylesoftware.htmlunit.javascript.background.JavaScriptJobManagerImpl.runSingleJob(JavaScriptJobManagerImpl.java:426)
        at com.gargoylesoftware.htmlunit.javascript.background.DefaultJavaScriptExecutor.run(DefaultJavaScriptExecutor.java:157)
        at java.lang.Thread.run(Thread.java:748)
Caused by: net.sourceforge.htmlunit.corejs.javascript.JavaScriptException: Error: TOOLTIP: Option "sanitizeFn" provided type "window" but expected type "(null|function)". (http://192.168.1.209:60724/static/jquery-3.4.1.min.js#2)
        at net.sourceforge.htmlunit.corejs.javascript.Interpreter.interpretLoop(Interpreter.java:1009)
        at net.sourceforge.htmlunit.corejs.javascript.Interpreter.interpret(Interpreter.java:800)
        at net.sourceforge.htmlunit.corejs.javascript.InterpretedFunction.call(InterpretedFunction.java:105)
        at net.sourceforge.htmlunit.corejs.javascript.ContextFactory.doTopCall(ContextFactory.java:413)
        at com.gargoylesoftware.htmlunit.javascript.HtmlUnitContextFactory.doTopCall(HtmlUnitContextFactory.java:252)
        at net.sourceforge.htmlunit.corejs.javascript.ScriptRuntime.doTopCall(ScriptRuntime.java:3264)
        at com.gargoylesoftware.htmlunit.javascript.JavaScriptEngine$4.doRun(JavaScriptEngine.java:828)
        at com.gargoylesoftware.htmlunit.javascript.JavaScriptEngine$HtmlUnitContextAction.run(JavaScriptEngine.java:889)
        ... 10 more
JavaScriptException value = Error: TOOLTIP: Option "sanitizeFn" provided type "window" but expected type "(null|function)".
== CALLING JAVASCRIPT ==
  function () {
      throw e;
  }
======= EXCEPTION END ========

I tried to upgrade HtmlUnit to 2.40.0 but what is worse, the test become not working even though it works on real browsers like Chrome, Safari and Firefox without error.

[info] UISeleniumSuite:
[info] - SPARK-31534: text for tooltip should be escaped *** FAILED *** (17 seconds, 745 milliseconds)
[info]   The code passed to eventually never returned normally. Attempted 2 times over 12.910785232 seconds. Last failure message: com.gargoylesoftware.htmlunit.ScriptException: ReferenceError: Assignment to undefined "regeneratorRuntime" in strict mode (http://192.168.1.209:62132/static/vis-timeline-graph2d.min.js#52(Function)#1)

To resolve those problems, it's better to support headless browser for UI test.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

I tested with following patterns. Both Chrome and Chrome driver should be installed to test.

  1. sbt / with chromedriver / include tag (expect to succeed)
    build/sbt -Dspark.test.webdriver.chrome.driver=/path/to/chromedriver "testOnly org.apache.spark.ui.ChromeUISeleniumSuite"
  2. sbt / with chromedriver / exclude tag (expect to be ignored)
    build/sbt -Dspark.test.webdriver.chrome.driver=/path/to/chromedriver "testOnly org.apache.spark.ui.ChromeUISeleniumSuite -l org.apache.spark.tags.ChromeUITest"
  3. sbt / without chromedriver / include tag (expect to be failed)
    build/sbt "testOnly org.apache.spark.ui.ChromeUISeleniumSuite"
  4. sbt / without chromedriver / exclude tag (expect to be skipped)
    build/sbt -Dtest.exclude.tags=org.apache.spark.tags.ChromeUITest "testOnly org.apache.spark.ui.ChromeUISeleniumSuite"
  5. Maven / wth chromedriver / include tag (expect to succeed)
    build/mvn -Dspark.test.webdriver.chrome.driver=/path/to/chromedriver -Dtest=none -DwildcardSuites=org.apache.spark.ui.ChromeUISeleniumSuite test
  6. Maven / with chromedriver / exclude tag (expect to be skipped)
    build/mvn -Dtest.exclude.tags="org.apache.spark.tags.ChromeUITest" -Dspark.test.webdriver.chrome.driver=/path/to/chromedriver -Dtest=none -DwildcardSuites=org.apache.spark.ui.ChromeUISeleniumSuite test
  7. Maven / without chromedriver / include tag (expect to be failed)
    build/mvn -Dtest=none -DwildcardSuites=org.apache.spark.ui.ChromeUISeleniumSuite test
  8. Maven / without chromedriver / exclude tag (expect to be skipped)
    build/mvn -Dtest.exclude.tags=org.apache.spark.tags.ChromeUITest -Dtest=none -DwildcardSuites=org.apache.spark.ui.ChromeUISeleniumSuite test

print("[info] Found the following changed modules:",
", ".join(x.name for x in changed_modules))

excluded_tags.extend(always_excluded_tags)
Copy link
Member Author

@sarutak sarutak May 19, 2020

Choose a reason for hiding this comment

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

For environments where Chrome and Chrome driver is not installed, including CI environment, tests tagged as ChromeUITest is filtered out when tests are run through run-tests.py.

@SparkQA
Copy link

SparkQA commented May 19, 2020

Test build #122835 has finished for PR 28578 at commit e701dc9.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented May 20, 2020

Test build #122863 has finished for PR 28578 at commit 6ccbc39.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@srowen
Copy link
Member

srowen commented May 20, 2020

Seems OK to me.

@HyukjinKwon
Copy link
Member

cc @gengliangwang FYI. Looks fine to me too

@gengliangwang
Copy link
Member

@sarutak seems there is code conflict after #28585 is merged.

@srowen srowen closed this in d955708 May 22, 2020
@srowen
Copy link
Member

srowen commented May 22, 2020

Merged to master

@dongjoon-hyun
Copy link
Member

dongjoon-hyun commented May 23, 2020

Hi, All.
This seems to break Hadoop 3.2/Hive 2.3 profile Jenkins Maven jobs in master branch.

ChromeUISeleniumSuite:
org.apache.spark.ui.ChromeUISeleniumSuite *** ABORTED ***
  java.lang.IllegalStateException: The driver executable does not exist: /home/jenkins/workspace/spark-master-test-maven-hadoop-3.2-hive-2.3@2/core/null
  at com.google.common.base.Preconditions.checkState(Preconditions.java:176)
  at org.openqa.selenium.remote.service.DriverService.checkExecutable(DriverService.java:121)
  at org.openqa.selenium.remote.service.DriverService.findExecutable(DriverService.java:116)
  at org.openqa.selenium.chrome.ChromeDriverService.access$000(ChromeDriverService.java:32)
  at org.openqa.selenium.chrome.ChromeDriverService$Builder.findDefaultExecutable(ChromeDriverService.java:137)
  at org.openqa.selenium.remote.service.DriverService$Builder.build(DriverService.java:296)
  at org.openqa.selenium.chrome.ChromeDriverService.createDefaultService(ChromeDriverService.java:88)
  at org.openqa.selenium.chrome.ChromeDriver.<init>(ChromeDriver.java:148)
  at org.apache.spark.ui.ChromeUISeleniumSuite.beforeAll(ChromeUISeleniumSuite.scala:37)
  at org.scalatest.BeforeAndAfterAll.liftedTree1$1(BeforeAndAfterAll.scala:212)

Hadoop 3.2 / Hive 2.3 / JDK11 Jenkins Maven job is also failing together consistently.

@sarutak
Copy link
Member Author

sarutak commented May 23, 2020

Ah, some profiles directly run tests without run-tests.py.
It might be better to disable the new suite by default.

@dongjoon-hyun
Copy link
Member

Thanks for taking a look at that. Could you make a follow-up PR, @sarutak ?

@sarutak
Copy link
Member Author

sarutak commented May 23, 2020

I'll try it.

@dongjoon-hyun
Copy link
Member

Since it's a weekend, can we simple revert this first to recover the master branch first? Then, we can make a PR with [test-maven] title to make it sure at PR stage.

@sarutak
Copy link
Member Author

sarutak commented May 24, 2020

Agree. I'll make a revert PR.

@dongjoon-hyun
Copy link
Member

Thank you so much.

@dongjoon-hyun
Copy link
Member

You can revert directly since this commit is pretty new.

@dongjoon-hyun
Copy link
Member

Thank you. I saw the commit.

srowen pushed a commit that referenced this pull request Jun 1, 2020
…ver tests

### What changes were proposed in this pull request?

This PR adds two things.

Real headless browser support for HistoryServer tests.
A test suite using headless Chrome as one instance of those browsers.

### Why are the changes needed?

The motivation is same as #28578 .
In the current master, there is a testcase for HistoryServer which uses Ajax so we need the support for HistoryServer tests.

Also this change is necessary to upgrade HtmlUnit (See #28585)

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

I tested with following patterns. Both Chrome and Chrome driver should be installed to test.
1. sbt / with default excluded tags (ChromeUIHistoryServerSuite is expected to be skipped and SQLQueryTestSuite is expected to succeed)
`build/sbt -Dspark.test.webdriver.chrome.driver=/path/to/chromedriver "testOnly org.apache.spark.deploy.history.ChromeUIHistoryServerSuite org.apache.spark.sql.SQLQueryTestSuite"

2. sbt / overwrite default excluded tags as empty string (Both suites are expected to succeed)
`build/sbt -Dtest.default.exclude.tags= -Dspark.test.webdriver.chrome.driver=/path/to/chromedriver "testOnly org.apache.spark.deploy.history.ChromeUIHistoryServerSuite org.apache.spark.sql.SQLQueryTestSuite"

3. sbt / set `test.exclude.tags` to `org.apache.spark.tags.ExtendedSQLTest` (Both suites are expected to be skipped)
`build/sbt -Dtest.exclude.tags=org.apache.spark.tags.ExtendedSQLTest -Dspark.test.webdriver.chrome.driver=/path/to/chromedriver "testOnly org.apache.spark.deploy.history.ChromeUIHistoryServerSuite org.apache.spark.sql.SQLQueryTestSuite"

4. Maven / with default excluded tags (ChromeUIHistoryServerSuite is expected to be skipped and SQLQueryTestSuite is expected to succeed)
`build/mvn -Dspark.test.webdriver.chrome.driver=/path/to/chromedriver -Dtest=none -DwildcardSuites=org.apache.spark.deploy.history.ChromeUIHistoryServerSuite,org.apache.spark.sql.SQLQueryTestSuite test`

5. Maven / overwrite default excluded tags as empty string (Both suites are expected to succeed)
`build/mvn -Dtest.default.exclude.tags= -Dspark.test.webdriver.chrome.driver=/path/to/chromedriver -Dtest=none -DwildcardSuites=org.apache.spark.deploy.history.ChromeUIHistoryServerSuite,org.apache.spark.sql.SQLQueryTestSuite test`

6. Maven / set `test.exclude.tags` to `org.apache.spark.tags.ExtendedSQLTest` (Both suites are expected to be skipped)
`build/mvn -Dtest.exclude.tags=org.apache.spark.tags.ExtendedSQLTest  -Dspark.test.webdriver.chrome.driver=/path/to/chromedriver -Dtest=none -DwildcardSuites=org.apache.spark.deploy.history.ChromeUIHistoryServerSuite,org.apache.spark.sql.SQLQueryTestSuite test`

Closes #28622 from sarutak/headless-browser-support-for-historyserver.

Authored-by: Kousuke Saruta <[email protected]>
Signed-off-by: Sean Owen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants