[GitHub] [beam] damccorm commented on a change in pull request #16942: [BEAM-13910] Improve coverage of gcsx package

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

damccorm commented on a change in pull request #16942:
URL: https://github.com/apache/beam/pull/16942#discussion_r814281991



##########
File path: sdks/go/pkg/beam/util/gcsx/gcs_test.go
##########
@@ -13,35 +13,62 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package gcsx_test
+package gcsx
 
 import (
-	"context"
-	"time"
+	"testing"
 
-	"cloud.google.com/go/storage"
-	"github.com/apache/beam/sdks/v2/go/pkg/beam/util/gcsx"
+	"github.com/apache/beam/sdks/v2/go/pkg/beam/internal/errors"
 )
 
-func Example() {
-	ctx := context.Background()
-	c, err := gcsx.NewClient(ctx, storage.ScopeReadOnly)
-	if err != nil {
-		// do something
+func TestMakeObject(t *testing.T) {
+	if got, want := MakeObject("some-bucket", "some/path"), "gs://some-bucket/some/path"; got != want {
+		t.Fatalf("MakeObject() Got: %v Want: %v", got, want)
 	}
+}
 
-	buckets, object, err := gcsx.ParseObject("gs://some-bucket/some-object")
-	if err != nil {
-		// do something
+func TestParseObject(t *testing.T) {
+	tests := []struct {
+		object string
+		bucket string
+		path   string
+		err    error
+	}{
+		{
+			object: "gs://some-bucket/some-object",
+			bucket: "some-bucket",
+			path:   "some-object",
+			err:    nil,
+		},
+		{
+			object: "gs://some-bucket",
+			bucket: "some-bucket",
+			path:   "",
+			err:    nil,
+		},
+		{
+			object: "gs://",
+			bucket: "",
+			path:   "",
+			err:    errors.Errorf("object gs:// must have bucket"),
+		},
+		{
+			object: "other://some-bucket/some-object",
+			bucket: "",
+			path:   "",
+			err:    errors.Errorf("object other://some-bucket/some-object must have 'gs' scheme"),
+		},
 	}
 
-	ctx, cancel := context.WithTimeout(ctx, 30*time.Second)
-	defer cancel()
-
-	bytes, err := gcsx.ReadObject(ctx, c, buckets, object)
-	if err != nil {
-		// do something
+	for _, test := range tests {

Review comment:
       Nice! I think in this case test.object acts as that identifier, but I appreciate the pointer (I vaguely knew there was some concept like this, but didn't know how to actually use it)




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