Skip to content

Conversation

@sarutak
Copy link
Member

@sarutak sarutak commented May 23, 2020

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

@SparkQA
Copy link

SparkQA commented May 23, 2020

Test build #123033 has finished for PR 28622 at commit e5ee94e.

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

@SparkQA
Copy link

SparkQA commented May 23, 2020

Test build #123034 has finished for PR 28622 at commit 4db2833.

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

Copy link
Member

@srowen srowen left a comment

Choose a reason for hiding this comment

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

Just checking - will existing tests still run on our amplab Jenkins instances then? I see that they get disabled without chromedriver.

@sarutak
Copy link
Member Author

sarutak commented May 23, 2020

Existing tests in HistoryServerSuite should still run on the CI environment except one test which I moved to ChromeUIHistoryServerSuite.
If we can, it's better to enable such tests to run on Jenkins.

@dongjoon-hyun
Copy link
Member

Please hold on this because the original PR breaks master branch. This can propagate the failures into new suites. Please see #28578 (comment) .

@HyukjinKwon HyukjinKwon changed the title [SPARK-31804][WEBUI] Add real headless browser support for HistoryServer tests [SPARK-31804][WEBUI][test-maven] Add real headless browser support for HistoryServer tests May 24, 2020
@HyukjinKwon
Copy link
Member

retest this please

@sarutak
Copy link
Member Author

sarutak commented May 24, 2020

@HyukjinKwon Thanks for taking a look at this. Tests in this PR will fail until #28627 is merged because this PR doesn't contain the way to skip suites with the tag ChromeUITest.

@SparkQA
Copy link

SparkQA commented May 24, 2020

Test build #123047 has finished for PR 28622 at commit 4db2833.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds no public classes.

@HyukjinKwon
Copy link
Member

Oh, I see. Thanks :D.

@sarutak sarutak changed the title [SPARK-31804][WEBUI][test-maven] Add real headless browser support for HistoryServer tests [SPARK-31804][WEBUI] Add real headless browser support for HistoryServer tests May 30, 2020
@sarutak
Copy link
Member Author

sarutak commented May 30, 2020

ChromeUITest is introduced by #28627 so now this change should be available.
I've updated the test patterns in the description.

@SparkQA
Copy link

SparkQA commented May 30, 2020

Test build #123307 has finished for PR 28622 at commit a852a5d.

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

@srowen
Copy link
Member

srowen commented Jun 1, 2020

@sarutak to be clear we can merge this, or does another need to come first?

@sarutak
Copy link
Member Author

sarutak commented Jun 1, 2020

Now we can merge this and the next is #28585.

@srowen
Copy link
Member

srowen commented Jun 1, 2020

Merged to master

@srowen srowen closed this in 6a895d0 Jun 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants