kennknowles commented on PR #40865: URL: https://github.com/apache/beam/pull/40865#issuecomment-6477674030
> Thanks for working on this! I appreciate the effort to remove the deprecated `DoFnTester`. > > However, based on the original Jira issue ([BEAM-3159](https://issues.apache.org/jira/browse/BEAM-3159?focusedCommentId=16340765&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16340765)), it seems the goal was to encourage using `TestStream` for overall behavior rather than testing internal elements so deeply. I'm a bit concerned that `ProcessFnTester` might just carry over the pattern we were trying to move away from and become another `DoFnTester`. > > Isn't that our target to convert these tests to use `TestStream` instead? WDYT? @kennknowles I agree with you, in spirit. In this case I think it is OK to merge for these reasons: 1. `ProcessFnTester` is a private class so it is OK for it to be used in specialized cases and also can be removed or changed any time. 2. It advances things a little bit by removing a use of DoFnTester. I admit that it was a pretty quick review, based on what this particular PR does, so @shunping please take a look. -- 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]
