damccorm commented on code in PR #17733:
URL: https://github.com/apache/beam/pull/17733#discussion_r879760052


##########
sdks/go/pkg/beam/testing/passert/equals_test.go:
##########
@@ -35,6 +35,16 @@ func TestEquals_Good(t *testing.T) {
        }
 }
 
+func TestEquals_Empty(t *testing.T) {
+       p, s := beam.NewPipelineWithRoot()
+       gotC := beam.CreateList(s, []int{})
+
+       Equals(s, gotC)
+       if err := ptest.Run(p); err != nil {
+               t.Errorf("Pipeline failed: %v", err)
+       }
+}
+

Review Comment:
   Could we add a test or two for windowing equality? Ideally at least 1 
success and 1+ failures



##########
sdks/go/pkg/beam/testing/passert/passert.go:
##########
@@ -44,6 +45,21 @@ func Diff(s beam.Scope, a, b beam.PCollection) (left, both, 
right beam.PCollecti
        return beam.ParDo3(s, &diffFn{Type: beam.EncodedType{T: t.Type()}}, 
imp, beam.SideInput{Input: a}, beam.SideInput{Input: b})
 }

Review Comment:
   I want to call out that we no longer actually use this ourselves, so its 
theoretically a candidate for deprecation/removal. With that said, I think this 
is both public and usable enough that people might have dependencies on it, so 
I don't _really_ think that's a good idea. Mostly just had the thought and 
wanted to see how others feel about it.



##########
sdks/go/pkg/beam/testing/passert/equals.go:
##########
@@ -53,13 +54,37 @@ func EqualsList(s beam.Scope, col beam.PCollection, list 
interface{}) beam.PColl
        return equals(subScope, col, listCollection)
 }
 
+// WindowedEqualsList verifies that the given collection has the same values 
as a
+// given list, under coder equality. The values must be provided as an array 
or a slice.
+// This function also takes a window function to window the list into for 
cases where
+// the PCollections cannot be globally windowed (e.g. tests in unbounded 
pipelines.)
+// This windowing function is applied to both the PCollection created from the 
list
+// and the impulse used to trigger the Diff function.
+func WindowedEqualsList(s beam.Scope, wfn *window.Fn, col beam.PCollection, 
list interface{}) beam.PCollection {

Review Comment:
   Ah, I think this actually doesn't quite do what I was hoping for (though it 
is an improvement IMO), sorry I didn't read your comment closely enough - what 
I'm hoping for is something that would allow you to leave your teststream test 
cases unchanged and have it "just work". That would require windowing both the 
input pcollection and our generated pcollection into some window (e.g. 
bigWinFn).



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

Reply via email to