[ 
https://issues.apache.org/jira/browse/BEAM-9883?focusedWorklogId=431871&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-431871
 ]

ASF GitHub Bot logged work on BEAM-9883:
----------------------------------------

                Author: ASF GitHub Bot
            Created on: 07/May/20 19:35
            Start Date: 07/May/20 19:35
    Worklog Time Spent: 10m 
      Work Description: lostluck commented on a change in pull request #11605:
URL: https://github.com/apache/beam/pull/11605#discussion_r421731826



##########
File path: sdks/go/pkg/beam/core/runtime/exec/sdf_invokers_test.go
##########
@@ -210,86 +229,154 @@ func TestInvokes(t *testing.T) {
        })
 }
 
-type Restriction struct {
-       Val int
+// VetRestriction is a restriction used for validating that SDF methods get
+// called with it. When using VetRestriction, the SDF methods it's used in
+// should pass it as a pointer so the method can make changes to the 
restriction
+// even if it doesn't output one directly (such as RestrictionSize).
+//
+type VetRestriction struct {
+       // An identifier to differentiate restrictions on the same elements. 
When
+       // split, a suffix in the form of ".#" is appended to this ID.
+       ID string
+
+       // Key and Val just copy the last seen input element's key and value to
+       // confirm that the restriction saw the expected element.
+       Key, Val interface{}
+
+       // These booleans should be flipped to true by the corresponding SDF 
methods
+       // to prove that the methods got called on the restriction.
+       CreateRest, SplitRest, RestSize, CreateTracker, ProcessElm bool
 }
 
-// RTracker's methods can all be no-ops, we just need it to implement 
sdf.RTracker.
-type RTracker struct {
-       Rest Restriction
-       Val  int
+// VetRTracker's methods can all be no-ops, we just need it to implement
+// sdf.RTracker and allow validating that it was passed to ProcessElement.
+type VetRTracker struct {
+       Rest *VetRestriction
 }
 
-func (rt *RTracker) TryClaim(interface{}) bool       { return false }
-func (rt *RTracker) GetError() error                 { return nil }
-func (rt *RTracker) GetProgress() (float64, float64) { return 0, 0 }
-func (rt *RTracker) IsDone() bool                    { return true }
-func (rt *RTracker) TrySplit(fraction float64) (interface{}, interface{}, 
error) {
+func (rt *VetRTracker) TryClaim(interface{}) bool       { return false }
+func (rt *VetRTracker) GetError() error                 { return nil }
+func (rt *VetRTracker) GetProgress() (float64, float64) { return 0, 0 }
+func (rt *VetRTracker) IsDone() bool                    { return true }
+func (rt *VetRTracker) TrySplit(fraction float64) (interface{}, interface{}, 
error) {
        return nil, nil, nil
 }
 
-// In order to test that these methods get called properly, each one has an
-// implementation that lets us confirm that each argument was passed properly.
-
-type Sdf struct {
+// VetSdf runs an SDF In order to test that these methods get called properly,
+// each method will flip the corresponding flag in the passed in 
VetRestriction,
+// overwrite the restriction's Key and Val with the last seen input elements,
+// and retain the other fields in the VetRestriction.
+type VetSdf struct {
 }
 
-// CreateInitialRestriction creates a restriction with the given value.
-func (fn *Sdf) CreateInitialRestriction(i int) Restriction {
-       return Restriction{i}
+// CreateInitialRestriction creates a restriction with the given values and
+// with the appropriate flags to track that this was called.
+func (fn *VetSdf) CreateInitialRestriction(i int) *VetRestriction {
+       return &VetRestriction{ID: "Sdf", Val: i, CreateRest: true}
 }
 
-// SplitRestriction outputs two restrictions, the first containing the sum of i
-// and rest.Val, the second containing the same value plus 1.
-func (fn *Sdf) SplitRestriction(i int, rest Restriction) []Restriction {
-       return []Restriction{{rest.Val + i}, {rest.Val + i + 1}}
+// SplitRestriction outputs two identical restrictions, each being a copy of 
the
+// initial one, but with the appropriate flags to track this was called. The
+// split restrictions add a suffix of the form ".#" to the ID.
+func (fn *VetSdf) SplitRestriction(i int, rest *VetRestriction) 
[]*VetRestriction {
+       rest.SplitRest = true
+       rest1 := &VetRestriction{
+               ID:            rest.ID + ".1",
+               Val:           i,
+               CreateRest:    rest.CreateRest,
+               SplitRest:     true,
+               RestSize:      rest.RestSize,
+               CreateTracker: rest.CreateTracker,
+               ProcessElm:    rest.ProcessElm,
+       }
+       rest2 := &VetRestriction{}
+       *rest2 = *rest1
+       rest2.ID = rest.ID + ".2"
+       return []*VetRestriction{rest1, rest2}
 }
 
-// RestrictionSize returns the sum of i and rest.Val as a float64.
-func (fn *Sdf) RestrictionSize(i int, rest Restriction) float64 {
-       return (float64)(i + rest.Val)
+// RestrictionSize just returns i as the size, as well as flipping appropriate
+// flags on the restriction to track that this was called.
+func (fn *VetSdf) RestrictionSize(i int, rest *VetRestriction) float64 {
+       rest.Key = nil
+       rest.Val = i
+       rest.RestSize = true
+       return (float64)(i)
 }
 
-// CreateTracker creates an RTracker containing the given restriction and a Val
-// of 1.
-func (fn *Sdf) CreateTracker(rest Restriction) *RTracker {
-       return &RTracker{rest, 1}
+// CreateTracker creates an RTracker containing the given restriction and flips
+// the appropriate flags on the restriction to track that this was called.
+func (fn *VetSdf) CreateTracker(rest *VetRestriction) *VetRTracker {
+       rest.CreateTracker = true
+       return &VetRTracker{rest}
 }
 
-// ProcessElement emits a pair of ints. The first is the input +
-// RTracker.Rest.Val. The second is the input + RTracker.Val.
-func (fn *Sdf) ProcessElement(rt *RTracker, i int, emit func(int, int)) {
-       emit(i+rt.Rest.Val, i+rt.Val)
+// ProcessElement emits a copy of the restriction in the restriction tracker it
+// received, with the appropriate flags flipped to track that this was called.
+func (fn *VetSdf) ProcessElement(rt *VetRTracker, i int, emit 
func(VetRestriction)) {
+       rest := *rt.Rest
+       rest.Key = nil
+       rest.Val = i
+       rest.ProcessElm = true
+       emit(rest)
 }
 
-type KvSdf struct {
+// VetKvSdf runs an SDF In order to test that these methods get called 
properly,
+// each method will flip the corresponding flag in the passed in 
VetRestriction,
+// overwrite the restriction's Key and Val with the last seen input elements,
+// and retain the other fields in the VetRestriction.
+type VetKvSdf struct {
 }
 
-// CreateInitialRestriction creates a restriction with the sum of the given
-// values.
-func (fn *KvSdf) CreateInitialRestriction(i int, j int) Restriction {
-       return Restriction{i + j}
+// CreateInitialRestriction creates a restriction with the given values and
+// with the appropriate flags to track that this was called.
+func (fn *VetKvSdf) CreateInitialRestriction(i, j int) *VetRestriction {
+       return &VetRestriction{ID: "KvSdf", Key: i, Val: j, CreateRest: true}
 }
 
-// SplitRestriction outputs two restrictions, the first containing the sum of i
-// and rest.Val, the second containing the sum of j and rest.Val.
-func (fn *KvSdf) SplitRestriction(i int, j int, rest Restriction) 
[]Restriction {
-       return []Restriction{{rest.Val + i}, {rest.Val + j}}
+// SplitRestriction outputs two identical restrictions, each being a copy of 
the
+// initial one, but with the appropriate flags to track this was called. The
+// split restrictions add a suffix of the form ".#" to the ID.
+func (fn *VetKvSdf) SplitRestriction(i, j int, rest *VetRestriction) 
[]*VetRestriction {
+       rest.SplitRest = true
+       rest1 := &VetRestriction{
+               ID:            rest.ID + ".1",
+               Key:           i,
+               Val:           j,
+               CreateRest:    rest.CreateRest,
+               SplitRest:     true,
+               RestSize:      rest.RestSize,
+               CreateTracker: rest.CreateTracker,
+               ProcessElm:    rest.ProcessElm,
+       }
+       rest2 := &VetRestriction{}
+       *rest2 = *rest1
+       rest2.ID = rest.ID + ".2"
+       return []*VetRestriction{rest1, rest2}
 }
 
-// RestrictionSize returns the sum of i, j, and rest.Val as a float64.
-func (fn *KvSdf) RestrictionSize(i int, j int, rest Restriction) float64 {
-       return (float64)(i + j + rest.Val)
+// RestrictionSize just returns the sum of i and j as the size, as well as
+// flipping appropriate flags on the restriction to track that this was called.
+func (fn *VetKvSdf) RestrictionSize(i, j int, rest *VetRestriction) float64 {
+       rest.Key = i
+       rest.Val = j
+       rest.RestSize = true
+       return (float64)(i + j)
 }
 
-// CreateTracker creates an RTracker containing the given restriction and a Val
-// of 2.
-func (fn *KvSdf) CreateTracker(rest Restriction) *RTracker {
-       return &RTracker{rest, 2}
+// CreateTracker creates an RTracker containing the given restriction and flips
+// the appropriate flags on the restriction to track that this was called.
+func (fn *VetKvSdf) CreateTracker(rest *VetRestriction) *VetRTracker {
+       rest.CreateTracker = true
+       return &VetRTracker{rest}
 }
 
-// ProcessElement emits two ints. The first is the first input (key) +
-// RTracker.Rest.Val. The second is the second input (value) + RTracker.Val.
-func (fn *KvSdf) ProcessElement(rt *RTracker, i1 int, i2 int, emit func(int, 
int)) {
-       emit(i1+rt.Rest.Val, i2+rt.Val)
+// ProcessElement emits a copy of the restriction in the restriction tracker it
+// received, with the appropriate flags flipped to track that this was called.
+func (fn *VetKvSdf) ProcessElement(rt *VetRTracker, i, j int, emit 
func(VetRestriction)) {

Review comment:
       While this is a Test SDF, consider documenting explicitly that returning 
a restriction is an artifact of testing SDFs, and not how a correct SDF is 
written.  

##########
File path: sdks/go/pkg/beam/core/runtime/exec/sdf_invokers_test.go
##########
@@ -210,86 +229,154 @@ func TestInvokes(t *testing.T) {
        })
 }
 
-type Restriction struct {
-       Val int
+// VetRestriction is a restriction used for validating that SDF methods get
+// called with it. When using VetRestriction, the SDF methods it's used in
+// should pass it as a pointer so the method can make changes to the 
restriction
+// even if it doesn't output one directly (such as RestrictionSize).
+//
+type VetRestriction struct {
+       // An identifier to differentiate restrictions on the same elements. 
When
+       // split, a suffix in the form of ".#" is appended to this ID.
+       ID string
+
+       // Key and Val just copy the last seen input element's key and value to
+       // confirm that the restriction saw the expected element.
+       Key, Val interface{}
+
+       // These booleans should be flipped to true by the corresponding SDF 
methods
+       // to prove that the methods got called on the restriction.
+       CreateRest, SplitRest, RestSize, CreateTracker, ProcessElm bool
 }
 
-// RTracker's methods can all be no-ops, we just need it to implement 
sdf.RTracker.
-type RTracker struct {
-       Rest Restriction
-       Val  int
+// VetRTracker's methods can all be no-ops, we just need it to implement
+// sdf.RTracker and allow validating that it was passed to ProcessElement.
+type VetRTracker struct {
+       Rest *VetRestriction
 }
 
-func (rt *RTracker) TryClaim(interface{}) bool       { return false }
-func (rt *RTracker) GetError() error                 { return nil }
-func (rt *RTracker) GetProgress() (float64, float64) { return 0, 0 }
-func (rt *RTracker) IsDone() bool                    { return true }
-func (rt *RTracker) TrySplit(fraction float64) (interface{}, interface{}, 
error) {
+func (rt *VetRTracker) TryClaim(interface{}) bool       { return false }
+func (rt *VetRTracker) GetError() error                 { return nil }
+func (rt *VetRTracker) GetProgress() (float64, float64) { return 0, 0 }
+func (rt *VetRTracker) IsDone() bool                    { return true }
+func (rt *VetRTracker) TrySplit(fraction float64) (interface{}, interface{}, 
error) {
        return nil, nil, nil
 }
 
-// In order to test that these methods get called properly, each one has an
-// implementation that lets us confirm that each argument was passed properly.
-
-type Sdf struct {
+// VetSdf runs an SDF In order to test that these methods get called properly,
+// each method will flip the corresponding flag in the passed in 
VetRestriction,
+// overwrite the restriction's Key and Val with the last seen input elements,
+// and retain the other fields in the VetRestriction.
+type VetSdf struct {
 }
 
-// CreateInitialRestriction creates a restriction with the given value.
-func (fn *Sdf) CreateInitialRestriction(i int) Restriction {
-       return Restriction{i}
+// CreateInitialRestriction creates a restriction with the given values and
+// with the appropriate flags to track that this was called.
+func (fn *VetSdf) CreateInitialRestriction(i int) *VetRestriction {
+       return &VetRestriction{ID: "Sdf", Val: i, CreateRest: true}
 }
 
-// SplitRestriction outputs two restrictions, the first containing the sum of i
-// and rest.Val, the second containing the same value plus 1.
-func (fn *Sdf) SplitRestriction(i int, rest Restriction) []Restriction {
-       return []Restriction{{rest.Val + i}, {rest.Val + i + 1}}
+// SplitRestriction outputs two identical restrictions, each being a copy of 
the
+// initial one, but with the appropriate flags to track this was called. The
+// split restrictions add a suffix of the form ".#" to the ID.
+func (fn *VetSdf) SplitRestriction(i int, rest *VetRestriction) 
[]*VetRestriction {
+       rest.SplitRest = true
+       rest1 := &VetRestriction{
+               ID:            rest.ID + ".1",
+               Val:           i,
+               CreateRest:    rest.CreateRest,
+               SplitRest:     true,
+               RestSize:      rest.RestSize,
+               CreateTracker: rest.CreateTracker,
+               ProcessElm:    rest.ProcessElm,
+       }
+       rest2 := &VetRestriction{}
+       *rest2 = *rest1
+       rest2.ID = rest.ID + ".2"
+       return []*VetRestriction{rest1, rest2}
 }
 
-// RestrictionSize returns the sum of i and rest.Val as a float64.
-func (fn *Sdf) RestrictionSize(i int, rest Restriction) float64 {
-       return (float64)(i + rest.Val)
+// RestrictionSize just returns i as the size, as well as flipping appropriate
+// flags on the restriction to track that this was called.
+func (fn *VetSdf) RestrictionSize(i int, rest *VetRestriction) float64 {
+       rest.Key = nil
+       rest.Val = i
+       rest.RestSize = true
+       return (float64)(i)
 }
 
-// CreateTracker creates an RTracker containing the given restriction and a Val
-// of 1.
-func (fn *Sdf) CreateTracker(rest Restriction) *RTracker {
-       return &RTracker{rest, 1}
+// CreateTracker creates an RTracker containing the given restriction and flips
+// the appropriate flags on the restriction to track that this was called.
+func (fn *VetSdf) CreateTracker(rest *VetRestriction) *VetRTracker {
+       rest.CreateTracker = true
+       return &VetRTracker{rest}
 }
 
-// ProcessElement emits a pair of ints. The first is the input +
-// RTracker.Rest.Val. The second is the input + RTracker.Val.
-func (fn *Sdf) ProcessElement(rt *RTracker, i int, emit func(int, int)) {
-       emit(i+rt.Rest.Val, i+rt.Val)
+// ProcessElement emits a copy of the restriction in the restriction tracker it
+// received, with the appropriate flags flipped to track that this was called.
+func (fn *VetSdf) ProcessElement(rt *VetRTracker, i int, emit 
func(VetRestriction)) {
+       rest := *rt.Rest
+       rest.Key = nil
+       rest.Val = i
+       rest.ProcessElm = true
+       emit(rest)
 }
 
-type KvSdf struct {
+// VetKvSdf runs an SDF In order to test that these methods get called 
properly,
+// each method will flip the corresponding flag in the passed in 
VetRestriction,
+// overwrite the restriction's Key and Val with the last seen input elements,
+// and retain the other fields in the VetRestriction.
+type VetKvSdf struct {
 }
 
-// CreateInitialRestriction creates a restriction with the sum of the given
-// values.
-func (fn *KvSdf) CreateInitialRestriction(i int, j int) Restriction {
-       return Restriction{i + j}
+// CreateInitialRestriction creates a restriction with the given values and
+// with the appropriate flags to track that this was called.
+func (fn *VetKvSdf) CreateInitialRestriction(i, j int) *VetRestriction {
+       return &VetRestriction{ID: "KvSdf", Key: i, Val: j, CreateRest: true}
 }
 
-// SplitRestriction outputs two restrictions, the first containing the sum of i
-// and rest.Val, the second containing the sum of j and rest.Val.
-func (fn *KvSdf) SplitRestriction(i int, j int, rest Restriction) 
[]Restriction {
-       return []Restriction{{rest.Val + i}, {rest.Val + j}}
+// SplitRestriction outputs two identical restrictions, each being a copy of 
the
+// initial one, but with the appropriate flags to track this was called. The
+// split restrictions add a suffix of the form ".#" to the ID.
+func (fn *VetKvSdf) SplitRestriction(i, j int, rest *VetRestriction) 
[]*VetRestriction {
+       rest.SplitRest = true
+       rest1 := &VetRestriction{
+               ID:            rest.ID + ".1",
+               Key:           i,
+               Val:           j,
+               CreateRest:    rest.CreateRest,
+               SplitRest:     true,
+               RestSize:      rest.RestSize,
+               CreateTracker: rest.CreateTracker,
+               ProcessElm:    rest.ProcessElm,
+       }
+       rest2 := &VetRestriction{}

Review comment:
       Consider having a copy() method on VetRestriction (on the value), which 
will make all these copy instances easier to read.

##########
File path: sdks/go/pkg/beam/core/runtime/exec/sdf_invokers_test.go
##########
@@ -210,86 +229,154 @@ func TestInvokes(t *testing.T) {
        })
 }
 
-type Restriction struct {
-       Val int
+// VetRestriction is a restriction used for validating that SDF methods get
+// called with it. When using VetRestriction, the SDF methods it's used in
+// should pass it as a pointer so the method can make changes to the 
restriction
+// even if it doesn't output one directly (such as RestrictionSize).
+//
+type VetRestriction struct {
+       // An identifier to differentiate restrictions on the same elements. 
When
+       // split, a suffix in the form of ".#" is appended to this ID.
+       ID string
+
+       // Key and Val just copy the last seen input element's key and value to
+       // confirm that the restriction saw the expected element.
+       Key, Val interface{}
+
+       // These booleans should be flipped to true by the corresponding SDF 
methods
+       // to prove that the methods got called on the restriction.
+       CreateRest, SplitRest, RestSize, CreateTracker, ProcessElm bool
 }
 
-// RTracker's methods can all be no-ops, we just need it to implement 
sdf.RTracker.
-type RTracker struct {
-       Rest Restriction
-       Val  int
+// VetRTracker's methods can all be no-ops, we just need it to implement
+// sdf.RTracker and allow validating that it was passed to ProcessElement.
+type VetRTracker struct {
+       Rest *VetRestriction
 }
 
-func (rt *RTracker) TryClaim(interface{}) bool       { return false }
-func (rt *RTracker) GetError() error                 { return nil }
-func (rt *RTracker) GetProgress() (float64, float64) { return 0, 0 }
-func (rt *RTracker) IsDone() bool                    { return true }
-func (rt *RTracker) TrySplit(fraction float64) (interface{}, interface{}, 
error) {
+func (rt *VetRTracker) TryClaim(interface{}) bool       { return false }
+func (rt *VetRTracker) GetError() error                 { return nil }
+func (rt *VetRTracker) GetProgress() (float64, float64) { return 0, 0 }
+func (rt *VetRTracker) IsDone() bool                    { return true }
+func (rt *VetRTracker) TrySplit(fraction float64) (interface{}, interface{}, 
error) {
        return nil, nil, nil
 }
 
-// In order to test that these methods get called properly, each one has an
-// implementation that lets us confirm that each argument was passed properly.
-
-type Sdf struct {
+// VetSdf runs an SDF In order to test that these methods get called properly,
+// each method will flip the corresponding flag in the passed in 
VetRestriction,
+// overwrite the restriction's Key and Val with the last seen input elements,
+// and retain the other fields in the VetRestriction.
+type VetSdf struct {
 }
 
-// CreateInitialRestriction creates a restriction with the given value.
-func (fn *Sdf) CreateInitialRestriction(i int) Restriction {
-       return Restriction{i}
+// CreateInitialRestriction creates a restriction with the given values and
+// with the appropriate flags to track that this was called.
+func (fn *VetSdf) CreateInitialRestriction(i int) *VetRestriction {
+       return &VetRestriction{ID: "Sdf", Val: i, CreateRest: true}
 }
 
-// SplitRestriction outputs two restrictions, the first containing the sum of i
-// and rest.Val, the second containing the same value plus 1.
-func (fn *Sdf) SplitRestriction(i int, rest Restriction) []Restriction {
-       return []Restriction{{rest.Val + i}, {rest.Val + i + 1}}
+// SplitRestriction outputs two identical restrictions, each being a copy of 
the
+// initial one, but with the appropriate flags to track this was called. The
+// split restrictions add a suffix of the form ".#" to the ID.
+func (fn *VetSdf) SplitRestriction(i int, rest *VetRestriction) 
[]*VetRestriction {
+       rest.SplitRest = true
+       rest1 := &VetRestriction{
+               ID:            rest.ID + ".1",
+               Val:           i,
+               CreateRest:    rest.CreateRest,
+               SplitRest:     true,
+               RestSize:      rest.RestSize,
+               CreateTracker: rest.CreateTracker,
+               ProcessElm:    rest.ProcessElm,
+       }
+       rest2 := &VetRestriction{}
+       *rest2 = *rest1
+       rest2.ID = rest.ID + ".2"
+       return []*VetRestriction{rest1, rest2}
 }
 
-// RestrictionSize returns the sum of i and rest.Val as a float64.
-func (fn *Sdf) RestrictionSize(i int, rest Restriction) float64 {
-       return (float64)(i + rest.Val)
+// RestrictionSize just returns i as the size, as well as flipping appropriate
+// flags on the restriction to track that this was called.
+func (fn *VetSdf) RestrictionSize(i int, rest *VetRestriction) float64 {
+       rest.Key = nil
+       rest.Val = i
+       rest.RestSize = true
+       return (float64)(i)
 }
 
-// CreateTracker creates an RTracker containing the given restriction and a Val
-// of 1.
-func (fn *Sdf) CreateTracker(rest Restriction) *RTracker {
-       return &RTracker{rest, 1}
+// CreateTracker creates an RTracker containing the given restriction and flips
+// the appropriate flags on the restriction to track that this was called.
+func (fn *VetSdf) CreateTracker(rest *VetRestriction) *VetRTracker {
+       rest.CreateTracker = true
+       return &VetRTracker{rest}
 }
 
-// ProcessElement emits a pair of ints. The first is the input +
-// RTracker.Rest.Val. The second is the input + RTracker.Val.
-func (fn *Sdf) ProcessElement(rt *RTracker, i int, emit func(int, int)) {
-       emit(i+rt.Rest.Val, i+rt.Val)
+// ProcessElement emits a copy of the restriction in the restriction tracker it
+// received, with the appropriate flags flipped to track that this was called.
+func (fn *VetSdf) ProcessElement(rt *VetRTracker, i int, emit 
func(VetRestriction)) {
+       rest := *rt.Rest
+       rest.Key = nil
+       rest.Val = i
+       rest.ProcessElm = true
+       emit(rest)
 }
 
-type KvSdf struct {
+// VetKvSdf runs an SDF In order to test that these methods get called 
properly,
+// each method will flip the corresponding flag in the passed in 
VetRestriction,
+// overwrite the restriction's Key and Val with the last seen input elements,
+// and retain the other fields in the VetRestriction.
+type VetKvSdf struct {
 }
 
-// CreateInitialRestriction creates a restriction with the sum of the given
-// values.
-func (fn *KvSdf) CreateInitialRestriction(i int, j int) Restriction {
-       return Restriction{i + j}
+// CreateInitialRestriction creates a restriction with the given values and
+// with the appropriate flags to track that this was called.
+func (fn *VetKvSdf) CreateInitialRestriction(i, j int) *VetRestriction {
+       return &VetRestriction{ID: "KvSdf", Key: i, Val: j, CreateRest: true}
 }
 
-// SplitRestriction outputs two restrictions, the first containing the sum of i
-// and rest.Val, the second containing the sum of j and rest.Val.
-func (fn *KvSdf) SplitRestriction(i int, j int, rest Restriction) 
[]Restriction {
-       return []Restriction{{rest.Val + i}, {rest.Val + j}}
+// SplitRestriction outputs two identical restrictions, each being a copy of 
the
+// initial one, but with the appropriate flags to track this was called. The
+// split restrictions add a suffix of the form ".#" to the ID.
+func (fn *VetKvSdf) SplitRestriction(i, j int, rest *VetRestriction) 
[]*VetRestriction {
+       rest.SplitRest = true
+       rest1 := &VetRestriction{
+               ID:            rest.ID + ".1",
+               Key:           i,
+               Val:           j,
+               CreateRest:    rest.CreateRest,
+               SplitRest:     true,
+               RestSize:      rest.RestSize,
+               CreateTracker: rest.CreateTracker,
+               ProcessElm:    rest.ProcessElm,
+       }
+       rest2 := &VetRestriction{}
+       *rest2 = *rest1
+       rest2.ID = rest.ID + ".2"
+       return []*VetRestriction{rest1, rest2}
 }
 
-// RestrictionSize returns the sum of i, j, and rest.Val as a float64.
-func (fn *KvSdf) RestrictionSize(i int, j int, rest Restriction) float64 {
-       return (float64)(i + j + rest.Val)
+// RestrictionSize just returns the sum of i and j as the size, as well as
+// flipping appropriate flags on the restriction to track that this was called.
+func (fn *VetKvSdf) RestrictionSize(i, j int, rest *VetRestriction) float64 {
+       rest.Key = i
+       rest.Val = j
+       rest.RestSize = true
+       return (float64)(i + j)
 }
 
-// CreateTracker creates an RTracker containing the given restriction and a Val
-// of 2.
-func (fn *KvSdf) CreateTracker(rest Restriction) *RTracker {
-       return &RTracker{rest, 2}
+// CreateTracker creates an RTracker containing the given restriction and flips
+// the appropriate flags on the restriction to track that this was called.
+func (fn *VetKvSdf) CreateTracker(rest *VetRestriction) *VetRTracker {
+       rest.CreateTracker = true
+       return &VetRTracker{rest}
 }
 
-// ProcessElement emits two ints. The first is the first input (key) +
-// RTracker.Rest.Val. The second is the second input (value) + RTracker.Val.
-func (fn *KvSdf) ProcessElement(rt *RTracker, i1 int, i2 int, emit func(int, 
int)) {
-       emit(i1+rt.Rest.Val, i2+rt.Val)
+// ProcessElement emits a copy of the restriction in the restriction tracker it
+// received, with the appropriate flags flipped to track that this was called.
+func (fn *VetKvSdf) ProcessElement(rt *VetRTracker, i, j int, emit 
func(VetRestriction)) {

Review comment:
       It's a little awkward to have both Value instances of a type and Pointer 
instances of a type. Usually one is using one or the other. This makes it 
harder in to understand the test expectations, which if things need to change, 
could cause harder to diagnose errors.
   
   Since you're using pointers to allow for the  "was this specific instance 
processed?" consider consolidating on the pointer types.




----------------------------------------------------------------
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:
us...@infra.apache.org


Issue Time Tracking
-------------------

    Worklog Id:     (was: 431871)
    Time Spent: 0.5h  (was: 20m)

> Generalize SDF-validating restrictions
> --------------------------------------
>
>                 Key: BEAM-9883
>                 URL: https://issues.apache.org/jira/browse/BEAM-9883
>             Project: Beam
>          Issue Type: Sub-task
>          Components: sdk-go
>            Reporter: Daniel Oliveira
>            Assignee: Daniel Oliveira
>            Priority: Major
>          Time Spent: 0.5h
>  Remaining Estimate: 0h
>
> We have some restrictions written for the purpose of validating that SDFs 
> work in sdf_invokers_test.go, but they can be improved and generalized. The 
> main improvement is changing the validation approach so that the restriction 
> keeps track of each method it's had called on it. Then this can be 
> generalized so that it can be used in upcoming integration tests to validate 
> which methods are being called.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to