[GitHub] [beam] lostluck commented on a change in pull request #15253: [BEAM-11088] Add TestStream package to Go SDK testing capabilities

[GitBox <gi...@apache.org>]

lostluck commented on a change in pull request #15253:
URL: https://github.com/apache/beam/pull/15253#discussion_r681860491



##########
File path: sdks/go/pkg/beam/testing/teststream/teststream.go
##########
@@ -0,0 +1,150 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements.  See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License.  You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// Package teststream contains code configuring the TestStream primitive for
+// use in testing code that is meant to be run on streaming data sources.
+//
+// See https://beam.apache.org/blog/test-stream/ for more information.
+//
+// TestStream is supported on the Flink runner.
+package teststream
+
+import (
+	"bytes"
+	"fmt"
+	"reflect"
+
+	"github.com/apache/beam/sdks/go/pkg/beam"
+	"github.com/apache/beam/sdks/go/pkg/beam/core/graph/mtime"
+	"github.com/apache/beam/sdks/go/pkg/beam/core/typex"
+	"github.com/apache/beam/sdks/go/pkg/beam/core/util/protox"
+
+	pipepb "github.com/apache/beam/sdks/go/pkg/beam/model/pipeline_v1"
+)
+
+const urn = "beam:transform:teststream:v1"
+
+// Config holds information used to create a TestStreamPayload object.
+type Config struct {
+	elmType   beam.FullType
+	events    []*pipepb.TestStreamPayload_Event
+	endpoint  *pipepb.ApiServiceDescriptor
+	watermark int64
+}
+
+// NewConfig returns a Config to build a sequence of a test stream's events.
+// Requires that users provide the coder for the elements they are trying to emit.
+func NewConfig() Config {
+	return Config{elmType: nil,
+		events:    []*pipepb.TestStreamPayload_Event{},
+		endpoint:  &pipepb.ApiServiceDescriptor{},
+		watermark: 0,
+	}
+}
+
+// SetEndpoint sets a URL for a TestStreamService that will emit events instead of having them
+// defined manually. Currently does not support authentication, so the TestStreamService should
+// be accessed in a trusted context.
+func (c *Config) setEndpoint(url string) {
+	c.endpoint.Url = url
+}
+
+// createPayload converts the Config object into a TestStreamPayload to be sent to the runner.
+func (c *Config) createPayload() *pipepb.TestStreamPayload {
+	return &pipepb.TestStreamPayload{CoderId: "c0", Events: c.events, Endpoint: c.endpoint}
+}
+
+// AdvanceWatermark adds an event to the Config Events struct advancing the watermark for the PCollection
+// to the given timestamp. Timestamp is in milliseconds
+func (c *Config) AdvanceWatermark(timestamp int64) error {
+	if c.watermark >= timestamp {
+		return fmt.Errorf("watermark must be monotonally increasing, is at %v, got %v", c.watermark, timestamp)
+	}
+	watermarkAdvance := &pipepb.TestStreamPayload_Event_AdvanceWatermark{NewWatermark: timestamp}
+	watermarkEvent := &pipepb.TestStreamPayload_Event_WatermarkEvent{WatermarkEvent: watermarkAdvance}
+	c.events = append(c.events, &pipepb.TestStreamPayload_Event{Event: watermarkEvent})
+	c.watermark = timestamp
+	return nil
+}
+
+// AdvanceWatermarkToInfinity advances the watermark to the maximum timestamp.
+func (c *Config) AdvanceWatermarkToInfinity() error {
+	return c.AdvanceWatermark(mtime.MaxTimestamp.Milliseconds())
+}
+
+// AdvanceProcessingTime adds an event advancing the processing time by a given duration.
+// This advancement is applied to all of the PCollections output by the TestStream.
+func (c *Config) AdvanceProcessingTime(duration int64) {
+	processingAdvance := &pipepb.TestStreamPayload_Event_AdvanceProcessingTime{AdvanceDuration: duration}
+	processingEvent := &pipepb.TestStreamPayload_Event_ProcessingTimeEvent{ProcessingTimeEvent: processingAdvance}
+	c.events = append(c.events, &pipepb.TestStreamPayload_Event{Event: processingEvent})
+}
+
+// AdvanceProcessingTimeToInfinity moves the TestStream processing time to the largest possible
+// timestamp.
+func (c *Config) AdvanceProcessingTimeToInfinity() {
+	c.AdvanceProcessingTime(mtime.MaxTimestamp.Milliseconds())
+}
+
+// AddElements adds a number of elements to the stream at the specified event timestamp. Must be called with
+// at least one element.
+//
+// On the first call, a type will be inferred from the passed in elements, which must be of all the same type.
+// Type mismatches on this or subsequent calls will cause AddElements to return an error.
+func (c *Config) AddElements(timestamp int64, elements ...interface{}) error {
+	t := reflect.TypeOf(elements[0])
+	if c.elmType == nil {
+		c.elmType = typex.New(t)
+	} else if c.elmType.Type() != t {
+		return fmt.Errorf("element type mismatch, previous additions were of type %v, tried to add type %v", c.elmType, t)
+	}
+	newElements := []*pipepb.TestStreamPayload_TimestampedElement{}
+	enc := beam.NewElementEncoder(t)
+	for _, e := range elements {
+		var buf bytes.Buffer

Review comment:
       Consider validating that the rest of the elements in the slice are actually the same type to provide a clearer error message than "encoding value %v failed..."
   
   Something like "element %d was type %T but previous additions were of type %v", i, e, c.elmType  ?

##########
File path: sdks/go/pkg/beam/testing/teststream/teststream.go
##########
@@ -0,0 +1,150 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements.  See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License.  You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// Package teststream contains code configuring the TestStream primitive for
+// use in testing code that is meant to be run on streaming data sources.
+//
+// See https://beam.apache.org/blog/test-stream/ for more information.
+//
+// TestStream is supported on the Flink runner.
+package teststream
+
+import (
+	"bytes"
+	"fmt"
+	"reflect"
+
+	"github.com/apache/beam/sdks/go/pkg/beam"
+	"github.com/apache/beam/sdks/go/pkg/beam/core/graph/mtime"
+	"github.com/apache/beam/sdks/go/pkg/beam/core/typex"
+	"github.com/apache/beam/sdks/go/pkg/beam/core/util/protox"
+
+	pipepb "github.com/apache/beam/sdks/go/pkg/beam/model/pipeline_v1"
+)
+
+const urn = "beam:transform:teststream:v1"
+
+// Config holds information used to create a TestStreamPayload object.
+type Config struct {
+	elmType   beam.FullType
+	events    []*pipepb.TestStreamPayload_Event
+	endpoint  *pipepb.ApiServiceDescriptor
+	watermark int64
+}
+
+// NewConfig returns a Config to build a sequence of a test stream's events.
+// Requires that users provide the coder for the elements they are trying to emit.
+func NewConfig() Config {
+	return Config{elmType: nil,
+		events:    []*pipepb.TestStreamPayload_Event{},
+		endpoint:  &pipepb.ApiServiceDescriptor{},
+		watermark: 0,
+	}
+}
+
+// SetEndpoint sets a URL for a TestStreamService that will emit events instead of having them
+// defined manually. Currently does not support authentication, so the TestStreamService should
+// be accessed in a trusted context.
+func (c *Config) setEndpoint(url string) {
+	c.endpoint.Url = url
+}
+
+// createPayload converts the Config object into a TestStreamPayload to be sent to the runner.
+func (c *Config) createPayload() *pipepb.TestStreamPayload {
+	return &pipepb.TestStreamPayload{CoderId: "c0", Events: c.events, Endpoint: c.endpoint}

Review comment:
       Add a comment explaining what the "c0" is and why we can get away with hard coding it.

##########
File path: sdks/go/pkg/beam/testing/teststream/teststream.go
##########
@@ -0,0 +1,150 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements.  See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License.  You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// Package teststream contains code configuring the TestStream primitive for
+// use in testing code that is meant to be run on streaming data sources.
+//
+// See https://beam.apache.org/blog/test-stream/ for more information.
+//
+// TestStream is supported on the Flink runner.
+package teststream
+
+import (
+	"bytes"
+	"fmt"
+	"reflect"
+
+	"github.com/apache/beam/sdks/go/pkg/beam"
+	"github.com/apache/beam/sdks/go/pkg/beam/core/graph/mtime"
+	"github.com/apache/beam/sdks/go/pkg/beam/core/typex"
+	"github.com/apache/beam/sdks/go/pkg/beam/core/util/protox"
+
+	pipepb "github.com/apache/beam/sdks/go/pkg/beam/model/pipeline_v1"
+)
+
+const urn = "beam:transform:teststream:v1"
+
+// Config holds information used to create a TestStreamPayload object.
+type Config struct {
+	elmType   beam.FullType
+	events    []*pipepb.TestStreamPayload_Event
+	endpoint  *pipepb.ApiServiceDescriptor
+	watermark int64
+}
+
+// NewConfig returns a Config to build a sequence of a test stream's events.
+// Requires that users provide the coder for the elements they are trying to emit.
+func NewConfig() Config {
+	return Config{elmType: nil,
+		events:    []*pipepb.TestStreamPayload_Event{},
+		endpoint:  &pipepb.ApiServiceDescriptor{},
+		watermark: 0,

Review comment:
       Nit: should probably start the watermark at the [mtime.MinTimestamp](https://github.com/apache/beam/blob/master/sdks/go/pkg/beam/core/graph/mtime/time.go#L32) 

##########
File path: sdks/go/pkg/beam/testing/teststream/teststream_test.go
##########
@@ -0,0 +1,116 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements.  See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License.  You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package teststream
+
+import (
+	"bytes"
+	"reflect"
+	"testing"
+
+	"github.com/apache/beam/sdks/go/pkg/beam"
+)
+
+func TestNewConfig(t *testing.T) {
+	con := NewConfig()
+	if con.elmType != nil {
+		t.Errorf("type is not correct, expected nil, got %v", con.elmType)
+	}
+	if len(con.events) != 0 {
+		t.Errorf("config has too many elements, expected 0, got %v", len(con.events))
+	}
+	if con.endpoint.Url != "" {
+		t.Errorf("config has URL endpoint when it should be empty")
+	}
+}
+
+func TestAdvanceWatermark(t *testing.T) {
+	con := NewConfig()
+	con.AdvanceWatermark(500)
+	if w := con.watermark; w != 500 {
+		t.Errorf("default watermark expected 500, got %v", w)
+	}
+	if len(con.events) != 1 {
+		t.Fatalf("expected only 1 event in config, got %v", len(con.events))
+	}
+	if eventWatermark := con.events[0].GetWatermarkEvent().NewWatermark; eventWatermark != 500 {
+		t.Errorf("expected watermark in event was 500, got %v", eventWatermark)

Review comment:
       Style nit: Go idiom prefers using "want" over "expected". Not required if want is more convoluted to use of course.




-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org