lostluck commented on code in PR #31757:
URL: https://github.com/apache/beam/pull/31757#discussion_r1664610896
##########
sdks/go/pkg/beam/runners/prism/internal/worker/worker_test.go:
##########
@@ -386,3 +387,81 @@ func TestWorker_State_MultimapKeysSideInput(t *testing.T) {
})
}
}
+
+func TestWorker_State_MultimapSideInput(t *testing.T) {
+ for _, tt := range []struct {
+ name string
+ w typex.Window
+ }{
+ {
+ name: "global window",
+ w: window.GlobalWindow{},
+ },
+ {
+ name: "interval window",
+ w: window.IntervalWindow{
+ Start: 1000,
+ End: 2000,
+ },
+ },
+ } {
+ t.Run(tt.name, func(t *testing.T) {
+ var encW []byte
+ if !tt.w.Equals(window.GlobalWindow{}) {
+ buf := bytes.Buffer{}
+ if err :=
exec.MakeWindowEncoder(coder.NewIntervalWindow()).EncodeSingle(tt.w, &buf); err
!= nil {
+ t.Fatalf("error encoding window: %v,
err: %v", tt.w, err)
+ }
+ encW = buf.Bytes()
+ }
+ wk, stateStream, done := serveTestWorkerStateStream(t)
+ defer done()
+ instID := wk.NextInst()
+ wk.activeInstructions[instID] = &B{
+ MultiMapSideInputData:
map[SideInputKey]map[typex.Window]map[string][][]byte{
+ SideInputKey{
+ TransformID: "transformID",
+ Local: "i1",
+ }: {
+ tt.w: map[string][][]byte{"a":
{{5}}, "b": {{12}}},
+ },
+ },
+ }
+ var testKey = []string{"a", "b", "x"}
+ expectedResult := map[string][]int{
Review Comment:
Nit: in Go, we tend to use `got` and `want` as the idiomatic test words for
`what was received after processing` and `results that were desired`.
But TBH since this is a lookup table for any specific `want` I think what's
here is fine vs having `want[key]` downstream. The non-standard name makes the
lookup pattern more visible.
##########
sdks/go/pkg/beam/runners/prism/internal/worker/worker_test.go:
##########
@@ -386,3 +387,81 @@ func TestWorker_State_MultimapKeysSideInput(t *testing.T) {
})
}
}
+
+func TestWorker_State_MultimapSideInput(t *testing.T) {
+ for _, tt := range []struct {
+ name string
+ w typex.Window
+ }{
+ {
+ name: "global window",
+ w: window.GlobalWindow{},
+ },
+ {
+ name: "interval window",
+ w: window.IntervalWindow{
+ Start: 1000,
+ End: 2000,
+ },
+ },
+ } {
+ t.Run(tt.name, func(t *testing.T) {
+ var encW []byte
+ if !tt.w.Equals(window.GlobalWindow{}) {
+ buf := bytes.Buffer{}
+ if err :=
exec.MakeWindowEncoder(coder.NewIntervalWindow()).EncodeSingle(tt.w, &buf); err
!= nil {
+ t.Fatalf("error encoding window: %v,
err: %v", tt.w, err)
+ }
+ encW = buf.Bytes()
+ }
+ wk, stateStream, done := serveTestWorkerStateStream(t)
+ defer done()
+ instID := wk.NextInst()
+ wk.activeInstructions[instID] = &B{
+ MultiMapSideInputData:
map[SideInputKey]map[typex.Window]map[string][][]byte{
+ SideInputKey{
+ TransformID: "transformID",
+ Local: "i1",
+ }: {
+ tt.w: map[string][][]byte{"a":
{{5}}, "b": {{12}}},
+ },
+ },
+ }
+ var testKey = []string{"a", "b", "x"}
+ expectedResult := map[string][]int{
+ "a": {5},
+ "b": {12},
+ }
+ for _, key := range testKey {
+ stateStream.Send(&fnpb.StateRequest{
+ Id: "first",
+ InstructionId: instID,
+ Request: &fnpb.StateRequest_Get{
+ Get: &fnpb.StateGetRequest{},
+ },
+ StateKey: &fnpb.StateKey{Type:
&fnpb.StateKey_MultimapSideInput_{
+ MultimapSideInput:
&fnpb.StateKey_MultimapSideInput{
+ TransformId:
"transformID",
+ SideInputId: "i1",
+ Window: encW,
+ Key:
[]byte(key),
+ },
+ }},
+ })
+
+ resp, err := stateStream.Recv()
+ if err != nil {
+ t.Fatal("Couldn't receive state
response:", err)
+ }
+
+ var got []int
+ for _, b := range resp.GetGet().GetData() {
+ got = append(got, int(b))
+ }
+ if !cmp.Equal(got, expectedResult[key]) {
Review Comment:
Edit: Once I realized that this was comparing just individual primitive
values, there's no real need for cmp.Diff. But here's the original comment I
put down.
-----
Consider using `cmp.Diff` instead which will provided a good printout for
this.
See https://go.dev/wiki/TestComments#print-diffs
Note that for cmp.Diff, you want to pass `want` before `got` so the diff
would be (-want, +got), which shows what's missing from what's wanted, and
extra in what's received.
Aside: You're already doing the other usual bit of advice correctly for the
message here "got before want": See
https://go.dev/wiki/TestComments#got-before-want
--
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]
