cloud-fan commented on code in PR #57550:
URL: https://github.com/apache/spark/pull/57550#discussion_r3705084358
##########
python/pyspark/sql/tests/test_job_cancellation.py:
##########
@@ -37,6 +37,23 @@ def test_tags(self):
self.assertEqual(self.spark.getTags(), set())
self.spark.clearTags()
+ def test_invalid_tags(self):
Review Comment:
Please cover removal of a valid absent tag here, as the Scala test does.
This mixin also runs against Connect, where `remove_tag` calls `set.remove` and
raises `KeyError` despite `SparkSession.removeTag` documenting a no-op;
changing it to `discard` would restore parity.
##########
python/pyspark/sql/tests/test_job_cancellation.py:
##########
@@ -37,6 +37,23 @@ def test_tags(self):
self.assertEqual(self.spark.getTags(), set())
self.spark.clearTags()
+ def test_invalid_tags(self):
+ # A tag cannot be an empty string or contain the ',' separator
(documented on
+ # SparkSession.addTag / removeTag). Both the classic and Spark Connect
paths reject
+ # such tags, so assert that an exception is raised and that no invalid
tag leaks into
+ # the tag set. The two paths raise different exception types (a JVM
+ # IllegalArgumentException vs. a PySparkValueError), hence the
assertion is on the
+ # shared contract -- that the call fails -- rather than on a specific
error type.
+ self.spark.clearTags()
+ for invalid_tag in ["", "a,b", ","]:
+ with self.assertRaises(Exception):
Review Comment:
Please assert the expected mode-specific types instead of `Exception`. A
tuple of captured `IllegalArgumentException` and `PySparkValueError` keeps this
shared while preventing unrelated session or transport failures from satisfying
the test.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
