-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-40424][CORE][TESTS] Refactor ChromeUIHistoryServerSuite to add UTs for RocksDB
#37878
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
|
cc @sarutak FYI |
|
rebased and updated pr description |
|
also cc @dongjoon-hyun due to the new test is for RocksDB |
|
friendly ping @sarutak |
| } | ||
|
|
||
| @ChromeUITest | ||
| class RocksBackendChromeUIHistoryServerSuite extends ChromeUIHistoryServerSuite { |
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.
RocksBackendChromeUIHistoryServerSuite -> RocksDBBackendChromeUIHistoryServerSuite
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.
Sorry for being late. Could you rename the test suite class, @LuciferYang ?
- RocksBackendChromeUIHistoryServerSuite
+ RocksDBBackendChromeUIHistoryServerSuite
dongjoon-hyun
left a comment
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.
+1, LGTM. I tested this test-only PR manually. Merged to master.
$ build/sbt -Dguava.version=31.1-jre -Dspark.test.webdriver.chrome.driver=/opt/homebrew/bin/chromedriver -Dtest.default.exclude.tags="" "core/testOnly org.apache.spark.deploy.history.RocksDBBackendChromeUIHistoryServerSuite"
...
[info] RocksDBBackendChromeUIHistoryServerSuite:
Starting ChromeDriver 105.0.5195.52 (412c95e518836d8a7d97250d62b29c2ae6a26a85-refs/branch-heads/5195@{#853}) on port 64391
Only local connections are allowed.
Please see https://chromedriver.chromium.org/security-considerations for suggestions on keeping ChromeDriver safe.
ChromeDriver was started successfully.
[info] - ajax rendered relative links are prefixed with uiRoot (spark.ui.proxyBase) (1 second, 736 milliseconds)
[info] Run completed in 4 seconds, 924 milliseconds.
[info] Total number of tests run: 1
[info] Suites: completed 1, aborted 0
[info] Tests: succeeded 1, failed 0, canceled 0, ignored 0, pending 0
[info] All tests passed.
[success] Total time: 12 s, completed Sep 18, 2022, 6:28:18 PM
|
Thank you, @LuciferYang and @HyukjinKwon . |
Thanks for your help @dongjoon-hyun ~ |
…dd UTs for RocksDB
### What changes were proposed in this pull request?
`ChromeUIHistoryServerSuite` only test LevelDB backend now, this pr refactor the UTs of `ChromeUIHistoryServerSuite` to add UTs for RocksDB
### Why are the changes needed?
Add UTs related to RocksDB.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
- Pass GA
- Manual test on Apple Silicon environment:
```
build/sbt -Dguava.version=31.1-jre -Dspark.test.webdriver.chrome.driver=/path/to/chromedriver -Dtest.default.exclude.tags="" "core/testOnly org.apache.spark.deploy.history.RocksBackendChromeUIHistoryServerSuite"
```
```
[info] RocksBackendChromeUIHistoryServerSuite:
Starting ChromeDriver 105.0.5195.52 (412c95e518836d8a7d97250d62b29c2ae6a26a85-refs/branch-heads/5195{apache#853}) on port 54402
Only local connections are allowed.
Please see https://chromedriver.chromium.org/security-considerations for suggestions on keeping ChromeDriver safe.
ChromeDriver was started successfully.
[info] - ajax rendered relative links are prefixed with uiRoot (spark.ui.proxyBase) (5 seconds, 387 milliseconds)
[info] Run completed in 20 seconds, 838 milliseconds.
[info] Total number of tests run: 1
[info] Suites: completed 1, aborted 0
[info] Tests: succeeded 1, failed 0, canceled 0, ignored 0, pending 0
[info] All tests passed.
[success] Total time: 118 s (01:58), completed 2022-9-15 10:30:53
```
Closes apache#37878 from LuciferYang/SPARK-40424.
Lead-authored-by: yangjie01 <[email protected]>
Co-authored-by: Dongjoon Hyun <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
What changes were proposed in this pull request?
ChromeUIHistoryServerSuiteonly test LevelDB backend now, this pr refactor the UTs ofChromeUIHistoryServerSuiteto add UTs for RocksDBWhy are the changes needed?
Add UTs related to RocksDB.
Does this PR introduce any user-facing change?
No.
How was this patch tested?