From 9b13ee89ca08cd295fd96e0fd7c369a86d2935fe Mon Sep 17 00:00:00 2001 From: Nicolas Lopez Date: Tue, 18 Feb 2020 17:17:41 -0500 Subject: [PATCH] address review comments --- test/e2e/e2e_test.go | 2 +- test/e2e/lib/pubsub.go | 7 ++++++- test/e2e/test_broker_pubsub.go | 35 ++++++++++++++++++++++++++++------ 3 files changed, 36 insertions(+), 8 deletions(-) diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index 684646b495..9c415f896a 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -202,7 +202,7 @@ func TestBrokerWithPubSubChannel(t *testing.T) { BrokerWithPubSubChannelTestImpl(t) } -// TestBrokerWithPubSubChannel tests we can knock a Knative Service from a broker with PubSub Channel from a CloudPubSubSource. +// TestCloudPubSubSourceBrokerWithPubSubChannel tests we can knock a Knative Service from a broker with PubSub Channel from a CloudPubSubSource. func TestCloudPubSubSourceBrokerWithPubSubChannel(t *testing.T) { cancel := logstream.Start(t) defer cancel() diff --git a/test/e2e/lib/pubsub.go b/test/e2e/lib/pubsub.go index 537b6090d7..38629a2883 100644 --- a/test/e2e/lib/pubsub.go +++ b/test/e2e/lib/pubsub.go @@ -29,7 +29,12 @@ import ( pkgmetrics "knative.dev/pkg/metrics" ) -func MakePubSubOrDie(t *testing.T, client *Client, gvk metav1.GroupVersionKind, psName, targetName, topicName string, so ...kngcptesting.CloudPubSubSourceOption) { +func MakePubSubOrDie(t *testing.T, + client *Client, + gvk metav1.GroupVersionKind, + psName, targetName, topicName string, + so ...kngcptesting.CloudPubSubSourceOption, +) { so = append(so, kngcptesting.WithCloudPubSubSourceSink(gvk, targetName)) so = append(so, kngcptesting.WithCloudPubSubSourceTopic(topicName)) eventsPubsub := kngcptesting.NewCloudPubSubSource(psName, client.Namespace, so...) diff --git a/test/e2e/test_broker_pubsub.go b/test/e2e/test_broker_pubsub.go index a0d1e5b54c..fdb777f392 100644 --- a/test/e2e/test_broker_pubsub.go +++ b/test/e2e/test_broker_pubsub.go @@ -66,7 +66,14 @@ func BrokerWithPubSubChannelTestImpl(t *testing.T) { client := lib.Setup(t, true) defer lib.TearDown(client) - u := createBrokerWithPubSubChannel(t, client, brokerName, dummyTriggerName, kserviceName, respTriggerName, targetName) + u := createBrokerWithPubSubChannel(t, + client, + brokerName, + dummyTriggerName, + kserviceName, + respTriggerName, + targetName, + ) // Just to make sure all resources are ready. time.Sleep(5 * time.Second) @@ -104,13 +111,27 @@ func PubSubSourceBrokerWithPubSubChannelTestImpl(t *testing.T) { client := lib.Setup(t, true) defer lib.TearDown(client) - u := createBrokerWithPubSubChannel(t, client, brokerName, dummyTriggerName, kserviceName, respTriggerName, targetName) + u := createBrokerWithPubSubChannel(t, + client, + brokerName, + dummyTriggerName, + kserviceName, + respTriggerName, + targetName, + ) var url apis.URL = apis.URL(u) // Just to make sure all resources are ready. time.Sleep(5 * time.Second) // Create the PubSub source. - lib.MakePubSubOrDie(t, client, lib.ServiceGVK, psName, targetName, topicName, kngcptesting.WithCloudPubSubSourceSinkURI(&url)) + lib.MakePubSubOrDie(t, + client, + lib.ServiceGVK, + psName, + targetName, + topicName, + kngcptesting.WithCloudPubSubSourceSinkURI(&url), + ) topic := lib.GetTopic(t, topicName) @@ -131,11 +152,13 @@ func PubSubSourceBrokerWithPubSubChannelTestImpl(t *testing.T) { t.Error("resp event didn't hit the target pod") t.Failed() } - - // TODO(nlopezgi): define if we want to assert StackDriver metrics here. CloudPubSubSourceWithTargetTestImpl can do so, but the test is currently disabled, so its unclear whether we want to replicate that functionality here. + // TODO(nlopezgi): assert StackDriver metrics after https://github.com/google/knative-gcp/issues/317 is resolved } -func createBrokerWithPubSubChannel(t *testing.T, client *lib.Client, brokerName, dummyTriggerName, kserviceName, respTriggerName, targetName string) url.URL { +func createBrokerWithPubSubChannel(t *testing.T, + client *lib.Client, + brokerName, dummyTriggerName, kserviceName, respTriggerName, targetName string, +) url.URL { // Create a new Broker. // TODO(chizhg): maybe we don't need to create these RBAC resources as they will now be automatically created? client.Core.CreateRBACResourcesForBrokers()