This is an automated email from the ASF dual-hosted git repository.

gengliang pushed a commit to branch branch-3.1
in repository https://gitbox.apache.org/repos/asf/spark.git


The following commit(s) were added to refs/heads/branch-3.1 by this push:
     new c5ba6bb  [SPARK-35004][TEST] Fix Incorrect assertion of `master/worker 
web ui available behind front-end reverseProxy` in MasterSuite
c5ba6bb is described below

commit c5ba6bb3eb724db7ee455c56a00f0043220af864
Author: yangjie01 <yangji...@baidu.com>
AuthorDate: Fri Apr 9 21:18:49 2021 +0800

    [SPARK-35004][TEST] Fix Incorrect assertion of `master/worker web ui 
available behind front-end reverseProxy` in MasterSuite
    
    ### What changes were proposed in this pull request?
    Line 425 in `MasterSuite` is considered as unused expression by Intellij 
IDE,
    
    
https://github.com/apache/spark/blob/bfba7fadd2e65c853971fb2983bdea1c52d1ed7f/core/src/test/scala/org/apache/spark/deploy/master/MasterSuite.scala#L421-L426
    
    If we merge lines 424 and 425 into one as:
    
    ```
    System.getProperty("spark.ui.proxyBase") should startWith 
(s"$reverseProxyUrl/proxy/worker-")
    ```
    
    this assertion will fail:
    
    ```
    - master/worker web ui available behind front-end reverseProxy *** FAILED 
***
      The code passed to eventually never returned normally. Attempted 45 times 
over 5.091914027 seconds. Last failure message: 
"http://proxyhost:8080/path/to/spark"; did not start with substring 
"http://proxyhost:8080/path/to/spark/proxy/worker-";. (MasterSuite.scala:405)
    ```
    
    `System.getProperty("spark.ui.proxyBase")` should be `reverseProxyUrl` 
because `Master#onStart` and `Worker#handleRegisterResponse` have not changed 
it.
    
    So the main purpose of this pr is to fix the condition of this assertion.
    
    ### Why are the changes needed?
    Bug fix.
    
    ### Does this PR introduce _any_ user-facing change?
    No.
    
    ### How was this patch tested?
    
    - Pass the Jenkins or GitHub Action
    
    - Manual test:
    
    1. merge lines 424 and 425 in `MasterSuite` into one to eliminate the 
unused expression:
    
    ```
    System.getProperty("spark.ui.proxyBase") should startWith 
(s"$reverseProxyUrl/proxy/worker-")
    ```
    
    2. execute `mvn clean test -pl core -Dtest=none 
-DwildcardSuites=org.apache.spark.deploy.master.MasterSuite`
    
    **Before**
    
    ```
    - master/worker web ui available behind front-end reverseProxy *** FAILED 
***
      The code passed to eventually never returned normally. Attempted 45 times 
over 5.091914027 seconds. Last failure message: 
"http://proxyhost:8080/path/to/spark"; did not start with substring 
"http://proxyhost:8080/path/to/spark/proxy/worker-";. (MasterSuite.scala:405)
    
    Run completed in 1 minute, 14 seconds.
    Total number of tests run: 32
    Suites: completed 2, aborted 0
    Tests: succeeded 31, failed 1, canceled 0, ignored 0, pending 0
    *** 1 TEST FAILED ***
    
    ```
    
    **After**
    
    ```
    Run completed in 1 minute, 11 seconds.
    Total number of tests run: 32
    Suites: completed 2, aborted 0
    Tests: succeeded 32, failed 0, canceled 0, ignored 0, pending 0
    All tests passed.
    ```
    
    Closes #32105 from LuciferYang/SPARK-35004.
    
    Authored-by: yangjie01 <yangji...@baidu.com>
    Signed-off-by: Gengliang Wang <ltn...@gmail.com>
    (cherry picked from commit c06758834e6192b1888b4a885c612a151588b390)
    Signed-off-by: Gengliang Wang <ltn...@gmail.com>
---
 .../test/scala/org/apache/spark/deploy/master/MasterSuite.scala    | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git 
a/core/src/test/scala/org/apache/spark/deploy/master/MasterSuite.scala 
b/core/src/test/scala/org/apache/spark/deploy/master/MasterSuite.scala
index 9296274..07ef46d 100644
--- a/core/src/test/scala/org/apache/spark/deploy/master/MasterSuite.scala
+++ b/core/src/test/scala/org/apache/spark/deploy/master/MasterSuite.scala
@@ -418,12 +418,7 @@ class MasterSuite extends SparkFunSuite
           (workerResponse \ "masterwebuiurl").extract[String] should be 
(reverseProxyUrl + "/")
         }
 
-        // with LocalCluster, we have masters and workers in the same JVM, 
each overwriting
-        // system property spark.ui.proxyBase.
-        // so we need to manage this property explicitly for test
-        System.getProperty("spark.ui.proxyBase") should startWith
-          (s"$reverseProxyUrl/proxy/worker-")
-        System.setProperty("spark.ui.proxyBase", reverseProxyUrl)
+        System.getProperty("spark.ui.proxyBase") should be (reverseProxyUrl)
         val html = Utils
           
.tryWithResource(Source.fromURL(s"$masterUrl/"))(_.getLines().mkString("\n"))
         html should include ("Spark Master at spark://")

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org
For additional commands, e-mail: commits-h...@spark.apache.org

Reply via email to