lostluck commented on a change in pull request #16575:
URL: https://github.com/apache/beam/pull/16575#discussion_r788940176
##########
File path: sdks/go/pkg/beam/core/runtime/exec/pardo_test.go
##########
@@ -132,10 +132,12 @@ func BenchmarkParDo_EmitSumFn(b *testing.B) {
}
go func() {
if err := p.Execute(context.Background(), "1", DataContext{});
err != nil {
- b.Fatalf("execute failed: %v", err)
+ b.Errorf("execute failed: %v", err)
Review comment:
I gather the issue vet/staticcheck has is that the "fatal" doesn't end
the test?
Since usually seeing the combo of "error " return means one must translate
it back to fatal, consider adding another comment to the effect about the
goroutines, and the vet warning. I assume though that this fix lets staticcheck
and vet run cleanly on this issue.
Alternatively to unambiguously avoid the different lint trigger: Add a new
buffered channel `errchan` that takes in a string. Then send the message and
close the channel (though a `defer close(errchan)` at the top of the goroutine
is also good.
Receive on the errchan at the end of the benchmark function (after closing
`process`), and print out any out put to b.Fatal. (Yes we lose line locality,
but we can make that up in message verbosity). This then blocks the benchmark
until the goroutine shuts down, which will close the channel and unblock exit.
```
close(process)
for msg := range errchan {
b.Fatal(msg)
}
```
The rule of thumb here is "the channel writer should close the channel".
--
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]
