[
https://issues.apache.org/jira/browse/BEAM-14546?focusedWorklogId=777639&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-777639
]
ASF GitHub Bot logged work on BEAM-14546:
-----------------------------------------
Author: ASF GitHub Bot
Created on: 02/Jun/22 15:25
Start Date: 02/Jun/22 15:25
Worklog Time Spent: 10m
Work Description: damccorm commented on code in PR #17813:
URL: https://github.com/apache/beam/pull/17813#discussion_r888079762
##########
sdks/go/pkg/beam/testing/passert/count.go:
##########
@@ -30,6 +30,10 @@ func Count(s beam.Scope, col beam.PCollection, name string,
count int) {
if typex.IsKV(col.Type()) {
col = beam.DropKey(s, col)
}
+
+ if count > 0 {
+ NonEmpty(s, col)
Review Comment:
Do we need to add the same thing to
[Hash](https://github.com/apache/beam/blob/9e986e74efff2a7293ec5c7a86917479882d9590/sdks/go/pkg/beam/testing/passert/hash.go#L36)
and
[Sum](https://github.com/apache/beam/blob/9e986e74efff2a7293ec5c7a86917479882d9590/sdks/go/pkg/beam/testing/passert/sum.go#L33)?
I think an empty pcollection would silently pass for both of those as well
##########
sdks/go/pkg/beam/testing/passert/count_test.go:
##########
@@ -22,24 +22,62 @@ import (
"github.com/apache/beam/sdks/v2/go/pkg/beam/testing/ptest"
)
-func TestCount_Good(t *testing.T) {
- p, s := beam.NewPipelineWithRoot()
- col := beam.Create(s, "a", "b", "c", "d", "e")
- count := 5
+func TestCount(t *testing.T) {
+ var tests = []struct {
+ name string
+ elements []string
+ count int
+ }{
+ {
+ "full",
+ []string{"a", "b", "c", "d", "e"},
+ 5,
+ },
+ {
+ "empty",
+ []string{},
+ 0,
+ },
+ }
+ for _, test := range tests {
+ t.Run(test.name, func(t *testing.T) {
+ p, s := beam.NewPipelineWithRoot()
+ col := beam.CreateList(s, test.elements)
- Count(s, col, "TestCount_Good", count)
- if err := ptest.Run(p); err != nil {
- t.Errorf("Pipeline failed: %v", err)
+ Count(s, col, test.name, test.count)
+ if err := ptest.Run(p); err != nil {
+ t.Errorf("Pipeline failed: %v", err)
+ }
+ })
}
}
func TestCount_Bad(t *testing.T) {
Review Comment:
Can we simplify and combine these tests into 1 by adding a `expectErr`
variable to the tests struct?
Issue Time Tracking
-------------------
Worklog Id: (was: 777639)
Time Spent: 1h (was: 50m)
> [Go SDK] passert.Count succeeds for empty PCollections.
> -------------------------------------------------------
>
> Key: BEAM-14546
> URL: https://issues.apache.org/jira/browse/BEAM-14546
> Project: Beam
> Issue Type: Improvement
> Components: sdk-go
> Reporter: Robert Burke
> Assignee: Jack McCluskey
> Priority: P3
> Time Spent: 1h
> Remaining Estimate: 0h
>
> https://github.com/apache/beam/blob/sdks/v2.39.0/sdks/go/pkg/beam/testing/passert/count.go#L28
> Since it's using a Combine to do the count, it never executes for empty
> Pcollections, and is unable to fail.
> The fix is: when count > 0, plumb the pcollection through as a side input to
> a DoFn that requires the side input to be non-empty. This would catch the
> empty PCollection case.
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
