Github user vanzin commented on a diff in the pull request: https://github.com/apache/spark/pull/22623#discussion_r223497773 --- Diff: core/src/test/scala/org/apache/spark/deploy/SparkSubmitSuite.scala --- @@ -74,20 +74,28 @@ trait TestPrematureExit { @volatile var exitedCleanly = false mainObject.exitFn = (_) => exitedCleanly = true + var exception: Exception = null val thread = new Thread { override def run() = try { mainObject.main(input) } catch { // If exceptions occur after the "exit" has happened, fine to ignore them. // These represent code paths not reachable during normal execution. - case e: Exception => if (!exitedCleanly) throw e + case e: Exception => exception = e } } thread.start() thread.join() val joined = printStream.lineBuffer.mkString("\n") - if (!joined.contains(searchString)) { - fail(s"Search string '$searchString' not found in $joined") + val searchStrContainsInEx = exception != null && exception.getMessage.contains(searchString) + if (!searchStrContainsInEx) { --- End diff -- I thinks this is a bit hard to follow. Isn't this equivalent to: ``` if (exitedCleanly) { // check the output captured in printStream } else { assert(exception != null) if (message does not contain search string) { throw exception } } ``` Also, now that you moved the logic here, the comment in L83 is kinda orphaned, and a fresher comment explaining this block would be better.
--- --------------------------------------------------------------------- To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org