squito commented on a change in pull request #23260: [SPARK-26311][CORE] New 
feature: apply custom log URL pattern for executor log URLs in SHS
URL: https://github.com/apache/spark/pull/23260#discussion_r249836436
 
 

 ##########
 File path: 
core/src/test/scala/org/apache/spark/deploy/history/HistoryServerSuite.scala
 ##########
 @@ -183,19 +183,43 @@ class HistoryServerSuite extends SparkFunSuite with 
BeforeAndAfter with Matchers
   // in the test resource folder
   cases.foreach { case (name, path) =>
     test(name) {
-      val (code, jsonOpt, errOpt) = getContentAndCode(path)
-      code should be (HttpServletResponse.SC_OK)
-      jsonOpt should be ('defined)
-      errOpt should be (None)
-
-      val exp = IOUtils.toString(new FileInputStream(
-        new File(expRoot, HistoryServerSuite.sanitizePath(name) + 
"_expectation.json")))
-      // compare the ASTs so formatting differences don't cause failures
-      import org.json4s._
-      import org.json4s.jackson.JsonMethods._
-      val jsonAst = parse(clearLastUpdated(jsonOpt.get))
-      val expAst = parse(exp)
-      assertValidDataInJson(jsonAst, expAst)
+      val expectationFile = new File(expRoot, 
HistoryServerSuite.sanitizePath(name) +
+        "_expectation.json")
+      assertApiCallResponse(path, expectationFile)
+    }
+  }
+
+  val casesCustomLogUrl = Seq(
+    "completed app executor list json apply custom log urls" ->
+      "applications/application_1547723113049_0005/1/executors",
+    "incomplete app executor list json apply custom log urls" ->
+      "applications/application_1547723113049_0006/1/executors"
+  )
+
+  casesCustomLogUrl.foreach { case (name, path) =>
+    Seq(true, false).foreach { applyToIncompleteApp =>
+      val newName = name + s" apply incomplete app $applyToIncompleteApp"
+      test(newName) {
+        // This only verifies whether applying custom log URLs is in effect 
for SHS.
+        // Validation of "custom log URLs" functionality will be covered from 
different UTs.
+
+        // restart server
+        try {
+          stop()
+          init(
+            // compiler complains when using `->`, so made a tuple
+            (CUSTOM_EXECUTOR_LOG_URL.key, "http://newhost:9999/logs/clusters/"; 
+
+              
"{{CLUSTER_ID}}/users/{{USER}}/containers/{{CONTAINER_ID}}/{{FILE_NAME}}"),
+            APPLY_CUSTOM_EXECUTOR_LOG_URL_TO_INCOMPLETE_APP.key -> 
applyToIncompleteApp.toString)
+
+          val expectationFile = new File(expRoot, 
HistoryServerSuite.sanitizePath(newName) +
+            "_expectation.json")
+          assertApiCallResponse(path, expectationFile)
+        } finally {
+          // make sure other UTs are not affected from relaunching 
HistoryServer
+          stop()
 
 Review comment:
   you did this right, but I'm realizing now this suite doesn't stop the 
history server at the very end.  While you're touching this file, can you add 
an `afterAll` from `BeforeAndAfterAll`?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to