You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "vito-lin-dev (via GitHub)" <gi...@apache.org> on 2026/10/11 05:54:54 UTC
[PR] [SPARK-60149][PYTHON][TESTS] Add tests for ErrorClassesReader lookup and breaking_change_info fallback [spark]
vito-lin-dev opened a new pull request, #59352:
URL: https://github.com/apache/spark/pull/59352
### What changes were proposed in this pull request?
This PR adds tests to `python/pyspark/errors/tests/test_errors.py` for code paths in `ErrorClassesReader` (`python/pyspark/errors/utils.py`) that were not covered:
- `get_message_template`: raising `ValueError` when the main class exists but the sub-class does not
- `get_breaking_change_info`:
- returning `None` for a `None` error class
- raising `ValueError` for an unknown main class or sub-class
- falling back to the main class's `breaking_change_info` when the sub-class has none
- returning `None` when neither the main class nor the sub-class declares it
- `get_sqlstate`: returning `None` for a `None` error class
### Why are the changes needed?
To improve test coverage of `pyspark.errors.utils.ErrorClassesReader`. These error-handling and fallback branches had no tests, so regressions in them would not be caught.
### Does this PR introduce _any_ user-facing change?
No. Test-only change.
### How was this patch tested?
Added unit tests in `python/pyspark/errors/tests/test_errors.py`.
Ran the test module with coverage before and after the change:
```
python/run-tests-with-coverage --python-executables=python3 --testnames pyspark.errors.tests.test_errors
```
Coverage of `pyspark/errors/utils.py`:
| | Stmts | Miss | BrPart | Cover |
|---|---|---|---|---|
| Before | 152 | 72 | 7 | 49% |
| After | 152 | 66 | 0 | 55% |
All tests pass. `dev/reformat-python` and `dev/lint-python` pass for the changed file.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Fable 5.1 (Anthropic), used as a pair-programming assistant. The author reviewed every line, ran the tests, and verified the coverage results locally.
--
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
Re: [PR] [SPARK-60149][PYTHON][TESTS] Add tests for ErrorClassesReader lookup and breaking_change_info fallback [spark]
Posted by "gaogaotiantian (via GitHub)" <gi...@apache.org>.
gaogaotiantian commented on code in PR #59352:
URL: https://github.com/apache/spark/pull/59352#discussion_r4240536635
##########
python/pyspark/errors/tests/test_errors.py:
##########
@@ -114,6 +114,59 @@ def test_breaking_change_info(self):
subclass_map = error_reader.error_info_map["TEST_ERROR_WITH_SUB_CLASS"]["sub_class"]
self.assertEqual(breaking_change_info2, subclass_map["SUBCLASS"]["breaking_change_info"])
+ def test_message_template_sub_class_not_found(self):
+ # get_message_template raises when the main class exists but the sub-class does not.
+ error_reader = ErrorClassesReader()
+ error_reader.error_info_map = {
+ "TEST_ERROR": {
+ "message": ["Error message."],
+ "sub_class": {"SUBCLASS": {"message": ["Subclass message."]}},
+ },
+ }
+ with self.assertRaisesRegex(ValueError, "Cannot find sub error class 'NON_EXISTENT_SUB'"):
+ error_reader.get_message_template("TEST_ERROR.NON_EXISTENT_SUB")
+
+ def test_breaking_change_info_lookup_errors(self):
Review Comment:
Do we need two new methods for `breaking_change_info`? We already have a test and an `error_info_map` in `test_breaking_change_info`. Can we just add some access that raise exceptions that we need to test? We don't need these many methods.
--
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
Re: [PR] [SPARK-60149][PYTHON][TESTS] Add tests for ErrorClassesReader lookup and breaking_change_info fallback [spark]
Posted by "vito-lin-dev (via GitHub)" <gi...@apache.org>.
vito-lin-dev commented on code in PR #59352:
URL: https://github.com/apache/spark/pull/59352#discussion_r4240602586
##########
python/pyspark/errors/tests/test_errors.py:
##########
@@ -114,6 +114,59 @@ def test_breaking_change_info(self):
subclass_map = error_reader.error_info_map["TEST_ERROR_WITH_SUB_CLASS"]["sub_class"]
self.assertEqual(breaking_change_info2, subclass_map["SUBCLASS"]["breaking_change_info"])
+ def test_message_template_sub_class_not_found(self):
+ # get_message_template raises when the main class exists but the sub-class does not.
+ error_reader = ErrorClassesReader()
+ error_reader.error_info_map = {
+ "TEST_ERROR": {
+ "message": ["Error message."],
+ "sub_class": {"SUBCLASS": {"message": ["Subclass message."]}},
+ },
+ }
+ with self.assertRaisesRegex(ValueError, "Cannot find sub error class 'NON_EXISTENT_SUB'"):
+ error_reader.get_message_template("TEST_ERROR.NON_EXISTENT_SUB")
+
+ def test_breaking_change_info_lookup_errors(self):
Review Comment:
@gaogaotiantian Thanks, I merged two methods into `test_breaking_change_info` one.
--
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
Re: [PR] [SPARK-60149][PYTHON][TESTS] Add tests for ErrorClassesReader lookup errors and breaking_change_info fallback [spark]
Posted by "gaogaotiantian (via GitHub)" <gi...@apache.org>.
gaogaotiantian commented on PR #59352:
URL: https://github.com/apache/spark/pull/59352#issuecomment-6430513415
**Merge Summary:**
- merged into master https://github.com/apache/spark/commit/f784da87e6db27672ab778c79d1a76420d7c3b83
- merged into branch-4.x https://github.com/apache/spark/commit/b4a692959b4a553cf136837c4bd7d55591ebbc1f
*Posted by `merge_spark_pr.py`*
--
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
Re: [PR] [SPARK-60149][PYTHON][TESTS] Add tests for ErrorClassesReader lookup errors and breaking_change_info fallback [spark]
Posted by "gaogaotiantian (via GitHub)" <gi...@apache.org>.
gaogaotiantian commented on PR #59352:
URL: https://github.com/apache/spark/pull/59352#issuecomment-6430522985
Congrats to your first PR to Spark!
--
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
Re: [PR] [SPARK-60149][PYTHON][TESTS] Add tests for ErrorClassesReader lookup errors and breaking_change_info fallback [spark]
Posted by "gaogaotiantian (via GitHub)" <gi...@apache.org>.
gaogaotiantian closed pull request #59352: [SPARK-60149][PYTHON][TESTS] Add tests for ErrorClassesReader lookup errors and breaking_change_info fallback
URL: https://github.com/apache/spark/pull/59352
--
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