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/05/02 15:30:46 UTC
[GitHub] [spark] pralabhkumar opened a new pull request, #36432: [SPARK-39029][PYTHON][TEST]Improve the test coverage for pyspark/broadcast.py
pralabhkumar opened a new pull request, #36432:
URL: https://github.com/apache/spark/pull/36432
### What changes were proposed in this pull request?
This PR add test cases for broadcast.py
### Why are the changes needed?
To cover corner test cases and increase coverage
### Does this PR introduce _any_ user-facing change?
No - test only
### How was this patch tested?
CI in this PR should test it out
--
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] pralabhkumar commented on pull request #36432: [SPARK-39029][PYTHON][TESTS] Improve the test coverage for pyspark/broadcast.py
Posted by GitBox <gi...@apache.org>.
pralabhkumar commented on PR #36432:
URL: https://github.com/apache/spark/pull/36432#issuecomment-1116359198
@HyukjinKwon Please review the changes . Build is failing may be because of unrelated changes
Execution halted
------------------------------------------------
Jekyll 4.2.1 Please append `--trace` to the `build` command
for any additional information or backtrace.
------------------------------------------------
/__w/spark/spark/docs/_plugins/copy_api_dirs.rb:147:in `<top (required)>': R doc generation failed (RuntimeError)
--
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] HyukjinKwon commented on pull request #36432: [SPARK-39029][PYTHON][TESTS] Improve the test coverage for pyspark/broadcast.py
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on PR #36432:
URL: https://github.com/apache/spark/pull/36432#issuecomment-1116786434
Yeah, I don't think that's related.
Merged to master.
--
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] HyukjinKwon commented on a diff in pull request #36432: [SPARK-39029][PYTHON][TEST]Improve the test coverage for pyspark/broadcast.py
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on code in PR #36432:
URL: https://github.com/apache/spark/pull/36432#discussion_r863260459
##########
python/pyspark/tests/test_broadcast.py:
##########
@@ -99,6 +101,30 @@ def test_broadcast_value_against_gc(self):
finally:
b.destroy()
+ def test_broadcast_when_sc_none(self):
+ # SPARK-39029 : Test case to improve test coverage of broadcast.py
+ # It test the case when sc is none and Broadcast is called at executor
+ conf = SparkConf()
+ conf.setMaster("local-cluster[2,1,1024]")
+ self.sc = SparkContext(conf=conf)
+ bs = self.sc.broadcast([10])
+ bs_sc_none = Broadcast(sc=None, path=bs._path)
+ self.assertEqual(bs_sc_none.value, [10])
+
+ def test_broadcast_for_error_condition(self):
+ # SPARK-39029: Test case to improve test coverage of broadcast.py
+ # It test the case when broadcast should raise error .
+ conf = SparkConf()
+ conf.setMaster("local-cluster[2,1,1024]")
+ self.sc = SparkContext(conf=conf)
+ bs = self.sc.broadcast([1])
+ with self.assertRaisesRegex(pickle.PickleError, "Could.*not.*serialize.*broadcast"):
+ self.sc.broadcast(self.sc)
+ with self.assertRaisesRegex(Exception, "RuntimeError.*Broadcast.*destroyed.*driver"):
Review Comment:
Can we catch narrower exception?
--
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] HyukjinKwon commented on a diff in pull request #36432: [SPARK-39029][PYTHON][TEST]Improve the test coverage for pyspark/broadcast.py
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on code in PR #36432:
URL: https://github.com/apache/spark/pull/36432#discussion_r863260273
##########
python/pyspark/tests/test_broadcast.py:
##########
@@ -99,6 +101,30 @@ def test_broadcast_value_against_gc(self):
finally:
b.destroy()
+ def test_broadcast_when_sc_none(self):
+ # SPARK-39029 : Test case to improve test coverage of broadcast.py
+ # It test the case when sc is none and Broadcast is called at executor
Review Comment:
```suggestion
# It tests the case when SparkContext is none and Broadcast is called at executor
```
##########
python/pyspark/tests/test_broadcast.py:
##########
@@ -99,6 +101,30 @@ def test_broadcast_value_against_gc(self):
finally:
b.destroy()
+ def test_broadcast_when_sc_none(self):
+ # SPARK-39029 : Test case to improve test coverage of broadcast.py
+ # It test the case when sc is none and Broadcast is called at executor
+ conf = SparkConf()
+ conf.setMaster("local-cluster[2,1,1024]")
+ self.sc = SparkContext(conf=conf)
+ bs = self.sc.broadcast([10])
+ bs_sc_none = Broadcast(sc=None, path=bs._path)
+ self.assertEqual(bs_sc_none.value, [10])
+
+ def test_broadcast_for_error_condition(self):
+ # SPARK-39029: Test case to improve test coverage of broadcast.py
+ # It test the case when broadcast should raise error .
Review Comment:
```suggestion
# It tests the case when broadcast should raise error .
```
--
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] HyukjinKwon closed pull request #36432: [SPARK-39029][PYTHON][TESTS] Improve the test coverage for pyspark/broadcast.py
Posted by GitBox <gi...@apache.org>.
HyukjinKwon closed pull request #36432: [SPARK-39029][PYTHON][TESTS] Improve the test coverage for pyspark/broadcast.py
URL: https://github.com/apache/spark/pull/36432
--
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] AmplabJenkins commented on pull request #36432: [SPARK-39029][PYTHON][TEST]Improve the test coverage for pyspark/broadcast.py
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on PR #36432:
URL: https://github.com/apache/spark/pull/36432#issuecomment-1115228427
Can one of the admins verify this patch?
--
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