kojiromike commented on code in PR #1181:
URL: https://github.com/apache/avro/pull/1181#discussion_r1265363907
##########
lang/py/avro/test/test_schema.py:
##########
@@ -634,13 +832,29 @@ def test_fixed_decimal_invalid_max_precision(self):
def test_parse_invalid_symbol(self):
"""Disabling enumschema symbol validation should allow invalid symbols
to pass."""
test_schema_string = json.dumps({"type": "enum", "name": "AVRO2174",
"symbols": ["white space"]})
- with self.assertRaises(avro.errors.InvalidName, msg="When enum symbol
validation is enabled, an invalid symbol should raise InvalidName."):
+
+ try:
avro.schema.parse(test_schema_string, validate_enum_symbols=True)
+ except avro.errors.InvalidName:
+ pass
+ else:
+ self.fail("When enum symbol validation is enabled, an invalid
symbol should raise InvalidName.")
Review Comment:
Doesn't the `msg` argument to `assertRaises` display that message?
I'm concerned about a couple of things here:
1. The happy path, where the test passes, actually has no assert statements
in it. So it doesn't look like a test. Some test discovery tools I've worked
with in the past will complain about that, or ignore the test altogether.
2. Since we don't want the test to fail in CI, there's a branch of this code
that cannot be covered. (The `self.fail` part.) So that needs to be excluded
from coverage.
--
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]
