[GitHub] [spark] Yikun commented on pull request #32867: [SPARK-35721][PYTHON] Path level discover for python unittests
Yikun commented on pull request #32867: URL: https://github.com/apache/spark/pull/32867#issuecomment-872072514 The root reason of failed to discover is that the deps of PySpark modules is not installed, so we get the wrong list when we do discover. I re-propose it in https://github.com/apache/spark/pull/33174 , the mainly changed: - add the error check for discover to make sure the discover work, that means if the error like this pr happend would be raised with exception. - Move the discover from `dev/run-tests.py` to `python/run-tests.py`, we don't need to discover python test and install python test deps in other modules. - Add doctest to make sure discover work as expected. -- 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-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] Yikun commented on pull request #32867: [SPARK-35721][PYTHON] Path level discover for python unittests
Yikun commented on pull request #32867: URL: https://github.com/apache/spark/pull/32867#issuecomment-871032645 Thanks for revert for it. I'm so sorry for the trouble, I will recheck and fix soon. -- 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-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] Yikun commented on pull request #32867: [SPARK-35721][PYTHON] Path level discover for python unittests
Yikun commented on pull request #32867: URL: https://github.com/apache/spark/pull/32867#issuecomment-870221373 > Just wondering we can only verify it manually? If by any chance, some tests are not found, can we easily know it? It just same as before, if we forgot to add the path of tests, these tests would not be ran. But compare to original implementations, the path level discover need less redundant work to add the test file name one by one, and also it decrease the possibility of forgetting to add test on some level. -- 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-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] Yikun commented on pull request #32867: [SPARK-35721][PYTHON] Path level discover for python unittests
Yikun commented on pull request #32867: URL: https://github.com/apache/spark/pull/32867#issuecomment-870221373 > Just wondering we can only verify it manually? If by any chance, some tests are not found, can we easily know it? It just same as before, if we forgot to add the path of tests, these tests would not be ran. But compare to original implementations, the path level discover need less redundant work to add the test file name one by one, and also it decrease the possibility of forgetting to add test on some level. -- 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-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] Yikun commented on pull request #32867: [SPARK-35721][PYTHON] Path level discover for python unittests
Yikun commented on pull request #32867: URL: https://github.com/apache/spark/pull/32867#issuecomment-869430292 > BTW, this change will likely affect many other vendors who maintain their forks so I will take a close look few more times. Thanks for bearing with me in advance .. ;-). Sure, thanks for your patient 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-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] Yikun commented on pull request #32867: [SPARK-35721][PYTHON] Path level discover for python unittests
Yikun commented on pull request #32867: URL: https://github.com/apache/spark/pull/32867#issuecomment-868934485 @HyukjinKwon Ready for review, it would be good if you could take a look again. : ) -- 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-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
