[GitHub] [beam] lostluck commented on pull request #12170: [BEAM-10166] Improve execution time errors with more concise error messages from DoFns

[GitBox <gi...@apache.org>]

lostluck commented on pull request #12170:
URL: https://github.com/apache/beam/pull/12170#issuecomment-669466247


   This looks good to me! Thank you for the contribution!
   
   Would you mind adding a test to validate this behaviour?
   exec/util_test.go doesn't currently exist, so it would need to be added.
   
   There's no need to test everything in exec/util.go, just callNoPanic.  
   
   You can test three cases: 
   *  one where the passed in func(context.Context) error doesn't panic, and the error is just returned as is (they should compare as equal for a simple error)
    * another test where the the passed in function panics, which should return an error that contains "panic: " in it. 
     * finally, one where the error passed to panic is wrapped in your new doFnError error type.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org