[GitHub] liupc commented on a change in pull request #23404: [SPARK-26501]Fix unexpected overriden of exitFn in SparkSubmitSuite
liupc commented on a change in pull request #23404: [SPARK-26501]Fix unexpected
overriden of exitFn in SparkSubmitSuite
URL: https://github.com/apache/spark/pull/23404#discussion_r244657632
##
File path: core/src/test/scala/org/apache/spark/deploy/SparkSubmitSuite.scala
##
@@ -72,26 +72,32 @@ trait TestPrematureExit {
mainObject.printStream = printStream
@volatile var exitedCleanly = false
-mainObject.exitFn = (_) => exitedCleanly = true
-
-@volatile var exception: Exception = null
-val thread = new Thread {
- override def run() = try {
-mainObject.main(input)
- } catch {
-// Capture the exception to check whether the exception contains
searchString or not
-case e: Exception => exception = e
- }
+def withFakeExit(body: => Unit): Unit = {
Review comment:
I agree that a try-finally block is necessary, because the func body throws
exception.
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:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] liupc commented on a change in pull request #23404: [SPARK-26501]Fix unexpected overriden of exitFn in SparkSubmitSuite
liupc commented on a change in pull request #23404: [SPARK-26501]Fix unexpected
overriden of exitFn in SparkSubmitSuite
URL: https://github.com/apache/spark/pull/23404#discussion_r244656931
##
File path: core/src/test/scala/org/apache/spark/deploy/SparkSubmitSuite.scala
##
@@ -72,26 +72,32 @@ trait TestPrematureExit {
mainObject.printStream = printStream
@volatile var exitedCleanly = false
-mainObject.exitFn = (_) => exitedCleanly = true
-
-@volatile var exception: Exception = null
-val thread = new Thread {
- override def run() = try {
-mainObject.main(input)
- } catch {
-// Capture the exception to check whether the exception contains
searchString or not
-case e: Exception => exception = e
- }
+def withFakeExit(body: => Unit): Unit = {
Review comment:
It's ok to just modify/restore exitFn in a straight way, but use a helper
method here will make the modification of exitFn more clear?
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:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] liupc commented on a change in pull request #23404: [SPARK-26501]Fix unexpected overriden of exitFn in SparkSubmitSuite
liupc commented on a change in pull request #23404: [SPARK-26501]Fix unexpected
overriden of exitFn in SparkSubmitSuite
URL: https://github.com/apache/spark/pull/23404#discussion_r244656931
##
File path: core/src/test/scala/org/apache/spark/deploy/SparkSubmitSuite.scala
##
@@ -72,26 +72,32 @@ trait TestPrematureExit {
mainObject.printStream = printStream
@volatile var exitedCleanly = false
-mainObject.exitFn = (_) => exitedCleanly = true
-
-@volatile var exception: Exception = null
-val thread = new Thread {
- override def run() = try {
-mainObject.main(input)
- } catch {
-// Capture the exception to check whether the exception contains
searchString or not
-case e: Exception => exception = e
- }
+def withFakeExit(body: => Unit): Unit = {
Review comment:
It's ok to just modify/restore exitFn in a straight way, but use a helper
method here will make the modification of exitFn more clear?
moreover, I was wondering if a try-finally block is necessary? there is
already a try-catch block in the func body of the only occurrence.
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:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
