You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/02/02 18:49:17 UTC

[GitHub] [spark] xkrogen opened a new pull request #35383: [SPARK-38089][CORE][TESTS] Improve assertion failure message in TestUtils.assertExceptionMsg

xkrogen opened a new pull request #35383:
URL: https://github.com/apache/spark/pull/35383


   ### What changes were proposed in this pull request?
   Improve assertion failure message in `TestUtils.assertExceptionMsg` to print out the exception tree in which it was searching (upon failure).
   
   ### Why are the changes needed?
   `TestUtils.assertExceptionMsg` is great, but when the assertion _doesn't_ match, it can be challenging to tell why, because the exception tree that was searched isn't printed. Only way I could find to fix it up was to run things in a debugger and check the exception tree. This makes it easier to tell what went wrong just from the assertion failure message.
   
   ### Does this PR introduce _any_ user-facing change?
   No. Easier for devs to see why their test failed.
   
   ### How was this patch tested?
   Used extensively while writing the tests for PR #34009. It was very useful!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on pull request #35383: [SPARK-38089][CORE][TESTS] Show the root cause exception in `TestUtils.assertExceptionMsg`

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #35383:
URL: https://github.com/apache/spark/pull/35383#issuecomment-1028464349


   BTW, for new feature and improvement JIRAs, `Affected Version` is `master` branch's version because Apache Spark doesn't allow backporting of new features and improvements.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] xkrogen commented on pull request #35383: [SPARK-38089][CORE][TESTS] Show the root cause exception in `TestUtils.assertExceptionMsg`

Posted by GitBox <gi...@apache.org>.
xkrogen commented on pull request #35383:
URL: https://github.com/apache/spark/pull/35383#issuecomment-1028473986


   Got it, thanks for the tip and for the review!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun edited a comment on pull request #35383: [SPARK-38089][CORE][TESTS] Show the root cause exception in `TestUtils.assertExceptionMsg`

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun edited a comment on pull request #35383:
URL: https://github.com/apache/spark/pull/35383#issuecomment-1028464349


   BTW, for new feature and improvement JIRAs, `Affected Version` is `master` branch's version because Apache Spark doesn't allow backporting of new features and improvements. I updated the affected version of SPARK-38089 to 3.3.0 from 3.2.1.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun closed pull request #35383: [SPARK-38089][CORE][TESTS] Show the root cause exception in `TestUtils.assertExceptionMsg`

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun closed pull request #35383:
URL: https://github.com/apache/spark/pull/35383


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] xkrogen commented on pull request #35383: [SPARK-38089][CORE][TESTS] Show the root cause exception in `TestUtils.assertExceptionMsg`

Posted by GitBox <gi...@apache.org>.
xkrogen commented on pull request #35383:
URL: https://github.com/apache/spark/pull/35383#issuecomment-1028371947


   Looks like the test failures are some unrelated issues in the `yarn` tests:
   ```
   [info] - run Spark in yarn-client mode with different configurations, ensuring redaction *** FAILED *** (3 minutes, 0 seconds)
   [info]   The code passed to eventually never returned normally. Attempted 190 times over 3.0012781688500003 minutes. Last failure message: handle.getState().isFinal() was false. (BaseYarnClusterSuite.scala:204)
   ....
   [info] - run Spark in yarn-cluster mode with different configurations, ensuring redaction *** FAILED *** (3 minutes, 0 seconds)
   [info]   The code passed to eventually never returned normally. Attempted 190 times over 3.001207979333333 minutes. Last failure message: handle.getState().isFinal() was false. (BaseYarnClusterSuite.scala:204)
   ....
   [info] *** Test still running after 2 minutes, 55 seconds: suite name: YarnClusterSuite, test name: yarn-cluster should respect conf overrides in SparkHadoopUtil (SPARK-16414, SPARK-23630). 
   [info] - yarn-cluster should respect conf overrides in SparkHadoopUtil (SPARK-16414, SPARK-23630) *** FAILED *** (3 minutes, 0 seconds)
   [info]   The code passed to eventually never returned normally. Attempted 190 times over 3.0010310271500003 minutes. Last failure message: handle.getState().isFinal() was false. (BaseYarnClusterSuite.scala:204)
   ....
   [info] - SPARK-35672: run Spark in yarn-client mode with additional jar using URI scheme 'local' *** FAILED *** (3 minutes, 0 seconds)
   [info]   The code passed to eventually never returned normally. Attempted 190 times over 3.0009631187333334 minutes. Last failure message: handle.getState().isFinal() was false. (BaseYarnClusterSuite.scala:204)
   ```
   I'll re-trigger.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] xkrogen commented on pull request #35383: [SPARK-38089][CORE][TESTS] Improve assertion failure message in TestUtils.assertExceptionMsg

Posted by GitBox <gi...@apache.org>.
xkrogen commented on pull request #35383:
URL: https://github.com/apache/spark/pull/35383#issuecomment-1028289225


   Weird, GA is definitely enabled on my fork, I have used it many times. I pushed again a few minutes ago and it seems to be working again.
   I also updated the description per your request.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on pull request #35383: [SPARK-38089][CORE][TESTS] Improve assertion failure message in TestUtils.assertExceptionMsg

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #35383:
URL: https://github.com/apache/spark/pull/35383#issuecomment-1028255230


   In addition, please include some example into the PR description. I have no idea how helpful it was to you.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on pull request #35383: [SPARK-38089][CORE][TESTS] Show the root cause exception in `TestUtils.assertExceptionMsg`

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #35383:
URL: https://github.com/apache/spark/pull/35383#issuecomment-1028463540


   Merged to master for Apache Spark 3.3.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] xkrogen commented on pull request #35383: [SPARK-38089][CORE][TESTS] Show the root cause exception in `TestUtils.assertExceptionMsg`

Posted by GitBox <gi...@apache.org>.
xkrogen commented on pull request #35383:
URL: https://github.com/apache/spark/pull/35383#issuecomment-1028297859


   Shouldn't be any change in log size unless there are test failures, since this will only print anything extra on assertion failure (which would cause a test failure). But of course let's wait for CI.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on pull request #35383: [SPARK-38089][CORE][TESTS] Show the root cause exception in `TestUtils.assertExceptionMsg`

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #35383:
URL: https://github.com/apache/spark/pull/35383#issuecomment-1028418803


   Thank you for re-triggering. I agree with you. I'll merge this when the CI passes, @xkrogen .


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org