chrisqiqiu commented on code in PR #38429:
URL: https://github.com/apache/beam/pull/38429#discussion_r3215120155
##########
sdks/python/apache_beam/runners/common_test.py:
##########
@@ -154,6 +154,43 @@ def process(self, element, mykey=DoFn.KeyParam):
test_stream = (TestStream().advance_watermark_to(10).add_elements([1,
2]))
(p | test_stream | beam.ParDo(DoFnProcessWithKeyparam()))
+ def test_dofn_returning_str_raises_clear_error(self):
+ """Regression test for https://github.com/apache/beam/issues/18712.
+
+ A DoFn returning a str instead of an iterable wrapping one used to
+ silently iterate per-character. It should now raise a clear TypeError.
+ """
+ class BadDoFn(DoFn):
+ def process(self, element):
+ return 'hello'
+
+ with self.assertRaisesRegex(
+ Exception, 'Returning a str from a ParDo or FlatMap is discouraged'):
Review Comment:
Tested locally — `assertRaisesRegex(TypeError, ...)` fails because
`DoFnRunner._reraise_augmented` wraps the `TypeError` before it surfaces to the
test framework. The trace still shows the original `TypeError`, but
`isinstance` against the wrapped exception class doesn't match.
Keeping `Exception` to match the existing convention in
[`typecheck_test.py:test_do_fn_returning_non_iterable_throws_error`](https://github.com/apache/beam/blob/master/sdks/python/apache_beam/typehints/typecheck_test.py#L191-L198),
which uses `Exception` for the same reason. Added a code comment explaining
this.
--
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]
